From: Corrado Zoccolo on
Fixed Jens' mail address, and resending the patches based on for-2.6.36 tree.
On Wed, Jul 7, 2010 at 5:22 PM, Corrado Zoccolo <czoccolo(a)gmail.com> wrote:
> 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.
>
> An other option to consider is the partial revert of 8e55063, if the
> corner cases we are trying to handle are not frequent enough to
> justify this added complexity.
>
> Thanks,
> Corrado
>



--
__________________________________________________________________________

dott. Corrado Zoccolo mailto:czoccolo(a)gmail.com
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeff Moyer on
Corrado Zoccolo <czoccolo(a)gmail.com> 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:

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 80.82 138.1 | 4.5 130.7

fs_mark results are in files/second, fio results are in MB/s. All
results are the average of 5 runs. In order to get results for the
mixed workload for both vanilla and Corrado's kernels, I had to extend
the runtime from 30s to 300s.

So, the changes proposed in this thread actually make performance worse
across the board.

I re-ran my tests against a RHEL 5 kernel (which is based on 2.6.18),
and it shows that fs_mark performance is much better than stock CFQ in
2.6.35-rc3, and the mixed workload results are much the same as they are
now (which is to say, the fs_mark process is completely starved by the
sequential reader). So, that problem has existed for a long time.

I'm still in the process of collecting data from production servers and
will report back with my findings there.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Corrado Zoccolo on
On Wed, Jul 7, 2010 at 7:03 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote:
> Corrado Zoccolo <czoccolo(a)gmail.com> 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?

>                 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      80.82   138.1 |   4.5   130.7

So it doesn't seem to help. I wonder if other parts of that commit are
affecting those workloads.

>
> fs_mark results are in files/second, fio results are in MB/s.  All
> results are the average of 5 runs.  In order to get results for the
> mixed workload for both vanilla and Corrado's kernels, I had to extend
> the runtime from 30s to 300s.
>
> So, the changes proposed in this thread actually make performance worse
> across the board.
>
> I re-ran my tests against a RHEL 5 kernel (which is based on 2.6.18),
> and it shows that fs_mark performance is much better than stock CFQ in
> 2.6.35-rc3, and the mixed workload results are much the same as they are
> now (which is to say, the fs_mark process is completely starved by the
> sequential reader).  So, that problem has existed for a long time.
>
> I'm still in the process of collecting data from production servers and
> will report back with my findings there.

Thanks,
Corrado

>
> Cheers,
> Jeff
>



--
__________________________________________________________________________

dott. Corrado Zoccolo mailto:czoccolo(a)gmail.com
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Vivek Goyal on
On Wed, Jul 07, 2010 at 01:03:08PM -0400, Jeff Moyer wrote:
> Corrado Zoccolo <czoccolo(a)gmail.com> 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:
>
> 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 80.82 138.1 | 4.5 130.7
>
> fs_mark results are in files/second, fio results are in MB/s. All
> results are the average of 5 runs. In order to get results for the
> mixed workload for both vanilla and Corrado's kernels, I had to extend
> the runtime from 30s to 300s.
>
> So, the changes proposed in this thread actually make performance worse
> across the board.

This is really surprising. It should have atleast helped in just fs_mark
case.

I think what is happening is that we are idling on the fsync queue
(because it is last queue in the group). After some time jbd thread will
submit some IO and we will not preempt the fsync thread. That's why
I had also implemented the logic of allowing preemption in case of group
idle and that had helped.

>
> I re-ran my tests against a RHEL 5 kernel (which is based on 2.6.18),
> and it shows that fs_mark performance is much better than stock CFQ in
> 2.6.35-rc3, and the mixed workload results are much the same as they are
> now (which is to say, the fs_mark process is completely starved by the
> sequential reader). So, that problem has existed for a long time.

If we just stop idling on WRITE_SYNC, we will should be back to almost
2.6.18 CFQ behavior.

>
> I'm still in the process of collecting data from production servers and
> will report back with my findings there.

That would be great.

Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeff Moyer on
Corrado Zoccolo <czoccolo(a)gmail.com> writes:

> On Wed, Jul 7, 2010 at 7:03 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote:
>> Corrado Zoccolo <czoccolo(a)gmail.com> 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.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/