[2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state

When the ALUA state indicates transitioning we should not retry
the command immediately, but rather complete the command with
BLK_STS_AGAIN to signal the completion handler that it might
be retried.
This allows multipathing to redirect the command to another path
if possible, and avoid stalls during lengthy transitioning times.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 2 +-
 drivers/scsi/scsi_lib.c                    | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

On 2020-09-30 01:02, Hannes Reinecke wrote:
> When the ALUA state indicates transitioning we should not retry
> the command immediately, but rather complete the command with
> BLK_STS_AGAIN to signal the completion handler that it might
> be retried.
> This allows multipathing to redirect the command to another path
> if possible, and avoid stalls during lengthy transitioning times.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 2 +-
>  drivers/scsi/scsi_lib.c                    | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 308bda2e9c00..a68222e324e9 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -1092,7 +1092,7 @@ static blk_status_t alua_prep_fn(struct scsi_device *sdev, struct request *req)
>  	case SCSI_ACCESS_STATE_LBA:
>  		return BLK_STS_OK;
>  	case SCSI_ACCESS_STATE_TRANSITIONING:
> -		return BLK_STS_RESOURCE;
> +		return BLK_STS_AGAIN;
>  	default:
>  		req->rq_flags |= RQF_QUIET;
>  		return BLK_STS_IOERR;
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index f0ee11dc07e4..b628aa0d824c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1726,6 +1726,11 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>  		    scsi_device_blocked(sdev))
>  			ret = BLK_STS_DEV_RESOURCE;
>  		break;
> +	case BLK_STS_AGAIN:
> +		scsi_req(req)->result = DID_BUS_BUSY << 16;
> +		if (req->rq_flags & RQF_DONTPREP)
> +			scsi_mq_uninit_cmd(cmd);
> +		break;
>  	default:
>  		if (unlikely(!scsi_device_online(sdev)))
>  			scsi_req(req)->result = DID_NO_CONNECT << 16;

Hi Hannes,

What will happen if all remote ports have the state "transitioning"?
Does the above code resubmit a request immediately in that case? Can
this cause spinning with 100% CPU usage if the ALUA device handler
notices the transitioning state before multipathd does?

Thanks,

Bart.

On 10/1/20 4:44 AM, Bart Van Assche wrote:
> On 2020-09-30 01:02, Hannes Reinecke wrote:
>> When the ALUA state indicates transitioning we should not retry
>> the command immediately, but rather complete the command with
>> BLK_STS_AGAIN to signal the completion handler that it might
>> be retried.
>> This allows multipathing to redirect the command to another path
>> if possible, and avoid stalls during lengthy transitioning times.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/scsi/device_handler/scsi_dh_alua.c | 2 +-
>>   drivers/scsi/scsi_lib.c                    | 5 +++++
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
>> index 308bda2e9c00..a68222e324e9 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
>> @@ -1092,7 +1092,7 @@ static blk_status_t alua_prep_fn(struct scsi_device *sdev, struct request *req)
>>   	case SCSI_ACCESS_STATE_LBA:
>>   		return BLK_STS_OK;
>>   	case SCSI_ACCESS_STATE_TRANSITIONING:
>> -		return BLK_STS_RESOURCE;
>> +		return BLK_STS_AGAIN;
>>   	default:
>>   		req->rq_flags |= RQF_QUIET;
>>   		return BLK_STS_IOERR;
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index f0ee11dc07e4..b628aa0d824c 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -1726,6 +1726,11 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>>   		    scsi_device_blocked(sdev))
>>   			ret = BLK_STS_DEV_RESOURCE;
>>   		break;
>> +	case BLK_STS_AGAIN:
>> +		scsi_req(req)->result = DID_BUS_BUSY << 16;
>> +		if (req->rq_flags & RQF_DONTPREP)
>> +			scsi_mq_uninit_cmd(cmd);
>> +		break;
>>   	default:
>>   		if (unlikely(!scsi_device_online(sdev)))
>>   			scsi_req(req)->result = DID_NO_CONNECT << 16;
> 
> Hi Hannes,
> 
> What will happen if all remote ports have the state "transitioning"?
> Does the above code resubmit a request immediately in that case? Can
> this cause spinning with 100% CPU usage if the ALUA device handler
> notices the transitioning state before multipathd does?
> 
Curiously this patch only improves the current situation :-)
With the current behaviour we will return 
BLK_STS_DEV_RESOURCE/BLK_STS_RESOURCE, causing an _immediate_ retry on 
the same queue.
This patch will cause the I/O to be _completed_, to be requeued via the 
end_request() path.
So yes, we will requeue (that's kinda the point of 'transitioning' ...), 
but with a lower frequency as originally. _And_ with a chance of 
multipath intercepting the I/O, which didn't happen previously.

Cheers,

Hannes