Date: Mon, 24 Nov 2008 10:38:16 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: huntting@glarp.com Cc: freebsd-drivers@freebsd.org Subject: Re: mutex quandry Message-ID: <20081124.103816.1649771647.imp@bsdimp.com> In-Reply-To: <fd20970d0811240819n510846ffw123c65407e58cfc2@mail.gmail.com> References: <fd20970d0811240819n510846ffw123c65407e58cfc2@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <fd20970d0811240819n510846ffw123c65407e58cfc2@mail.gmail.com>
"Brad Huntting" <huntting@glarp.com> writes:
: 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).
Why do you care that it is initialized? This mutex should be created
in the attach routine, and nothing can race it there unless it is
creating threads, registering interrupts, etc before initializing it.
: 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).
Right. I guess I still don't understand why you need to check to see
if it is initialized. You should never be destroying the mutex if
there are other threads accessing it.
I guess I'm having problems understanding why you are even doing the
non-atomic test, and why it is needed. Once you remove the test, the
code falls into standard norms.
Warner
: 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);
: /*....*/
: }
: _______________________________________________
: freebsd-drivers@freebsd.org mailing list
: http://lists.freebsd.org/mailman/listinfo/freebsd-drivers
: To unsubscribe, send any mail to "freebsd-drivers-unsubscribe@freebsd.org"
:
:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081124.103816.1649771647.imp>