Date: Thu, 14 Jan 2010 11:43:51 +0800
From: Shaohua Li <shaohua.li@intel.com>
To: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
Cc: "jens.axboe@oracle.com" <jens.axboe@oracle.com>,
       "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
       "jmoyer@redhat.com" <jmoyer@redhat.com>,
       "vgoyal@redhat.com" <vgoyal@redhat.com>,
       "yanmin_zhang@linux.intel.com" <yanmin_zhang@linux.intel.com>
Subject: Re: [PATCH]cfq-iosched: don't stop async queue with async requests
 pending
Message-ID: <20100114034351.GB3922@sli10-desk.sh.intel.com>
References: <20100113074442.GA10492@sli10-desk.sh.intel.com>
 <4B4E8590.8060403@cn.fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4B4E8590.8060403@cn.fujitsu.com>
User-Agent: Mutt/1.5.20 (2009-06-14)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1740
Lines: 37

On Thu, Jan 14, 2010 at 10:46:40AM +0800, Gui Jianfeng wrote:
> 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;
> 
> It seems only sync queue could open the idle window, so i don't think we really need this.
the purpose here isn't to open idle window. with it, we could send more requests instead
of wait previous request finish. Vivek's mail explains the behavior correctly.

Thanks,
Shaohua
--
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/