Re: trying to understand READ_META, READ_SYNC, WRITE_SYNC & co

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, Jun 29, 2010 at 11:06:19AM +0200, Corrado Zoccolo wrote:

[..]
> > I'm now testing OCFS2, and I'm seeing performance that is not great
> > (even with the blk_yield patches applied).  What happens is that we
> > successfully yield the queue to the journal thread, but then idle on the
> > journal thread (even though RQ_NOIDLE was set).
> >
> > So, can we just get rid of idling when RQ_NOIDLE is set?
> Hi Jeff,
> I think I spotted a problem with the initial implementation of the
> tree-wide idle when RQ_NOIDLE is set: I assumed that a queue would
> either send possibly-idling requests or no-idle requests, but it seems
> that RQ_NOIDLE is being used to mark the end of a stream of
> possibly-idling requests (in my initial implementation, this will then
> cause an unintended idle). The attached patch should fix it, and I
> think the logic is simpler than Vivek's. Can you give it a spin?
> Otherwise, I think that reverting the "noidle_tree_requires_idle"
> behaviour completely may be better than adding complexity, since it is
> really trying to solve corner cases (that maybe happen only on
> synthetic workloads), but affecting negatively more common cases.
> 

Hi Corrado,

I think you forgot to attach the patch? Can't find it.


> About what it is trying to solve, since I think it was not clear:
> - we have a workload of 2 queues, both issuing requests that are being
> put in the no-idle tree (e.g. they are random) + 1 queue issuing
> idling requests (e.g. sequential).
> - if one of the 2 "random" queues marks its requests as RQ_NOIDLE,
> then the timeslice for the no-idle tree is not preserved, causing
> unfairness, as soon as an RQ_NOIDLE request is serviced and the tree
> is empty.

I think Jeff's primary regressions were coming from the fact that we
will continue to idle on SYNC_WORKLOAD even if RQ_NOIDLE() was set.

Regarding giving up idling on sync-noidle workload, I think it still
makes some sense to keep track if some other random queue is doing IO on
that tree or not and if yes, then continue to idle. That's a different
thing that current logic if more coarse and could be fine grained a bit.

Because I don't have a practical workload example at this point of time, I
also don't mind reverting your old patch and restoring the policy of not
idling if RQ_NOIDLE() was set.

But it still does not answer the question that why O_DIRECT and O_SYNC
paths be different when it comes to RQ_NOIDLE.

Thanks
Vivek