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

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

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().

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.

> * scsi-mq.
> 
> Please note that the queue_rq() function of *all* these drivers can return
> BLK_STS_RESOURCE.

Yes, but this patch doesn't remove the RESTART for the current hctx, and
only removed the part of handling cross-queue RESTART.

> 
> > Actually SCSI's RESTART is very thin, and it is just the per-host starved_list.
> 
> Do you see any reason that would prevent to move that mechanism from scsi-mq
> into the blk-mq core?

I don't see it is required from SCSI's view since this way has been run
for SCSI for long time, and we don't have this requirement for other
in-tree drivers meantime.

Also as I mentioned before, driver has more knowledge than generic block
layer for making it more efficiently.

Thanks,
Ming