Re: CVS commit: src/sys

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

On 27 Aug 2015, at 13:26, Michael van Elst <mlelstv%serpens.de@localhost> wrote:

> On Thu, Aug 27, 2015 at 11:48:15AM +0200, J. Hannken-Illjes wrote:
> 
>>                disk_busy(&dksc->sc_dkdev);
>> +               mutex_exit(&dksc->sc_iolock);
>>                error = dkd->d_diskstart(dksc->sc_dev, bp);
>> +               mutex_enter(&dksc->sc_iolock);
>>                if (error == EAGAIN) {
>> +                       dksc->sc_deferred = bp;
>>                        disk_unbusy(&dksc->sc_dkdev, 0, (bp->b_flags & B_READ));
>>                        break;
>>                }
>> 
>> 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).
> 
> It would be simpler if you could unget something into the original
> bufq.

This was the intention with "bufq_peek".  All current bufq strategies
seem to keep the current buf the same over "bufq_peek .. bufq_get"
even if more work comes in via "bufq_put".

I'm quite sure most of our floppy drivers work this way.

It is just callers of "bufq_drain" that have to protect against
"bufq_pee .. bufq_get" sequences.

--
J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig (Germany)