linux-kernel - Re: [patch v3 2/3] block: hold queue if flush is running for non-queueable flush drive

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]

Message-ID: <20110506065347.GA8824@htj.dyndns.org>
Date:	Fri, 6 May 2011 08:53:47 +0200
From:	Tejun Heo <htejun@...il.com>
To:	Shaohua Li <shaohua.li@...el.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
	"jaxboe@...ionio.com" <jaxboe@...ionio.com>,
	"hch@...radead.org" <hch@...radead.org>,
	"jgarzik@...ox.com" <jgarzik@...ox.com>,
	"djwong@...ibm.com" <djwong@...ibm.com>,
	"sshtylyov@...sta.com" <sshtylyov@...sta.com>,
	James Bottomley <James.Bottomley@...senPartnership.com>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"ricwheeler@...il.com" <ricwheeler@...il.com>
Subject: Re: [patch v3 2/3] block: hold queue if flush is running for
 non-queueable flush drive

Hello,

On Fri, May 06, 2011 at 12:32:05PM +0800, Shaohua Li wrote:
> > - This is much more minor but if block layer already knows flushes are
> >   non-queueable, it might be a good idea to hold dispatching of
> >   flushes if other requests are already in progress.  It will only
> >   save dispatch/requeue overhead which might not matter at all, so
> >   this has pretty good chance of not being worth of the added
> >   complexity tho.
>
> I did some experiment to hold flush too, but no obvious performance
> difference. It doesn't make more flush requests merge. Avoiding
> unnecessary requeue is a gain for fast devices, but my test doesn't
> show.

I see.  Thanks for testing.

> Subject: block: hold queue if flush is running for non-queueable flush drive
> 
> Commit 53d63e6b0dfb9(block: make the flush insertion use the tail of
> the dispatch list) causes about 20% regression running a sysbench fileio
> workload. Let's consider the following scenario:
> - flush1 is dispatched with write1 in the elevator.
> - Driver dispatches write1 and requeues it.
> - flush2 is issued and appended to dispatch queue after the requeued write1. 
>   As write1 has been requeued flush2 can't be put in front of it.
> - When flush1 finishes, the driver has to process write1 before flush2 even
>   though there's no fundamental reason flush2 can't be processed first and,
>   when two flushes are issued back-to-back without intervening writes, the
>   second one essentially becomes noop.
> Without the commit, flush2 is inserted before write1, so the issue is hiden.
> But the commit itself makes sense, because flush request isn't a preempt
> request, there is no reason to add it to queue head.
> 
> The regression is exposed in a SATA device. In SATA, flush requests are
> non-queueable. When flush request is running, normal read/write requests
> can't run. If block layer dispatches such request, driver can't handle it
> and requeue it. Tejun suggested we can hold the queue when flush is running.
> This can avoid unnecessary requeue.
> 
> And also this can improve performance and solve the regression. In above
> scenario, when flush1 is running, queue is hold, so write1 isn't dispatched.
> flush2 will be the only request in the queue. After flush1 is finished, flush2
> will be dispatched soon. Since there is no write between flush1 and flush2,
> flush2 essentially becomes noop.
> 
> Signed-off-by: Shaohua Li <shaohua.li@...el.com>
> Acked-by: Tejun Heo <tj@...nel.org>

Jens, can you please queue this for the next merge window?

Thanks.

-- 
tejun
--
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/