[dm-devel] [PATCH V3 2/5] dm-mpath: return DM_MAPIO_REQUEUE in case of rq allocation failure

[Bart Van Assche]

On Thu, 2018-01-11 at 14:01 +0800, Ming Lei wrote:
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 86bf502a8e51..fcddf5a62581 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -533,8 +533,20 @@ static int multipath_clone_and_map(struct dm_target *ti, struct request *rq,
>  		if (queue_dying) {
>  			atomic_inc(&m->pg_init_in_progress);
>  			activate_or_offline_path(pgpath);
> +			return DM_MAPIO_DELAY_REQUEUE;
>  		}
> -		return DM_MAPIO_DELAY_REQUEUE;
> +
> +		/*
> +		 * blk-mq's SCHED_RESTART can cover this requeue, so
> +		 * we needn't to deal with it by DELAY_REQUEUE. More
> +		 * importantly, we have to return DM_MAPIO_REQUEUE
> +		 * so that blk-mq can get the queue busy feedback,
> +		 * otherwise I/O merge can be hurt.
> +		 */
> +		if (q->mq_ops)
> +			return DM_MAPIO_REQUEUE;
> +		else
> +			return DM_MAPIO_DELAY_REQUEUE;
>  	}

Sorry but the approach of this patch looks wrong to me. I'm afraid that this
approach will cause 100% CPU consumption if the underlying .queue_rq()
function returns BLK_STS_RESOURCE for another reason than what can be detected
by the .get_budget() call. This can happen if e.g. a SCSI LLD .queuecommand()
implementation returns SCSI_MLQUEUE_HOST_BUSY. Many SCSI LLDs can do this:
$ git grep 'SCSI_MLQUEUE_[^_]*_BUSY' | wc -l
204

Isn't this a severe regression?

Bart.