[lkml]   [2010]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRE: [PATCH]cfq-iosched: don't stop async queue with async requests pending

>-----Original Message-----
>From: Vivek Goyal []
>Sent: Thursday, January 14, 2010 7:09 PM
>To: Li, Shaohua
>Cc: Gui Jianfeng; Corrado Zoccolo;; linux-
>Subject: Re: [PATCH]cfq-iosched: don't stop async queue with async
>requests pending
>On Thu, Jan 14, 2010 at 02:17:31PM +0800, Shaohua Li wrote:
>> On Thu, Jan 14, 2010 at 01:27:21PM +0800, Gui Jianfeng wrote:
>> > Shaohua Li wrote:
>> > > On Wed, Jan 13, 2010 at 07:13:41PM +0800, Vivek Goyal wrote:
>> > >> On Wed, Jan 13, 2010 at 04:23:22PM +0800, Shaohua Li wrote:
>> > >>> On Wed, Jan 13, 2010 at 04:18:47PM +0800, Corrado Zoccolo wrote:
>> > >>>> On Wed, Jan 13, 2010 at 8:44 AM, Shaohua Li <>
>> > >>>>> My SSD speed of direct write is about 80m/s, while I test page
>> > >>>>> the speed can only go to 68m/s. Below patch fixes this.
>> > >>>>> It appears we missused cfq_should_idle in cfq_may_dispatch.
>> > >>>>> means a queue should idle because it's seekless sync queue or
>it's the last queue,
>> > >>>>> which is to maintain service tree time slice. So it doesn't mean
>> > >>>>> last queue is always a sync queue. If the last queue is asyn
>> > >>>>> we definitely shouldn't stop dispatch requests because of
>pending async
>> > >>>>> requests.
>> > >>>> An other option is that cfq_should_idle returns false for async
>> > >>>> queues, since cfq will never idle on them.
>> > >>> I'm considering this option too, but it appears we need make async
>> > >>> idle to maintain domain time slice.
>> > >> IMHO, we don't have to wait on async write service tree. Generally
>> > >> write queus contain many requests and they are not like reads where
>> > >> request is expected. So idling on aysnc write service tree is waste
>> > >> time and will lead to reduced throughput.
>> > > I fully agree async queue doesn't need wait. I thought the purpose
>we add the last
>> > > queue check in cfq_should_idle is we want a service tree or a group
>has dedicated
>> > > slice, because before the service tree/group slice is expired, new
>queue can jump
>> > > in and if we don't idle, the new queue can only run at next slice.
>Not sure if I
>> > > understand the code correctly.
>> >
>> > Hi Shaohua,
>> >
>> > If a cfq queue is the last one in the io group, if we expire this cfqq
>> > io group will be removed from service tree. When io group gets
>backlogged again, it
>> > will be put at the end of service tree, so it loses its previous share.
>so we add
>> > the last check here from the fairness point of view.
>> ya, this is what I'm understanding. So we can't return false for async
>> in cfq_should_idle if the queue is the last one of service tree.
>Yes cfq_should_idle() can check for async queue and return false.
>Regarding group loosing fair share, currently all async queues are in root
>group and not in individual groups, so this particular change should not
>affect a lot. We will continue to idle on sync-idle and sync-noidle
>service tree. Only async service tree is the exception.
>Once we introduce per group async queue in future, we shall have to come
>up with something else, if need be.
>So keep this as a separate patch. I think in the presence of mixed
>workload, (readers and buffered writers), it might give little performance
>boost. We need to test it though.
Ok, if you thought this method doesn't break group, here is the updated
patch. I'm sorry to send the attached patch, my mailbox has trouble.
[unhandled content-type:application/octet-stream]
 \ /
  Last update: 2010-01-19 01:55    [W:0.086 / U:2.360 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site