On Thu, Dec 10, 2009 at 01:50:28AM +, Mindaugas Rasiukevicius wrote:
> Hello,
>
> > Module Name:src
> > Committed By: dyoung
> > Date: Thu Nov 12 19:10:31 UTC 2009
> >
> > Modified Files:
> > src/sys/kern: subr_autoconf.c
> > src/sys/sys: device.h
> >
> > Log Message:
> > Move a device-deactivation pattern that is replicated throughout
> > the system into config_deactivate(dev): deactivate dev and all of
> > its descendants. Block all interrupts while calling each device's
> > activation hook, ca_activate. Now it is possible to simplify or
> > to delete several device-activation hooks throughout the system.
> >
> > ...
> >
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.186 -r1.187 src/sys/kern/subr_autoconf.c
> > cvs rdiff -u -r1.124 -r1.125 src/sys/sys/device.h
>
> - Can you tell what relevant code requires alldevs_mtx to be at IPL_HIGH?
>
> - Since alldevs_mtx became a spin-lock, it is no longer valid to assert for
> mutex _not_ being held. In other words, such cases are wrong:
>
> KASSERT(!mutex_owned(&alldevs_mtx));
Ah! Thanks.
> - You have added config_collect_garbage(), which is mostly called before
> config_alldevs_lock(). How about changing it to be used as/with last
> unlock? That is, collect the objects into a list, release the lock and
> then destroy all objects. Apart from avoiding unecessary unlock/relock
> dances, it would also be simpler.
Thank you for the suggestion. To make the change is easy. I have
attached a patch.
> - May I suggest to avoid "inverted" logic like this:
>
> if (rv != 0)
> ;
> else if (dev->dv_del_gen != 0)
> ;
> else {
> dev->dv_del_gen = alldevs_gen;
> alldevs_garbage = true;
> }
Thanks. I will change this to the short form.
Dave
--
David Young OJC Technologies
dyo...@ojctech.com Urbana, IL * (217) 278-3933
Index: sys/sys/device.h
===
RCS file: /cvsroot/src/sys/sys/device.h,v
retrieving revision 1.127
diff -u -p -u -p -r1.127 device.h
--- sys/sys/device.h7 Dec 2009 19:45:13 - 1.127
+++ sys/sys/device.h10 Dec 2009 18:17:54 -
@@ -181,6 +181,10 @@ struct device {
device_suspensor_t dv_bus_suspensors[DEVICE_SUSPENSORS_MAX];
device_suspensor_t dv_driver_suspensors[DEVICE_SUSPENSORS_MAX];
device_suspensor_t dv_class_suspensors[DEVICE_SUSPENSORS_MAX];
+ struct device_garbage {
+ device_t*dg_devs;
+ int dg_ndevs;
+ } dv_garbage;
};
/* dv_flags */
Index: sys/kern/subr_autoconf.c
===
RCS file: /cvsroot/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.188
diff -u -p -u -p -r1.188 subr_autoconf.c
--- sys/kern/subr_autoconf.c12 Nov 2009 23:16:28 - 1.188
+++ sys/kern/subr_autoconf.c10 Dec 2009 18:17:54 -
@@ -166,12 +166,14 @@ static char *number(char *, int);
static void mapply(struct matchinfo *, cfdata_t);
static device_t config_devalloc(const device_t, const cfdata_t, const int *);
static void config_devdelete(device_t);
+static void config_devunlink(device_t, struct devicelist *);
static void config_makeroom(int, struct cfdriver *);
static void config_devlink(device_t);
static void config_alldevs_unlock(int);
static int config_alldevs_lock(void);
-static void config_collect_garbage(void);
+static void config_collect_garbage(struct devicelist *);
+static void config_dump_garbage(struct devicelist *);
static void pmflock_debug(device_t, const char *, int);
@@ -368,10 +370,11 @@ config_cfdriver_attach(struct cfdriver *
int
config_cfdriver_detach(struct cfdriver *cd)
{
+ struct devicelist garbage = TAILQ_HEAD_INITIALIZER(garbage);
int i, rc = 0, s;
s = config_alldevs_lock();
- config_collect_garbage();
+ config_collect_garbage(&garbage);
/* Make sure there are no active instances. */
for (i = 0; i < cd->cd_ndevs; i++) {
if (cd->cd_devs[i] != NULL) {
@@ -380,6 +383,7 @@ config_cfdriver_detach(struct cfdriver *
}
}
config_alldevs_unlock(s);
+ config_dump_garbage(&garbage);
if (rc != 0)
return rc;
@@ -441,6 +445,7 @@ config_cfattach_attach(const char *drive
int
config_cfattach_detach(const char *driver, struct cfattach *ca)
{
+ struct devicelist garbage = TAILQ_HEAD_INITIALIZER(garbage);
struct cfdriver *cd;
device_t dev;
int i, rc = 0, s;
@@ -450,7 +455,7 @@ config_cfattach_detach(const char *drive
return ESRCH;
s = config_alldevs_lock();
- config_collect_garbage();
+ config_collect_garbage(&garbage);
/* Make sure there are no active instances. */
for (i = 0; i < cd->cd_ndevs; i++) {
