Re: [PATCH]cfq-iosched: don't take requests with long distence as close

[Corrado Zoccolo]

On Mon, Jan 11, 2010 at 6:20 AM, Zhang, Yanmin
<yanmin_zhang@xxxxxxxxxxxxxxx> wrote:
> On Thu, 2010-01-07 at 09:30 -0500, Jeff Moyer wrote:
>> Corrado Zoccolo <czoccolo@xxxxxxxxx> writes:
>>
>> > On Tue, Jan 5, 2010 at 10:16 PM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote:
>> >>
>> >> For now, I'm leaning towards asking Jens to revert this. ÂIt may still
>> >> be worth making sure that we don't merge a seeky queue with a non-seeky
>> >> queue. ÂI have a patch for that if folks are interested.
>> > Jeff, can you send this patch to Yanmin, that is investigating a
>> > regression apparently caused by excessive queue merge?
>> >
>> > http://lkml.org/lkml/2010/1/4/194
>>
>>
>> You first have to back out Shaohua's patch, then apply this one.
> Thanks for forwarding me the patches.
> Actually, we found tiobench randread has about 20% regression since kernel
> 2.6.33-rc1, and fio randread has more than 40% regression.
>
>>
>> Cheers,
>> Jeff
>>
>> cfq-iosched: don't allow merging with seeky queues
> With your new patch applied on 2.6.33-rc1, I don't see improvement on
> both tiobench and fio randread regression. I know unexpected merge/unmerge
> is just one root cause of the regressions. A couple of other patches
> are also related to them.

Hi Yanmin,
are you testing Jeff's patch with your full fio script, instead of the
simplified one?
Since they are fixing the merging part, that happens only with the
full fio script.

>
> I also tried to apply both your patch and Corrado's patch at
> http://lkml.org/lkml/2010/1/9/57. The result seems like the one when I just apply
> Corrado's patch, with which regression almost disappears when low_latency=0. But
> when low_latency=1, there is still about 25% regression.
>
Yes, low_latency is supposed to bring some regression.
http://lkml.org/lkml/2009/12/30/47

Thanks,
Corrado
>
>>
>> Shaohua Li noticed that cfq currently can merge with seeky queues, which
>> causes unwanted merge/unmerge activity. ÂWe already know that the
>> cur_cfqq is not seeky, so this patch just makes sure that the non-seeky
>> queue is not merged with a seeky one.
>>
>> Signed-off-by: Jeff Moyer <jmoyer@xxxxxxxxxx>
>>
>> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
>> index 8df4fe5..3db9050 100644
>> --- a/block/cfq-iosched.c
>> +++ b/block/cfq-iosched.c
>> @@ -1677,6 +1677,10 @@ static inline int cfq_rq_close(struct cfq_data *cfqd, struct cfq_queue *cfqq,
>> Â Â Â return cfq_dist_from_last(cfqd, rq) <= sdist;
>> Â}
>>
>> +/*
>> + * Search for a cfqq that is issuing non-seeky I/Os within the seek
>> + * mean of the current cfqq.
>> + */
>> Âstatic struct cfq_queue *cfqq_close(struct cfq_data *cfqd,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct cfq_queue *cur_cfqq)
>> Â{
>> @@ -1701,7 +1705,14 @@ static struct cfq_queue *cfqq_close(struct cfq_data *cfqd,
>> Â Â Â Â* will contain the closest sector.
>> Â Â Â Â*/
>> Â Â Â __cfqq = rb_entry(parent, struct cfq_queue, p_node);
>> - Â Â if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq))
>> + Â Â /*
>> + Â Â Â* If the cfqq does not have enough seek samples, assume it is
>> + Â Â Â* sequential until proven otherwise. ÂIf it is assumed that the
>> + Â Â Â* queue is seeky first, then the close cooperator detection logic
>> + Â Â Â* may never trigger as one queue strays further from the other(s).
>> + Â Â Â*/
>> + Â Â if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq) &&
>> + Â Â Â Â (!sample_valid(__cfqq->seek_samples) || !CFQQ_SEEKY(__cfqq)))
>> Â Â Â Â Â Â Â return __cfqq;
>>
>> Â Â Â if (blk_rq_pos(__cfqq->next_rq) < sector)
>> @@ -1712,7 +1723,8 @@ static struct cfq_queue *cfqq_close(struct cfq_data *cfqd,
>> Â Â Â Â Â Â Â return NULL;
>>
>> Â Â Â __cfqq = rb_entry(node, struct cfq_queue, p_node);
>> - Â Â if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq))
>> + Â Â if (cfq_rq_close(cfqd, cur_cfqq, __cfqq->next_rq) &&
>> + Â Â Â Â (!sample_valid(__cfqq->seek_samples) || !CFQQ_SEEKY(__cfqq)))
>> Â Â Â Â Â Â Â return __cfqq;
>>
>> Â Â Â return NULL;
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at Âhttp://www.tux.org/lkml/
>
>
>



-- 
__________________________________________________________________________

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