Re: [PATCH V2 0/2] block: remove unnecessary RESTART

[Ming Lei <ming.lei@redhat.com>]

On Wed, Nov 01, 2017 at 03:54:09AM +0000, Bart Van Assche wrote:
> On Tue, 2017-10-31 at 09:47 +0800, Ming Lei wrote:
> > On Mon, Oct 30, 2017 at 08:24:57PM +0000, Bart Van Assche wrote:
> > > On Fri, 2017-10-27 at 13:38 +0800, Ming Lei wrote:
> > > > On Fri, Oct 27, 2017 at 04:53:18AM +0000, Bart Van Assche wrote:
> > > > > On Fri, 2017-10-27 at 12:43 +0800, Ming Lei wrote:
> > > > > > The 1st patch removes the RESTART for TAG-SHARED because SCSI handles it
> > > > > > by itself, and not necessary to waste CPU to do the expensive RESTART.
> > > > > > And Roman Pen reported that this RESTART cuts half of IOPS in his case.
> > > > > > 
> > > > > > The 2nd patch removes the RESTART when .get_budget returns BLK_STS_RESOURCE,
> > > > > > and this RESTART is handled by SCSI's RESTART(scsi_end_request()) too.
> > > > > 
> > > > > There are more block drivers than the SCSI core that share tags. If the
> > > > 
> > > > Could you share us what the other in-tree driver which share tags is?
> > > 
> > > I think the following in-tree drivers support shared tags (in alphabetical
> > > order):
> > > * null_blk. See also the shared_tags kernel module parameter.
> > > * nvme. See also nvme_alloc_ns().
> > > * scsi-mq.
> > 
> > For both null_blk and nvme, we don't need to deal with cross-queue RESTART,
> > because BLK_MQ_S_TAG_WAITING has handled it already.
> 
> blk_mq_dispatch_wait_add() returns immediately if BLK_MQ_S_TAG_WAITING is
> already set. Are you really sure that removing the restart mechanism doesn't

If this flag is set, that means blk_mq_dispatch_wake() will be run when
driver tag is available, so the hw queue will be run at that time.

> break the NVMe driver if there are multiple namespaces?

Did your commit 6d8c6c0f97ad ("blk-mq: Restart a single queue if tag sets are shared")
suppose to fix NVMe?

Please take a look at the cover letter:

	https://marc.info/?t=149158911500010&r=1&w=2


-- 
Ming