On Wed, 2018-01-24 at 00:16 +0800, Ming Lei wrote: > @@ -1280,10 +1282,18 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, > struct list_head *list, > * - Some but not all block drivers stop a queue before > * returning BLK_STS_RESOURCE. Two exceptions are scsi-mq > * and dm-rq. > + * > + * If drivers return BLK_STS_RESOURCE and S_SCHED_RESTART > + * bit is set, run queue after 10ms for avoiding IO hang > + * because the queue may be idle and the RESTART mechanism > + * can't work any more. > */ > - if (!blk_mq_sched_needs_restart(hctx) || > + needs_restart = blk_mq_sched_needs_restart(hctx); > + if (!needs_restart || > (no_tag && list_empty_careful(&hctx->dispatch_wait.entry))) > blk_mq_run_hw_queue(hctx, true); > + else if (needs_restart && (ret == BLK_STS_RESOURCE)) > + blk_mq_delay_run_hw_queue(hctx, 10); > }
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. * This change makes block drivers less readable because anyone who encounters BLK_STS_DEV_RESOURCE will have to look up its definition to figure out what it's meaning is. * We don't need the new status code BLK_STS_DEV_RESOURCE because a delayed queue run can be implemented easily with the existing block layer API. Bart.
