Re: Pseudo disk driver vs. mutex

[Mindaugas Rasiukevicius <rmind%NetBSD.org@localhost>]

Juergen Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost> wrote:
> while trying to update the fss pseudo disk driver from simple_lock to
> mutex I'm not sure what to do. Does this fragment look ok?
> 
> - IPL_NONE or IPL_SOFTBIO?
> - Any need for splbio() / slpx()?
> - biodone() inside the mutex or outside?

It would be good to have documented buffercache(9) locking in vfs_bio.c... :)

While biodone() might run in softint at IPL_SOFTBIO level, it seems that
driver does not synchronise with it, as buffer is dispatched to biodone().
Buffer-cache subsystem has internal locking (it locks bp->b_objlock), thus
biodone() should be outside your mutex. No need of spl()s, they are no longer
used for such purpose, if you want to make code MP-safe.

So from my poor understanding of buffercache(9), code looks reasonable.
But do not trust me ;)

>       mutex_enter(&sc->sc_lock);
>       work = 1;
>       for (;;) {
>               if (work == 0)
>                       cv_wait(&sc->sc_work, &sc->sc_lock);
>               work = 0;
>               if ((bp = BUFQ_GET(sc->sc_bufq)) == NULL)
>                       continue;
>               work++;
>               mutex_exit(&sc->sc_lock);
>               <fulfill request>
>               biodone(bp);
>               mutex_enter(&sc->sc_lock);
>               continue;
>       }
> }

        mutex_enter(&sc->sc_lock);
        for (;;) {
                bp = BUFQ_GET(sc->sc_bufq);
                if (bp != NULL) {
                        mutex_exit(&sc->sc_lock);
                        <fulfill request>
                        biodone(bp);
                        mutex_enter(&sc->sc_lock);
                } else {
                        cv_wait(&sc->sc_work, &sc->sc_lock);
                }
        }

How about this?

-- 
Best regards,
Mindaugas
www.NetBSD.org