linux-kernel - Re: [PATCH]cfq-iosched: don't stop async queue with async requests pending

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 14 Jan 2010 11:43:51 +0800
From:	Shaohua Li <shaohua.li@...el.com>
To:	Gui Jianfeng <guijianfeng@...fujitsu.com>
Cc:	"jens.axboe@...cle.com" <jens.axboe@...cle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"jmoyer@...hat.com" <jmoyer@...hat.com>,
	"vgoyal@...hat.com" <vgoyal@...hat.com>,
	"yanmin_zhang@...ux.intel.com" <yanmin_zhang@...ux.intel.com>
Subject: Re: [PATCH]cfq-iosched: don't stop async queue with async requests
 pending

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@...el.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/