Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys Move a device-deactivation pattern that is replicated th...



details:   https://anonhg.NetBSD.org/src/rev/e1f117f4104b
branches:  trunk
changeset: 748979:e1f117f4104b
user:      dyoung <dyoung%NetBSD.org@localhost>
date:      Thu Nov 12 19:10:30 2009 +0000

description:
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.

Do not deactivate a driver while detaching it!  If the driver was
already deactivated (because of accidental/emergency removal), let
the driver cope with the knowledge that DVF_ACTIVE has been cleared.
Otherwise, let the driver access the underlying hardware (so that
it can flush caches, restore original register settings, et cetera)
until it exits its device-detachment hook.

Let multiple readers and writers simultaneously access the system's
device_t list, alldevs, from either interrupt or thread context:
postpone changing alldevs linkages and freeing autoconf device
structures until a garbage-collection phase that runs after all
readers & writers have left the list.

Give device iterators (deviter(9)) a consistent view of alldevs no
matter whether device_t's are added and deleted during iteration:
keep a global alldevs generation number.  When an iterator enters
alldevs, record the current generation number in the iterator and
increase the global number.  When a device_t is created, label it
with the current global generation number.  When a device_t is
deleted, add a second label, the current global generation number.
During iteration, compare a device_t's added- and deleted-generation
with the iterator's generation and skip a device_t that was deleted
before the iterator entered the list or added after the iterator
entered the list.

The alldevs generation number is never 0.  The garbage collector
reaps device_t's whose delete-generation number is non-zero.

Make alldevs private to sys/kern/subr_autoconf.c.  Use deviter(9)
to access it.

diffstat:

 sys/kern/subr_autoconf.c |  498 ++++++++++++++++++++++++++++++----------------
 sys/sys/device.h         |    7 +-
 2 files changed, 329 insertions(+), 176 deletions(-)

diffs (truncated from 814 to 300 lines):

diff -r fbd6298c5083 -r e1f117f4104b sys/kern/subr_autoconf.c
--- a/sys/kern/subr_autoconf.c  Thu Nov 12 18:37:10 2009 +0000
+++ b/sys/kern/subr_autoconf.c  Thu Nov 12 19:10:30 2009 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_autoconf.c,v 1.186 2009/10/12 23:33:02 yamt Exp $ */
+/* $NetBSD: subr_autoconf.c,v 1.187 2009/11/12 19:10:30 dyoung Exp $ */
 
 /*
  * Copyright (c) 1996, 2000 Christopher G. Demetriou
@@ -77,7 +77,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.186 2009/10/12 23:33:02 yamt Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.187 2009/11/12 19:10:30 dyoung Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -165,10 +165,13 @@
 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_devdealloc(device_t);
+static void config_devdelete(device_t);
 static void config_makeroom(int, struct cfdriver *);
 static void config_devlink(device_t);
-static void config_devunlink(device_t);
+static void config_alldevs_unlock(int);
+static int config_alldevs_lock(void);
+
+static void config_collect_garbage(void);
 
 static void pmflock_debug(device_t, const char *, int);
 
@@ -202,12 +205,12 @@
 static int config_finalize_done;
 
 /* list of all devices */
-struct devicelist alldevs = TAILQ_HEAD_INITIALIZER(alldevs);
-kcondvar_t alldevs_cv;
-kmutex_t alldevs_mtx;
-static int alldevs_nread = 0;
-static int alldevs_nwrite = 0;
-static lwp_t *alldevs_writer = NULL;
+static struct devicelist alldevs = TAILQ_HEAD_INITIALIZER(alldevs);
+static kmutex_t alldevs_mtx;
+static volatile bool alldevs_garbage = false;
+static volatile devgen_t alldevs_gen = 1;
+static volatile int alldevs_nread = 0;
+static volatile int alldevs_nwrite = 0;
 
 static int config_pending;             /* semaphore for mountroot */
 static kmutex_t config_misc_lock;
@@ -238,8 +241,7 @@
 
        KASSERT(config_initialized == false);
 
