lkml.org 
[lkml]   [2010]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH]cfq-iosched: don't stop async queue with async requests pending
On Wed, Jan 13, 2010 at 10:30:52PM +0100, Corrado Zoccolo wrote:
> On Wed, Jan 13, 2010 at 12:10 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Wed, Jan 13, 2010 at 03:44:42PM +0800, Shaohua Li wrote:
> >> My SSD speed of direct write is about 80m/s, while I test page writeback,
> >> the speed can only go to 68m/s. Below patch fixes this.
> >> It appears we missused cfq_should_idle in cfq_may_dispatch. cfq_should_idle
> >> 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 the
> >> last queue is always a sync queue. If the last queue is asyn queue,
> >> we definitely shouldn't stop dispatch requests because of pending async
> >> requests.
> >>
> >> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> >>
> >> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> >> index 918c7fd..8198079 100644
> >> --- a/block/cfq-iosched.c
> >> +++ b/block/cfq-iosched.c
> >> @@ -2222,7 +2222,8 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd, struct cfq_queue *cfqq)
> >>       /*
> >>        * Drain async requests before we start sync IO
> >>        */
> >> -     if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_driver[BLK_RW_ASYNC])
> >> +     if (cfq_cfqq_sync(cfqq) && cfq_should_idle(cfqd, cfqq)
> >> +             && cfqd->rq_in_driver[BLK_RW_ASYNC])
> >>               return false;
> >
> > So are we driving queue depth as 1 when pure buffered writes are going on?
> > Because in that case service_tree->count=1 and cfq_should_idle() will
> > return 1 and looks like we will not dispatch next write till previous
> > write is over?
>
> Yes, it seems so. It has to be fixed.
>
> >
> > A general question. Why do we need to drain async requests before we start
> > sync IO? How does that help?
> >
> > A related question, even if we have to do that, why do we check for
> > cfq_should_idle()? Why can't we just do following.
> >
> >        if (cfq_cfqq_sync(cfqq) && cfqd->rq_in_driver[BLK_RW_ASYNC])
> >
> This would wait also for seeky queues. I think we drain to avoid
> disrupting a sync
> stream with far writes, but it is not needed when the queues are already seeky.

Ok, that makes sense. But again the conflict is single disk vs RAID. RAID
array will most likely be doing some write caching or writes and reads
might be travelling to different disks hence sequential reads should not
be significantly impacted. Another case to optimize if we can reliably
identify RAIDs.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-01-13 23:29    [W:0.040 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site