Date: Mon, 11 Oct 2004 17:30:43 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: damien.bergamini@free.fr Cc: freebsd-hackers@freebsd.org Subject: Re: msleep(9) and recursed mutex Message-ID: <20041011173042.B744@curly.tele2.no> In-Reply-To: <1097500245.416a86556c692@imp1-q.free.fr>; from damien.bergamini@free.fr on Mon, Oct 11, 2004 at 03:10:45PM %2B0200 References: <1097500245.416a86556c692@imp1-q.free.fr>
next in thread | previous in thread | raw e-mail | index | archive | help
--gBBFr7Ir9EOA20Yy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Mon, Oct 11, 2004 at 03:10:45PM +0200, damien.bergamini@free.fr wrote:
> msleep(9) behaves strangely with recursed mutexes.
> In the attached code, calling foo_f() will make the kernel hang for
> two seconds.
> It seems like msleep does not release the mtx mutex completely but
> simply decrement its reference count by one. When calling msleep, the
> mtx mutex is still held which prevent the interrupt from happening.
> msleep will then return error code EWOULDBLOCK after two seconds.
> If I remove calls to mtx_lock/unlock from function foo_g(), it works
> without problem but this is not a solution since foo_g() can be
> called outside of function foo_f().
> Of course, the mtx mutex was created with the MTX_RECURSE flag set.
>
> Is it an expected behaviour? In msleep(9) it is said:
> "The msleep() function is a variation on tsleep. The parameter mtx
> is a mutex which will be released before sleeping and reacquired
> before msleep() returns."
>
> Seems like the mutex is not *really* released if it recurses.
>
Only the "Giant" mutex is released if it recurses. By the way there is
a bug in msleep:
mtx_lock(&Giant);
msleep(xxx, &Giant, 0, "foo", 0);
mtx_unlock(&Giant);
doesn't work. See attached patch.
>
> Thanks,
> Damien Bergamini
>
> ----
>
> void foo_intr(void *arg)
> {
> struct foo_softc *sc = arg;
>
> mtx_lock(&sc->mtx);
> ...
> wakeup(sc);
> ...
> mtx_unlock(&sc->mtx);
> }
>
> void foo_f(struct foo_softc *sc)
> {
> mtx_lock(&sc->mtx);
> ...
> foo_g(sc);
> ...
> mtx_unlock(&sc->mtx);
> }
>
> void foo_g(struct foo_softc *sc)
> {
> /* mtx will recurse if called from foo_f() */
> mtx_lock(&sc->mtx);
> ...
> /* do something that will make hardware raise an interrupt */
> ...
> /* wait at most 2 second for the interrupt */
> msleep(sc, &sc->mtx, 0, "foo", 2 * hz);
> ...
> mtx_unlock(&sc->mtx);
> }
-HPS
--gBBFr7Ir9EOA20Yy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="kern_synch.c.diff"
*** src/sys/kern/kern_synch.c.ref Wed Sep 29 14:21:09 2004
--- src/sys/kern/kern_synch.c Wed Sep 29 14:22:25 2004
***************
*** 182,193 ****
CTR5(KTR_PROC, "msleep: thread %p (pid %ld, %s) on %s (%p)",
(void *)td, (long)p->p_pid, p->p_comm, wmesg, ident);
- DROP_GIANT();
if (mtx != NULL) {
mtx_assert(mtx, MA_OWNED | MA_NOTRECURSED);
WITNESS_SAVE(&mtx->mtx_object, mtx);
mtx_unlock(mtx);
}
/*
* We put ourselves on the sleep queue and start our timeout
--- 182,194 ----
CTR5(KTR_PROC, "msleep: thread %p (pid %ld, %s) on %s (%p)",
(void *)td, (long)p->p_pid, p->p_comm, wmesg, ident);
if (mtx != NULL) {
mtx_assert(mtx, MA_OWNED | MA_NOTRECURSED);
WITNESS_SAVE(&mtx->mtx_object, mtx);
mtx_unlock(mtx);
}
+ /* drop Giant after mtx in case mtx == &Giant */
+ DROP_GIANT();
/*
* We put ourselves on the sleep queue and start our timeout
--gBBFr7Ir9EOA20Yy--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041011173042.B744>