Subject: Handling orphans in config(1)
To: None <tech-kern@netbsd.org>
From: Quentin Garnier <cube@cubidou.net>
List: tech-kern
Date: 09/24/2005 22:39:19
--Bmpniq+Thlljzm46
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi folks,
While trying to find ways of extending the 'no <dev> at <attach>' syntax
of config, I came upon the issue of orphans.
First, the way config(1) currently detects orphans is broken: it will
not warn about doing the following:
include "arch/i386/conf/GENERIC"
no pci* at mainbus?
although it disables completely PCI support.
Also, when it find an orphan, it only warns and goes on like nothing
happened, which means the kernel linking step will usually fail.
The following patch solves the first issue the only way it could
properly be done. As for the second, it keeps the warning but ignores
the orphaned device so that you get a kernel that compiles (well,
provided the code dependencies are correct, i.e. compiling in echi*
but no usb* will fail when it should not, no matter how stupid it is).
In essence, the following patch allows the user to do the following:
include "arch/i386/conf/GENERIC"
no echi* at pci?
no uhci* at pci?
no ohci* at pci?
to completely disable USB support. config(1) will warn about all
orphaned devices (e.g., uhub* at usb? which was declared in GENERIC),
but the kernel will compile and work as expected, that is with no USB
support.
The reason why I'm not committing this right away is because I want to
gather opinions on how to do the next level.
I think the current warning is useless: it's either we silently ignore
the orphans (or maybe, print a notice in verbose mode) or we exit in
error.
Another option would be to propagate the negation of a device instance
immediately, but in my eyes this is an issue, because it would mean
that 'no <dev> at <attach>' makes the configuration file ordered,
while all the other syntaxes don't have that requirement (see how
'pci* at ppb?' is declared before 'ppb* at pci?'). Of course, the
'no <dev> at <attach>' syntax itself depends on what happened before,
but that's the point of it. What I mean is that it is weird to be able
to move positive declaration around anywhere, except across negative
ones that affect ancestors. I'm not sure if I'm being very clear here.
Also, while I haven't thought of how to implement it right now, I think
it would make 'no <dev> at <attach>' (and derivates) very complex and
would require a lot more state to be kept during config file parsing.
I'd like to commit that patch anyway, because it makes existing
'no <dev> at <attach>' a lot more useful to people who are not afraid of
getting the warnings, and it makes it possible to have
'no device at <attach>' and 'no <dev>' very soon.
The only thing that bothers me with the patch is that it will not make
the kernel compilation fail which means the user might be confused by a
kernel ignoring devices he expected to be found, but then, how is that
less confusing that a kernel failing to link looking for obscure
functions?
Index: defs.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /rep/NetBSD-src/cvs/src/usr.bin/config/defs.h,v
retrieving revision 1.2
diff -u -r1.2 defs.h
--- defs.h 2005/09/10 15:38:46 1.2
+++ defs.h 2005/09/24 19:52:30
@@ -165,6 +165,7 @@
int p_atunit; /* optional parent device unit */
struct nvlist *p_devs; /* children using it */
int p_inst; /* parent spec instance */
+ int p_active; /* parent spec is actively used */
};
=20
/*
@@ -242,6 +243,7 @@
const char **i_locs; /* locators (as given by pspec's iattr) */
int i_cfflags; /* flags from config line */
int i_lineno; /* line # in config, for later errors */
+ int i_active; /* instance is not orphaned in any way */
=20
/* created during packing or ioconf.c generation */
short i_collapsed; /* set =3D> this alias no longer needed */
@@ -362,6 +364,7 @@
struct nvlist *appmkoptions; /* appending mkoptions */
struct hashtab *condmkopttab; /* conditional makeoption table */
struct hashtab *devbasetab; /* devbase lookup */
+struct hashtab *devroottab; /* attach at root lookup */
struct hashtab *devatab; /* devbase attachment lookup */
struct hashtab *devitab; /* device instance lookup */
struct hashtab *selecttab; /* selects things that are "optional foo" */
Index: main.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /rep/NetBSD-src/cvs/src/usr.bin/config/main.c,v
retrieving revision 1.1
diff -u -r1.1 main.c
--- main.c 2005/06/05 18:19:53 1.1
+++ main.c 2005/09/24 19:52:30
@@ -104,7 +104,9 @@
int main(int, char **);
static int mksymlinks(void);
static int mkident(void);
-static int hasparent(struct devi *);
+static void kill_orphans(void);
+static void do_kill_orphans(struct devbase *, struct attr *, struct devbas=
e *);
+static int kill_orphans_cb(const char *, void *, void *);
static int cfcrosscheck(struct config *, const char *, struct nvlist *);
static const char *strtolower(const char *);
void defopt(struct hashtab *ht, const char *fname,
@@ -249,6 +251,7 @@
initsem();
ident =3D NULL;
devbasetab =3D ht_new();
+ devroottab =3D ht_new();
devatab =3D ht_new();
devitab =3D ht_new();
selecttab =3D ht_new();
@@ -337,6 +340,11 @@
unlink(cname);
=20
/*
+ * Detect and properly ignore orphaned devices
+ */
+ kill_orphans();
+
+ /*
* Select devices and pseudo devices and their attributes
*/
fixdevis();
@@ -987,10 +995,8 @@
{
struct devi *i;
=20
- if (unit =3D=3D WILD)
- return (deva->d_ihead !=3D NULL);
for (i =3D deva->d_ihead; i !=3D NULL; i =3D i->i_asame)
- if (unit =3D=3D i->i_unit)
+ if (i->i_active && (unit =3D=3D WILD || unit =3D=3D i->i_unit))
return (1);
return (0);
}
@@ -1034,44 +1040,6 @@
}
=20
static int
-hasparent(struct devi *i)
-{
- struct pspec *p;
- struct nvlist *nv;
-
- /*
- * We determine whether or not a device has a parent in in one
- * of two ways:
- * (1) If a parent device was named in the config file,
- * i.e. cases (2) and (3) in sem.c:adddev(), then
- * we search its devbase for a matching unit number.
- * (2) If the device was attach to an attribute, then we
- * search all attributes the device can be attached to
- * for parents (with appropriate unit numebrs) that
- * may be able to attach the device.
- */
-
- /* No pspec, no parent (root node). */
- if ((p =3D i->i_pspec) =3D=3D NULL)
- return (0);
-
- /*
- * Case (1): A parent was named. Either it's configured, or not.
- */
- if (p->p_atdev !=3D NULL)
- return (devbase_has_instances(p->p_atdev, p->p_atunit));
-
- /*
- * Case (2): No parent was named. Look for devs that provide the attr.
- */
- if (p->p_iattr !=3D NULL)
- for (nv =3D p->p_iattr->a_refs; nv !=3D NULL; nv =3D nv->nv_next)
- if (devbase_has_instances(nv->nv_ptr, p->p_atunit))
- return (1);
- return (0);
-}
-
-static int
cfcrosscheck(struct config *cf, const char *what, struct nvlist *nv)
{
struct devbase *dev;
@@ -1120,21 +1088,10 @@
int
crosscheck(void)
{
- struct pspec *p;
- struct devi *i;
struct config *cf;
int errs;
=20
errs =3D 0;
- TAILQ_FOREACH(i, &alldevi, i_next) {
- if ((p =3D i->i_pspec) =3D=3D NULL || hasparent(i))
- continue;
- (void)fprintf(stderr,
- "%s:%d: `%s at %s' is orphaned (%s `%s' declared)\n",
- conffile, i->i_lineno, i->i_name, i->i_at,
- p->p_atunit =3D=3D WILD ? "nothing matching" : "no",
- i->i_at);
- }
if (TAILQ_EMPTY(&allcf)) {
(void)fprintf(stderr, "%s has no configurations!\n",
conffile);
@@ -1517,4 +1474,78 @@
close(kfd);
=20
return found;
+}
+
+static void
+do_kill_orphans(struct devbase *d, struct attr *at, struct devbase *parent)
+{
+ struct nvlist *nv, *nv1;
+ struct attr *a;
+ struct devi *i, *j =3D NULL;
+ struct pspec *p;
+
+ /*
+ * A pseudo-device will always attach at root, and if it has an
+ * instance (it cannot have more than one), it is enough to consider
+ * it active, as there is no real attachment.
+ */
+ if (d->d_ispseudo) {
+ if (d->d_ihead =3D=3D NULL)
+ return;
+ d->d_ihead->i_active =3D 1;
+ } else {
+ int active =3D 0;
+ for (i =3D d->d_ihead; i !=3D NULL; i =3D i->i_bsame) {
+ for (j =3D i; j !=3D NULL; j =3D j->i_alias) {
+ p =3D j->i_pspec;
+ if ((p =3D=3D NULL && at =3D=3D NULL) ||
+ (p !=3D NULL && p->p_iattr =3D=3D at &&
+ (p->p_atdev =3D=3D NULL ||
+ p->p_atdev =3D=3D parent))) {
+ if (p !=3D NULL &&
+ !devbase_has_instances(parent,
+ p->p_atunit))
+ continue;
+ /*
+ * There are Fry-like devices which can
+ * be their own grand-parent (or even
+ * parent, like uhub). We don't want
+ * to loop, so if we've already reached
+ * an instance for one reason or
+ * another, stop there.
+ */
+ if (j->i_active)
+ /*
+ * Device has already been
+ * seen
+ */
+ return;
+ j->i_active =3D active =3D 1;
+ if (p !=3D NULL)
+ p->p_active =3D 1;
+ }
+ }
+ }
+ if (!active)
+ return;
+ }
+
+ for (nv =3D d->d_attrs; nv !=3D NULL; nv =3D nv->nv_next) {
+ a =3D nv->nv_ptr;
+ for (nv1 =3D a->a_devs; nv1 !=3D NULL; nv1 =3D nv1->nv_next)
+ do_kill_orphans(nv1->nv_ptr, a, d);
+ }
+}
+
+static int
+kill_orphans_cb(const char *key, void *value, void *aux)
+{
+ do_kill_orphans((struct devbase *)value, NULL, NULL);
+ return 0;
+}
+
+static void
+kill_orphans()
+{
+ ht_enumerate(devroottab, kill_orphans_cb, NULL);
}
Index: mkioconf.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /rep/NetBSD-src/cvs/src/usr.bin/config/mkioconf.c,v
retrieving revision 1.3
diff -u -r1.3 mkioconf.c
--- mkioconf.c 2005/08/25 15:01:07 1.3
+++ mkioconf.c 2005/09/24 19:52:30
@@ -359,7 +359,7 @@
=20
NEWLINE;
TAILQ_FOREACH(p, &allpspecs, p_list) {
- if (p->p_devs =3D=3D NULL)
+ if (p->p_devs =3D=3D NULL || !p->p_active)
continue;
if (fprintf(fp,
"static const struct cfparent pspec%d =3D {\n", p->p_inst) < 0)
Index: pack.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /rep/NetBSD-src/cvs/src/usr.bin/config/pack.c,v
retrieving revision 1.1
diff -u -r1.1 pack.c
--- pack.c 2005/06/05 18:19:53 1.1
+++ pack.c 2005/09/24 19:52:30
@@ -115,7 +115,7 @@
*/
locspace =3D 0;
TAILQ_FOREACH(i, &alldevi, i_next) {
- if (i->i_collapsed)
+ if (!i->i_active || i->i_collapsed)
continue;
if ((p =3D i->i_pspec) =3D=3D NULL)
continue;
@@ -177,10 +177,17 @@
/*
* For each instance of each device, add or collapse
* all its aliases.
+ *
+ * Pseudo-devices have a non-empty d_ihead for convenience.
+ * Ignore them.
*/
+ if (d->d_ispseudo)
+ continue;
for (i =3D d->d_ihead; i !=3D NULL; i =3D i->i_bsame) {
m =3D n;
for (l =3D i; l !=3D NULL; l =3D l->i_alias) {
+ if (!l->i_active)
+ continue;
l->i_locoff =3D -1;
/* try to find an equivalent for l */
for (j =3D m; j < n; j++) {
Index: sem.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /rep/NetBSD-src/cvs/src/usr.bin/config/sem.c,v
retrieving revision 1.5
diff -u -r1.5 sem.c
--- sem.c 2005/08/07 15:11:12 1.5
+++ sem.c 2005/09/24 19:52:30
@@ -346,6 +346,13 @@
goto bad;
attrs =3D newnv(dev->d_name, NULL, getattr(dev->d_name), 0,
attrs);
+
+ /*
+ * Pseudo-devices can have children. Consider them as
+ * attaching at root.
+ */
+ if (ispseudo)
+ ht_insert(devroottab, dev->d_name, dev);
}
=20
/* Committed! Set up fields. */
@@ -493,8 +500,10 @@
error("attach at `%s' already done by `%s'",
a ? a->a_name : "root", da->d_name);
=20
- if (a =3D=3D NULL)
+ if (a =3D=3D NULL) {
+ ht_insert(devroottab, dev->d_name, dev);
continue; /* at root; don't add */
+ }
if (!a->a_iattr)
error("%s cannot be at plain attribute `%s'",
dev->d_name, a->a_name);
@@ -867,6 +876,7 @@
i->i_locs =3D NULL;
i->i_cfflags =3D 0;
i->i_lineno =3D currentline();
+ i->i_active =3D 0;
if (unit >=3D d->d_umax)
d->d_umax =3D unit + 1;
return (i);
@@ -1205,6 +1215,9 @@
i =3D newdevi(name, number - 1, d); /* foo 16 =3D> "foo0..foo15" */
if (ht_insert(devitab, name, i))
panic("addpseudo(%s)", name);
+ /* Useful to retrieve the instance from the devbase */
+ d->d_ihead =3D i;
+ i->i_active =3D 1;
TAILQ_INSERT_TAIL(&allpseudo, i, i_next);
}
=20
@@ -1275,10 +1288,22 @@
struct devi *i;
=20
TAILQ_FOREACH(i, &alldevi, i_next)
- selectbase(i->i_base, i->i_atdeva);
+ if (i->i_active)
+ selectbase(i->i_base, i->i_atdeva);
+ else
+ /*
+ * At this point, we can't have instances for which
+ * i_at or i_pspec are NULL.
+ */
+ (void)fprintf(stderr,
+ "%s:%d: `%s at %s' is orphaned"
+ " (%s `%s' found)\n", conffile, i->i_lineno,
+ i->i_name, i->i_at, i->i_pspec->p_atunit =3D=3D WILD ?
+ "nothing matching" : "no", i->i_at);
=20
TAILQ_FOREACH(i, &allpseudo, i_next)
- selectbase(i->i_base, NULL);
+ if (i->i_active)
+ selectbase(i->i_base, NULL);
}
=20
/*
@@ -1302,6 +1327,7 @@
p->p_atdev =3D ab;
p->p_atunit =3D atunit;
p->p_inst =3D npspecs++;
+ p->p_active =3D 0;
=20
TAILQ_INSERT_TAIL(&allpspecs, p, p_list);
=20
--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"When I find the controls, I'll go where I like, I'll know where I want
to be, but maybe for now I'll stay right here on a silent sea."
KT Tunstall, Silent Sea, Eye to the Telescope, 2004.
--Bmpniq+Thlljzm46
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (NetBSD)
iQEVAwUBQzW5d9goQloHrPnoAQIghAf+NLJcMm6vJ30WTTjCpJazjaDAjQINT86v
E9ExK79ulMJPz9+Tfh0DmQmFxaOG7S6uEusf14eGWUqHs4EOj7VUJ5HjDZnkBJ2h
sHaRmNtcsl7XLf5jRXOx9CnQbxvESnD749pYS45QUMLeydNsBBdjFuCZBWezWqNt
JSVDjXC9NReTPnoWFHA0vUgFbpC2oJLfhX6SL8lAyomS/Ib5Ug7wAWOD7+1QmxDK
d6U92RQujhVOm/gOKZK1YYj1ARErW8tevv4PsK+9wrE2T7xhmZLMNGiAae0WJbfr
qY0AG4k4nDklhHSyvP5HDNEtGDgbZiKSAcRGJBoeJiq0TplGOmduMA==
=5h7y
-----END PGP SIGNATURE-----
--Bmpniq+Thlljzm46--