OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
Re: bufq massage

From: Mark Kettenis (mark.kettenisxs4all.nl)
Date: Wed Sep 01 2010 - 05:13:25 CDT


> From: David Gwynne <lokianimata.net>
> Date: Wed, 1 Sep 2010 19:35:15 +1000
>
> On 01/09/2010, at 7:30 PM, Mark Kettenis wrote:
> > I realise you committed this already, but can you elaborate on those
> > XXX's in there?
>
> these two?

yes

> + if (bq->bufq_outstanding == 0 /* XXX and quiesced */)
> + SLIST_FOREACH(bq, &bufqs, bufq_entries) { /* XXX */
>
> the first one refers to the fact that we call wakeup when we run out
> of io on the device, as opposed to when we run out of io on the
> device when we're waiting for the device to quiesce.

Ah, that's a possible optimization. Could be worthwhile, since
wakeup(9) walks a list. I think

        if (bq->bufq_stop && bq->bufq_outstanding == 0)

should do the trick. I'll look at that again tonight.

> the second is there cos the SLIST is handled without taking the
> global bufqs_mtx, which every other user of the SLIST takes before
> mucking around with it.

That is completely intential, and actually essential for the quiescing
to work. The bufqs_mtx doesn't really protect the bufqs list, but
rather the bufqs_stop variable. The bufqs_stop variable protects the
list from being modified as soon as it is set to nonzero.

I agree that this isn't entirely intuitive. It took us a couple of
iterations to fix it during c2k10 and afterwards. I'll replace the
XXX with a comment explaining what's going on.