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

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?

Thanks,
Ming