Date: Mon, 24 Nov 2008 09:19:33 -0700 From: "Brad Huntting" <huntting@glarp.com> To: freebsd-drivers@freebsd.org Subject: mutex quandry Message-ID: <fd20970d0811240819n510846ffw123c65407e58cfc2@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
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);
/*....*/
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?fd20970d0811240819n510846ffw123c65407e58cfc2>