msleep() on recursivly locked mutexes

Attilio Rao attilio at freebsd.org
Thu Apr 26 21:38:56 UTC 2007


2007/4/26, Hans Petter Selasky <hselasky at c2i.net>:
> Hi,
>
> In the new USB stack I have defined the following:
>
> u_int32_t
> mtx_drop_recurse(struct mtx *mtx)
> {
>        u_int32_t recurse_level = mtx->mtx_recurse;
>        u_int32_t recurse_curr = recurse_level;
>
>        mtx_assert(mtx, MA_OWNED);
>
>        while(recurse_curr--) {
>            mtx_unlock(mtx);
>        }
>
>        return recurse_level;
> }
>
> void
> mtx_pickup_recurse(struct mtx *mtx, u_int32_t recurse_level)
> {
>        mtx_assert(mtx, MA_OWNED);
>
>        while(recurse_level--) {
>            mtx_lock(mtx);
>        }
>        return;
> }
>
> When I do a msleep() I do it like this:
>
>        level = mtx_drop_recurse(ctd->p_mtx);
>
>        error = msleep(ctd, ctd->p_mtx, 0,
>                       "config td sleep", timeout);
>
>        mtx_pickup_recurse(ctd->p_mtx, level);
>
> Are there any comments on integrating this functionality into msleep(), and
> adding mtx_drop_recurse() and mtx_pickup_recurse() to the FreeBSD kernel?

Several times I thought to adding recursed mutex handling in msleep &
condvar, but in the end the better approach is mantaining the current
behaviour.

Recursed mutexes often are results of incorrect locking strategies and
catering that kind of errors is necessarily a bad idea.
It is a lot better handling potential recursing mutexes outside from
sleeping points (that is what your implementation does).

Just, please, don't deal with internal struct mtx fields for that (so,
please, don't refer to mtx->mtx_recurse) just use the mutex(9) API
provided.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein


More information about the freebsd-hackers mailing list