On Thu, Aug 27, 2015 at 01:26:45PM +0200, Michael van Elst wrote:
> On Thu, Aug 27, 2015 at 11:48:15AM +0200, J. Hannken-Illjes wrote:
>
> > Looks racy: what if two threads run dk_strategy() -> dk_start() and
> > both get EAGAIN. Will it leak a buffer when the second thread tries
> > to save bp and dksc->sc_deferred already holds the buffer from the
> > first thread?
>
> Looks like it. sc_deferred probably needs to become a fcfs bufq.
> Currently this could happen for the ld driver (others are not MP_SAFE).
Maybe just this, a second thread is allowed to queue buffers,
but the loop is again single-threaded.
Of course that prevents multiple threads to call diskstart.
--- sys/dev/dksubr.c 27 Aug 2015 05:51:50 -0000 1.74
+++ sys/dev/dksubr.c 27 Aug 2015 17:25:03 -0000
@@ -301,6 +301,10 @@
if (bp != NULL)
bufq_put(dksc->sc_bufq, bp);
+ if (dksc->sc_busy)
+ goto done;
+ dksc->sc_busy = true;
+
/*
* Peeking at the buffer queue and committing the operation
* only after success isn't atomic.
@@ -339,6 +343,8 @@
bp = bufq_get(dksc->sc_bufq);
}
+ dksc->sc_busy = false;
+done:
mutex_exit(&dksc->sc_iolock);
}
--
Michael van Elst
Internet: mlel...@serpens.de
"A potential Snark may lurk in every tree."
