Re: [PATCH 0/2] cfq-iosched: fixing RQ_NOIDLE handling.

[Corrado Zoccolo]

On Wed, Jul 7, 2010 at 10:06 PM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:
> Corrado Zoccolo <czoccolo@xxxxxxxxx> writes:
>
>> On Wed, Jul 7, 2010 at 7:03 PM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:
>>> Corrado Zoccolo <czoccolo@xxxxxxxxx> writes:
>>>
>>>> Hi Jens,
>>>> patch 8e55063 "cfq-iosched: fix corner cases in idling logic", is
>>>> suspected for some regressions on high end hardware.
>>>> The two patches from this series:
>>>> - [PATCH 1/2] cfq-iosched: fix tree-wide handling of rq_noidle
>>>> - [PATCH 2/2] cfq-iosched: RQ_NOIDLE enabled for SYNC_WORKLOAD
>>>> fix two issues that I have identified, related to how RQ_NOIDLE is
>>>> used by the upper layers.
>>>> First patch makes sure that a RQ_NOIDLE coming after a sequence of
>>>> possibly idling requests from the same queue on the no-idle tree will
>>>> clear the noidle_tree_requires_idle flag.
>>>> Second patch enables RQ_NOIDLE for queues in the idling tree,
>>>> restoring the behaviour pre-8e55063 patch.
>>>
>>> Hi, Corrado,
>>>
>>> I ran your kernel through my tests. ÂHere are the results, up against
>>> vanilla, deadline, and the blk_yield patch set:
>>>
>> Hi Jeff,
>> can you also add cfq with 8e55063 reverted to the testing mix?
>
> Sure, the results now look like this:
>
>         just  Âjust
>        Âfs_mark Âfio    Âmixed
> -------------------------------+--------------
> deadline    Â529.44  151.4 | 450.0  Â78.2
> vanilla cfq   107.88  164.4 |  6.6  137.2
> blk_yield cfq  530.82  158.7 | 113.2  Â78.6
> corrado cfq   110.16  220.6 |  7.0  159.8
> 8e55063 revert Â559.66 Â 198.9 | Â16.1 Â 153.3
>
> I had accidentally run your patch set (corrado cfq) on ext3, so the
> numbers were a bit off (everything else was run against ext4). ÂThe
> corrected numbers above reflect the performance on ext4, which is much
> better for the sequential reader, but still not great for the fs_mark
> run. ÂReverting 8e55063 definitely gets us into better shape. ÂHowever,
> if we care about the mixed workload, then it won't be enough.

Wondering why deadline performs so well in the fs_mark workload. Is it
because it doesn't distinguish between sync and async writes?
Maybe we can achieve something similar by putting all sync writes
(that are marked as REQ_NOIDLE) in the noidle tree? This, coupled with
making jbd(2) perform sync writes, should make the yield automatic,
since they all live in the same tree for which we don't idle between
queues, and should be able to provide fairness compared to a
sequential reader (that lives in the other tree).

Can you test the attached patch, where I also added your changes to
make jbd(2) to perform sync writes?

Thanks,
Corrado

>
> It's worth noting that I can't explain that jump from 151MB/s for
> deadline vs 220MB/s for corrado cfq. ÂI'm not sure how you can vary
> driving a single queue depth sequential read at all. ÂThose are the
> averages of 5 runs and this storage should be solely accessible by me,
> so I am at a loss.
>
> Cheers,
> Jeff
>



-- 
__________________________________________________________________________

dott. Corrado Zoccolo             mailto:czoccolo@xxxxxxxxx
PhD - Department of Computer Science - University of Pisa, Italy
--------------------------------------------------------------------------
The self-confidence of a warrior is not the self-confidence of the average
man. The average man seeks certainty in the eyes of the onlooker and calls
that self-confidence. The warrior seeks impeccability in his own eyes and
calls that humbleness.
              Â Tales of Power - C. Castaneda

Attachment: 0001-p.o.c.-fairness-between-seq-reader-and-sync-writers.patch
Description: Binary data