On 16/01/20(Thu) 11:20, Martin Pieuchot wrote:
> While discussing recent config_detach(9) vs close(2) race I figured
> out that the 'indirect' flag (`cd_indirect') is never set.
>
> Time to kill this feature?
>
> This survived build on x86, armv7, sgi, hppa, luna88k, alpha, landisk,
> sparc64.
>
> Ok?
Anyone?
> Index: sys/sys/device.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/device.h,v
> retrieving revision 1.55
> diff -u -p -r1.55 device.h
> --- sys/sys/device.h 10 Sep 2018 16:18:34 -0000 1.55
> +++ sys/sys/device.h 16 Jan 2020 08:07:23 -0000
> @@ -140,7 +140,6 @@ struct cfdriver {
> void **cd_devs; /* devices found */
> char *cd_name; /* device name */
> enum devclass cd_class; /* device classification */
> - int cd_indirect; /* indirectly configure subdevices */
> int cd_ndevs; /* size of cd_devs array */
> };
>
> Index: sys/kern/subr_autoconf.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/subr_autoconf.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 subr_autoconf.c
> --- sys/kern/subr_autoconf.c 30 Dec 2019 23:56:26 -0000 1.94
> +++ sys/kern/subr_autoconf.c 16 Jan 2020 09:43:17 -0000
> @@ -71,7 +71,7 @@ struct matchinfo {
> cfmatch_t fn;
> struct device *parent;
> void *match, *aux;
> - int indirect, pri;
> + int pri;
> };
>
> #ifndef AUTOCONF_VERBOSE
> @@ -128,10 +128,7 @@ mapply(struct matchinfo *m, struct cfdat
> int pri;
> void *match;
>
> - if (m->indirect)
> - match = config_make_softc(m->parent, cf);
> - else
> - match = cf;
> + match = cf;
>
> if (autoconf_verbose) {
> printf(">>> probing for %s", cf->cf_driver->cd_name);
> @@ -154,15 +151,8 @@ mapply(struct matchinfo *m, struct cfdat
> pri);
>
> if (pri > m->pri) {
> - if (m->indirect && m->match) {
> - cf = ((struct device *)m->match)->dv_cfdata;
> - free(m->match, M_DEVBUF, cf->cf_attach->ca_devsize);
> - }
> m->match = match;
> m->pri = pri;
> - } else {
> - if (m->indirect)
> - free(match, M_DEVBUF, cf->cf_attach->ca_devsize);
> }
> }
>
> @@ -188,7 +178,6 @@ config_search(cfmatch_t fn, struct devic
> m.parent = parent;
> m.match = NULL;
> m.aux = aux;
> - m.indirect = parent && parent->dv_cfdata->cf_driver->cd_indirect;
> m.pri = 0;
>
> for (cf = cfdata; cf->cf_driver; cf++) {
> @@ -209,10 +198,7 @@ config_search(cfmatch_t fn, struct devic
>
> if (autoconf_verbose) {
> if (m.match) {
> - if (m.indirect)
> - cf = ((struct device *)m.match)->dv_cfdata;
> - else
> - cf = (struct cfdata *)m.match;
> + cf = (struct cfdata *)m.match;
> printf(">>> %s probe won\n",
> cf->cf_driver->cd_name);
> } else
> @@ -235,9 +221,6 @@ config_scan(cfscan_t fn, struct device *
> struct cfdata *cf;
> short *p;
> void *match;
> - int indirect;
> -
> - indirect = parent && parent->dv_cfdata->cf_driver->cd_indirect;
>
> for (cf = cfdata; cf->cf_driver; cf++) {
> /*
> @@ -252,9 +235,7 @@ config_scan(cfscan_t fn, struct device *
> continue;
> for (p = cf->cf_parents; *p >= 0; p++)
> if (parent->dv_cfdata == &cfdata[*p]) {
> - match = indirect?
> - config_make_softc(parent, cf) :
> - (void *)cf;
> + match = cf;
> (*fn)(parent, match);
> }
> }
> @@ -275,7 +256,6 @@ config_rootsearch(cfmatch_t fn, char *ro
> m.parent = ROOT;
> m.match = NULL;
> m.aux = aux;
> - m.indirect = 0;
> m.pri = 0;
> /*
> * Look at root entries for matching name. We do not bother
> @@ -348,13 +328,8 @@ config_attach(struct device *parent, voi
> autoconf_attdet++;
> mtx_leave(&autoconf_attdet_mtx);
>
> - if (parent && parent->dv_cfdata->cf_driver->cd_indirect) {
> - dev = match;
> - cf = dev->dv_cfdata;
> - } else {
> - cf = match;
> - dev = config_make_softc(parent, cf);
> - }
> + cf = match;
> + dev = config_make_softc(parent, cf);
>
> cd = cf->cf_driver;
> ca = cf->cf_attach;
> Index: share/man/man9/autoconf.9
> ===================================================================
> RCS file: /cvs/src/share/man/man9/autoconf.9,v
> retrieving revision 1.17
> diff -u -p -r1.17 autoconf.9
> --- share/man/man9/autoconf.9 18 Apr 2018 22:33:18 -0000 1.17
> +++ share/man/man9/autoconf.9 16 Jan 2020 09:46:06 -0000
> @@ -66,12 +66,6 @@ The autoconfiguration framework supports
> .Em direct configuration
> where the bus driver can determine the devices present.
> .Pp
> -The autoconfiguration framework also supports
> -.Em indirect configuration
> -where the drivers must probe the bus looking for the presence of a device.
> -Direct configuration is preferred since it can find hardware regardless of
> -the presence of proper drivers.
> -.Pp
> The autoconfiguration process occurs at system bootstrap and is driven by a
> table generated from a
> .Do
> @@ -92,64 +86,6 @@ ends with a unit number.
> The unit number identifies an instance of the driver.
> Device data structures are allocated dynamically during autoconfiguration,
> giving a unique address for each instance.
> -.Ss Indirect Configuration
> -The
> -.Fn config_search
> -function performs indirect configuration of physical devices by iterating
> -over all potential children, calling the given function
> -.Fa func
> -for each one.
> -.Pp
> -The
> -.Fn config_rootsearch
> -function finds the root device identified by the string
> -.Fa rootname ,
> -in a manner similar to
> -.Fn config_search ,
> -except that there is no parent device.
> -If
> -.Fa func
> -is
> -.Dv NULL ,
> -.Fn config_search
> -applies each child's match function instead.
> -The argument
> -.Fa parent
> -is the pointer to the parent's device structure.
> -The given
> -.Fa aux
> -argument describes the device that has been found and is simply passed
> -on through
> -.Fa func
> -to the child.
> -.Fn config_search
> -returns a pointer to the best-matched child or
> -.Dv NULL
> -otherwise.
> -.Pp
> -The role of
> -.Fa func
> -is to call
> -the match function for each device and call
> -.Fn config_attach
> -for any positive matches.
> -.Bd -literal
> -typedef int (*cfmatch_t)(struct device *parent, void *child, void *aux);
> -.Ed
> -.Pp
> -If
> -.Fa func
> -is
> -.Dv NULL ,
> -then the parent should record the return value from
> -.Fn config_search
> -and call
> -.Fn config_attach
> -itself.
> -.Pp
> -Note that this function is designed so that it can be used to apply an
> -arbitrary function to all potential children.
> -In this case callers may choose to ignore the return value.
> .Ss Direct Configuration
> The
> .Fn config_found_sm
>
