On Tue, Jan 23 2018 at 10:31pm -0500,
Ming Lei [off-list ref] wrote:

On Tue, Jan 23, 2018 at 04:57:34PM +0000, Bart Van Assche wrote:

quoted

On Wed, 2018-01-24 at 00:37 +0800, Ming Lei wrote:

quoted

On Tue, Jan 23, 2018 at 04:24:20PM +0000, Bart Van Assche wrote:

quoted

My opinion about this patch is as follows:
* Changing a blk_mq_delay_run_hw_queue() call followed by return
  BLK_STS_DEV_RESOURCE into return BLK_STS_RESOURCE is wrong because it changes
  a guaranteed queue rerun into a queue rerun that may or may not happen
  depending on whether or not multiple queue runs happen simultaneously.

You may not understand the two:

1) it is always safe to return BLK_STS_RESOURCE, which will make sure to
avoid IO hang by blk_mq_delay_run_hw_queue() or blk_mq_run_hw_queue(),
and using which one depends on SCHED_RESTART.

2) if driver can make sure the queue will be rerun after some resource
is available, either by itself or by blk-mq, it will return BLK_STS_DEV_RESOURCE

So what is wrong with this way?

Sorry, I swapped BLK_STS_DEV_RESOURCE and BLK_STS_RESOURCE accidentally in my
reply. What I meant is that changing a blk_mq_delay_run_hw_queue() call followed
by return BLK_STS_RESOURCE into BLK_STS_DEV_RESOURCE is wrong and introduces a
race condition in code where there was no race condition.

OK, then no such race you worried about in this patch.

Jens, could you take a look at this patch?

Hi Jens,

Ming let me know that he successfully tested this V3 patch using both
your test (fio to both mpath and underlying path) and Bart's (02-mq with
can_queue in guest).

Would be great if you'd review and verify this fix works for you too.

Ideally we'd get a fix for this regression staged for 4.16 inclusion.
This V3 patch seems like the best option we have at this point.

Mike