Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Aug 2011 12:35:15 +0200
From:      Hans Petter Selasky <hselasky@c2i.net>
To:        freebsd-arch@freebsd.org
Cc:        Andriy Gapon <avg@freebsd.org>
Subject:   Re: skipping locks, mutex_owned, usb
Message-ID:  <201108251235.15853.hselasky@c2i.net>
In-Reply-To: <201108230911.09021.jhb@freebsd.org>
References:  <4E53986B.5000804@FreeBSD.org> <201108230911.09021.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday 23 August 2011 15:11:08 John Baldwin wrote:
> > I.  [Why] do we need this pattern?
> > Can the code be re-written in a smarter (if not to say proper) way?
> 

Hi,

> Giant is recursive, it should just be always acquired.  Also, this
> recursive call pattern is very non-obvious.  A far more straightforward
> approach would be to just have:

Unless Witness is changed, that won't work. It will lead to LOR warnings I 
think.

Imagine that the root function locks Giant, then another lock is locked and 
then ukbd_poll() is called. Won't the second lock of Giant cause a LOR 
warning?

> 
> static int
> ukbd_poll(keyboard_t *kbd, int on)
> {
>         struct ukbd_softc *sc = kbd->kb_data;
> 
>         mtx_lock(&Giant);
>         if (on) {
>                 sc->sc_flags |= UKBD_FLAG_POLLING;
>                 sc->sc_poll_thread = curthread;
>         } else {
>                 sc->sc_flags &= ~UKBD_FLAG_POLLING;
>                 ukbd_start_timer(sc);   /* start timer */
>         }
>         mtx_unlock(&Giant);
>         return (0);
> }
> 
 
> Most code should not be abusing mtx_owned() in this fashion.  For Giant you
> should just follow a simple pattern like above that lets it recurse.  For
> your own locks you generally should use things like mtx_assert() to
> require all callers of a given routine to hold the lock rather than
> recursively acquiring the lock.  Very few legitimate cases of mtx_owned()
> exist IMO.  It is debatable if we should even have a mtx_owned() routine
> since we have mtx_assert().

How would you solve the LOR case?

--HPS



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108251235.15853.hselasky>