On Tue, Jan 23 2018 at 10:31pm -0500, Ming Lei <ming.lei@xxxxxxxxxx> wrote: > 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: > > > On Tue, Jan 23, 2018 at 04:24:20PM +0000, Bart Van Assche wrote: > > > > 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
