[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.027 / U:68.160 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site