-       mutex_init(&alldevs_mtx, MUTEX_DEFAULT, IPL_NONE);
-       cv_init(&alldevs_cv, "alldevs");
+       mutex_init(&alldevs_mtx, MUTEX_DEFAULT, IPL_HIGH);
 
        mutex_init(&config_misc_lock, MUTEX_DEFAULT, IPL_NONE);
        cv_init(&config_misc_cv, "cfgmisc");
@@ -366,13 +368,21 @@
 int
 config_cfdriver_detach(struct cfdriver *cd)
 {
-       int i;
-
+       int i, rc = 0, s;
+
+       s = config_alldevs_lock();
+       config_collect_garbage();
        /* Make sure there are no active instances. */
        for (i = 0; i < cd->cd_ndevs; i++) {
-               if (cd->cd_devs[i] != NULL)
-                       return EBUSY;
+               if (cd->cd_devs[i] != NULL) {
+                       rc = EBUSY;
+                       break;
+               }
        }
+       config_alldevs_unlock(s);
+
+       if (rc != 0)
+               return rc;
 
        /* ...and no attachments loaded. */
        if (LIST_EMPTY(&cd->cd_attach) == 0)
@@ -433,19 +443,27 @@
 {
        struct cfdriver *cd;
        device_t dev;
-       int i;
+       int i, rc = 0, s;
 
        cd = config_cfdriver_lookup(driver);
        if (cd == NULL)
                return ESRCH;
 
+       s = config_alldevs_lock();
+       config_collect_garbage();
        /* Make sure there are no active instances. */
        for (i = 0; i < cd->cd_ndevs; i++) {
                if ((dev = cd->cd_devs[i]) == NULL)
                        continue;
-               if (dev->dv_cfattach == ca)
-                       return EBUSY;
+               if (dev->dv_cfattach == ca) {
+                       rc = EBUSY;
+                       break;
+               }
        }
+       config_alldevs_unlock(s);
+
+       if (rc != 0)
+               return rc;
 
        LIST_REMOVE(ca, ca_list);
 
@@ -923,6 +941,11 @@
 
 /*
  * Expand the size of the cd_devs array if necessary.
+ *
+ * The caller must hold alldevs_mtx. config_makeroom() may release and
+ * re-acquire alldevs_mtx, so callers should re-check conditions such
+ * as alldevs_nwrite == 0 and alldevs_nread == 0 when config_makeroom()
+ * returns.
  */
 static void
 config_makeroom(int n, struct cfdriver *cd)
@@ -930,8 +953,11 @@
        int old, new;
        device_t *nsp;
 
+       alldevs_nwrite++;
+       mutex_exit(&alldevs_mtx);
+
        if (n < cd->cd_ndevs)
-               return;
+               goto out;
 
        /*
         * Need to expand the array.
@@ -943,7 +969,6 @@
                new = old * 2;
        while (new <= n)
                new *= 2;
-       cd->cd_ndevs = new;
        nsp = kmem_alloc(sizeof(device_t [new]), KM_SLEEP);
        if (nsp == NULL)
                panic("config_attach: %sing dev array",
@@ -953,37 +978,50 @@
                memcpy(nsp, cd->cd_devs, sizeof(device_t [old]));
                kmem_free(cd->cd_devs, sizeof(device_t [old]));
        }
+       cd->cd_ndevs = new;
        cd->cd_devs = nsp;
+out:
+       mutex_enter(&alldevs_mtx);
+       alldevs_nwrite--;
 }
 
 static void
 config_devlink(device_t dev)
 {
        struct cfdriver *cd = dev->dv_cfdriver;
+       int s;
 
        /* put this device in the devices array */
+       s = config_alldevs_lock();
        config_makeroom(dev->dv_unit, cd);
        if (cd->cd_devs[dev->dv_unit])
                panic("config_attach: duplicate %s", device_xname(dev));
        cd->cd_devs[dev->dv_unit] = dev;
 
        /* It is safe to add a device to the tail of the list while
-        * readers are in the list, but not while a writer is in
-        * the list.  Wait for any writer to complete.
+        * readers and writers are in the list.
         */
-       mutex_enter(&alldevs_mtx);
-       while (alldevs_nwrite != 0 && alldevs_writer != curlwp)
-               cv_wait(&alldevs_cv, &alldevs_mtx);
+       dev->dv_add_gen = alldevs_gen;
        TAILQ_INSERT_TAIL(&alldevs, dev, dv_list);      /* link up */
-       cv_signal(&alldevs_cv);
-       mutex_exit(&alldevs_mtx);
+       config_alldevs_unlock(s);
 }
 
+/*
+ * Caller must hold alldevs_mtx. config_devdelete() may release and
+ * re-acquire alldevs_mtx, so callers should re-check conditions such as
+ * alldevs_nwrite == 0 && alldevs_nread == 0 after config_devdelete()
+ * returns.
+ */
 static void
-config_devunlink(device_t dev)
+config_devdelete(device_t dev)
 {
+       device_lock_t dvl = device_getlock(dev);
+       int priv = (dev->dv_flags & DVF_PRIV_ALLOC);
        struct cfdriver *cd = dev->dv_cfdriver;
-       int i;
+       device_t *devs = NULL;
+       int i, ndevs = 0;
+
+       KASSERT(mutex_owned(&alldevs_mtx));
 
        /* Unlink from device list. */
        TAILQ_REMOVE(&alldevs, dev, dv_list);
@@ -996,14 +1034,47 @@
         */
        for (i = 0; i < cd->cd_ndevs; i++) {
                if (cd->cd_devs[i] != NULL)
-                       return;
+                       break;
+       }
+       /* nothing found.  unlink, now.  deallocate below. */
+       if (i == cd->cd_ndevs) {
+               ndevs = cd->cd_ndevs;
+               devs = cd->cd_devs;
+               cd->cd_devs = NULL;
+               cd->cd_ndevs = 0;
        }
-       /* nothing found; deallocate */
-       kmem_free(cd->cd_devs, sizeof(device_t [cd->cd_ndevs]));
-       cd->cd_devs = NULL;
-       cd->cd_ndevs = 0;
+
+       mutex_exit(&alldevs_mtx);
+
+       KASSERT(!mutex_owned(&alldevs_mtx));
+
+       if (devs != NULL)
+               kmem_free(devs, sizeof(device_t [ndevs]));
+
+       cv_destroy(&dvl->dvl_cv);
+       mutex_destroy(&dvl->dvl_mtx);
+
+       KASSERT(dev->dv_properties != NULL);
+       prop_object_release(dev->dv_properties);
+
+       if (dev->dv_activity_handlers)
+               panic("%s with registered handlers", __func__);
+
+       if (dev->dv_locators) {
+               size_t amount = *--dev->dv_locators;
+               kmem_free(dev->dv_locators, amount);
+       }
+
+       if (dev->dv_cfattach->ca_devsize > 0)
+               kmem_free(dev->dv_private, dev->dv_cfattach->ca_devsize);
+       if (priv)
+               kmem_free(dev, sizeof(*dev));
+
+       KASSERT(!mutex_owned(&alldevs_mtx));
+
+       mutex_enter(&alldevs_mtx);
 }
-       
+
 static device_t
 config_devalloc(const device_t parent, const cfdata_t cf, const int *locs)
 {
@@ -1011,7 +1082,7 @@
        struct cfattach *ca;
        size_t lname, lunit;
        const char *xunit;
-       int myunit;
+       int myunit, s;
        char num[10];
        device_t dev;
        void *dev_private;
@@ -1031,6 +1102,8 @@
                panic("config_devalloc: %s", cf->cf_atname);
 
 #ifndef __BROKEN_CONFIG_UNIT_USAGE
+       s = config_alldevs_lock();
+       config_collect_garbage();
        if (cf->cf_fstate == FSTATE_STAR) {
                for (myunit = cf->cf_unit; myunit < cd->cd_ndevs; myunit++)
                        if (cd->cd_devs[myunit] == NULL)
@@ -1042,8 +1115,11 @@
        } else {
                myunit = cf->cf_unit;
                if (myunit < cd->cd_ndevs && cd->cd_devs[myunit] != NULL)
-                       return NULL;
-       }       
+                       myunit = -1;
+       }
+       config_alldevs_unlock(s);
+       if (myunit == -1)
+               return NULL;
 #else
        myunit = cf->cf_unit;
 #endif /* ! __BROKEN_CONFIG_UNIT_USAGE */
@@ -1116,32 +1192,6 @@



Home | Main Index | Thread Index | Old Index