Re: CVS commit: src/sys

[Michael van Elst <mlelstv%serpens.de@localhost>]

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: mlelstv%serpens.de@localhost
                                "A potential Snark may lurk in every tree."