skipping locks, mutex_owned, usb

[Hans Petter Selasky]

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