mutex quandry

Brad Huntting huntting at glarp.com
Mon Nov 24 08:19:35 PST 2008


Dear hackers:

My device driver, like many, maintains a counter (sc_inuse) and a flag
(sc_running) to keep track of how many 'users' are using the device
instance variables (softc) and wheather the device is trying to
detach.  I call the functions scm_reserver() and scm_leave() (see
below) before and after any block of code that uses the softc.  Then,
in my detach() function I clear the sc_running flag and msleep()
waiting for the sc_inuse counter to drain to zero before continuing,
eventually destroying the very mutex I use to protect these two
variables.  My problem is here:

        if (sc && mtx_initialized(&sc->sc_inuse_mtx)) {
                mtx_lock(&sc->sc_inuse_mtx);
                ...

Unfortunately,  { mtx_initialized(x) && (mtx_lock(x), 1); } is not
atomic.  In the unlikely (but possible) event that the lock is
destroyed after mtx_initialized(x) and before mtx_lock(x), the system
will behave badly (probably panic).

It would seem to be a common problem for most device drivers to
implement this 'open/closing' lock where the lock starts out
zero-filled in the pre-open state and destroyed in the post-closed
state. So how should one deal with this?  I could invent my own
locking with atomic_*() operations, but then I loose the advantages of
using standard system locks (like witness).

thanx in advance,
brad

/*
 * scm_reserve() increments a inuse and scm_release() keep track
 * of how many customers are on site.  To shut down we close the
 * door (sc_running=0), but we dont actually turn out the lights
 * untill the last customer has left (sc_inuse==0).
 *
 * If device is configured, this returns TRUE and caller must call
 * scm_release() when done.  If device is not configured returns
 * FALSE and no further action is required.
 */
static int
scm_reserve(struct scmicro_softc *sc)
{
        int r;

        if (sc && mtx_initialized(&sc->sc_inuse_mtx)) {
                mtx_lock(&sc->sc_inuse_mtx);
                KASSERT(sc->sc_inuse >= 0, "scmicro: scm_reserve()
sc_inuse <0!\n");
                r = ( sc->sc_running ? ++sc->sc_inuse : 0 );
                mtx_unlock(&sc->sc_inuse_mtx);
        } else {
                r = 0;
                DPRINTF(("scmicro: scm_reserve() failed to reserve
sc=%p!\n", sc));
        }

        return (r);
}

/* call this iff scm_reserve() returns true. */
static inline void
scm_release(struct scmicro_softc *sc)
{

        mtx_lock(&sc->sc_inuse_mtx);
        if (--sc->sc_inuse == 0)
                wakeup(&sc->sc_inuse);
        KASSERT(sc->sc_inuse >= 0, "scmicro: scm_release() sc_inuse <0!\n");
        mtx_unlock(&sc->sc_inuse_mtx);
}

static int
scmicro_detach(device_t self)
{
                /*....*/
                mtx_lock(&sc->sc_inuse_mtx);
                sc->sc_running = 0; /* redundant */
                if (sc->sc_inuse > 0)
                        msleep_spin((void *)&sc->sc_inuse, &sc->sc_inuse_mtx,
                            "scmicro detach", 0);
                mtx_unlock(&sc->sc_inuse_mtx);
                /*....*/
                mtx_destroy(&sc->sc_inuse_mtx);
                /*....*/
}


More information about the freebsd-drivers mailing list