Hello,
"Juergen Hannken-Illjes" <hann...@netbsd.org> wrote:
> Module Name: src
> Committed By: hannken
> Date: Tue Nov 23 09:30:43 UTC 2010
>
> Modified Files:
> src/sys/dev: md.c
>
> Log Message:
> Make md(4) mp-safe.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.64 -r1.65 src/sys/dev/md.c
Few comments:
> @@ -597,15 +626,18 @@ md_server_loop(struct md_softc *sc)
> int error;
> bool is_read;
>
> + KASSERT(mutex_owned(&sc->sc_lock));
> +
> for (;;) {
> /* Wait for some work to arrive. */
> while ((bp = bufq_get(sc->sc_buflist)) == NULL) {
> - error = tsleep((void *)sc, md_sleep_pri, "md_idle", 0);
> + error = cv_wait_sig(&sc->sc_cv, &sc->sc_lock);
> <...>
> biodone(bp);
> + mutex_enter(&sc->sc_lock);
> }
Is this (as well as other parts of code) are safe in respect of mdclose()?
For example, what happens if other thread executes close(2) while the lock
is dropped here?
> @@ -383,7 +396,8 @@ mdstrategy(struct buf *bp)
> case MD_UMEM_SERVER:
> /* Just add this job to the server's queue. */
> bufq_put(sc->sc_buflist, bp);
> - wakeup((void *)sc);
> + cv_signal(&sc->sc_cv);
> + mutex_exit(&sc->sc_lock);
It should be cv_broadcast(9).
> @@ -421,6 +435,8 @@ mdstrategy(struct buf *bp)
> }
> done:
> biodone(bp);
> +
> + mutex_exit(&sc->sc_lock);
Any reason why biodone() is under lock?
> @@ -534,6 +561,8 @@ md_ioctl_kalloc(struct md_softc *sc, str
> vaddr_t addr;
> vsize_t size;
>
> + KASSERT(mutex_owned(&sc->sc_lock));
Ideally, allocations should be outside the locks (just FYI).
> + kmutex_t sc_lock; /* Protect self. */
> + kcondvar_t sc_cv; /* Signal work. */
I think "Signal work" is missleading. :)
--
Mindaugas
