[PATCH 0/4] scsi: remove devices in ALUA transitioning status

[PATCH 0/4] scsi: remove devices in ALUA transitioning status

[Hannes Reinecke]

Hi all,

during testing we found that there is an issue with dev_loss_tmo and
devices in ALUA transitioning state.
What happens is that I/O gets requeued via BLK_STS_RESOURCE for these
devices, so when dev_loss_tmo triggers the SCSI core cannot flush the
request list as I/O is simply requeued.

So when the driver is trying to re-establish the device it'll wait for
that last reference to drop in order to re-attach the device, but as I/O
is still outstanding on the (old) device it'll wait for ever.

Fix this by returning 'BLK_STS_AGAIN' from scsi_dh_alua when the device
is in ALUA transitioning, and also set the 'transitioning' state when
scsi_dh_alua is receiving a sense code, and not only after scsi_dh_alua
successfully received the response to a REPORT TARGET PORT GROUPS
command.

Hannes Reinecke (4):
  block: return status code in blk_mq_end_request()
  scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state
  scsi_dh_alua: set 'transitioning' state on unit attention
  scsi: return BLK_STS_AGAIN for ALUA transitioning

 block/blk-mq.c                             |  2 +-
 drivers/scsi/device_handler/scsi_dh_alua.c | 10 +++++++++-
 drivers/scsi/scsi_lib.c                    |  8 ++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.16.4

[PATCH 1/4] block: return status code in blk_mq_end_request()

[Hannes Reinecke]

blk_mq_end_request() will use the block status returned from
queue_rq() as argument, except in one instance in blk_mq_dispatch_rq_list(),
where the generic BLK_STS_IOERR is used.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 0015a1892153..a19dda3a4ad0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1403,7 +1403,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 			break;
 		default:
 			errors++;
-			blk_mq_end_request(rq, BLK_STS_IOERR);
+			blk_mq_end_request(rq, ret);
 		}
 	} while (!list_empty(list));
 out:
-- 
2.16.4

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

[Hannes Reinecke]

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;
-- 
2.16.4

[PATCH 3/4] scsi_dh_alua: set 'transitioning' state on unit attention

[Hannes Reinecke]

We should be setting the 'transitioning' ALUA state once we get
a unit attention indicating the array is in transitioning.
There are arrays which cannot respond to an rtpg while in transitioning,
and others have issues correctly reporting the state.
So better to set the state during unit attention handling and wait
for TUR / RTPG to run its course.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index a68222e324e9..ea436a14087f 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -408,12 +408,20 @@ static char print_alua_state(unsigned char state)
 static int alua_check_sense(struct scsi_device *sdev,
 			    struct scsi_sense_hdr *sense_hdr)
 {
+	struct alua_dh_data *h = sdev->handler_data;
+	struct alua_port_group *pg;
+
 	switch (sense_hdr->sense_key) {
 	case NOT_READY:
 		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0a) {
 			/*
 			 * LUN Not Accessible - ALUA state transition
 			 */
+			rcu_read_lock();
+			pg = rcu_dereference(h->pg);
+			if (pg)
+				pg->state = SCSI_ACCESS_STATE_TRANSITIONING;
+			rcu_read_unlock();
 			alua_check(sdev, false);
 			return NEEDS_RETRY;
 		}
-- 
2.16.4

[PATCH 4/4] scsi: return BLK_STS_AGAIN for ALUA transitioning

[Hannes Reinecke]

Whenever we encounter a sense code of ALUA transitioning in
scsi_io_completion() it means that the SCSI midlayer ran out
of retries trying to wait for ALUA transitioning.
In these cases we should be passing up the error, but signalling
that the I/O might be retried, preferably on another path.
So return BLK_STS_AGAIN in these cases.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_lib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b628aa0d824c..e6221d65fa8e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -764,6 +764,9 @@ static void scsi_io_completion_action(struct scsi_cmnd *cmd, int result)
 				case 0x24: /* depopulation in progress */
 					action = ACTION_DELAYED_RETRY;
 					break;
+				case 0x0a: /* ALUA state transistion */
+					blk_stat = BLK_STS_AGAIN;
+					/* fall through */
 				default:
 					action = ACTION_FAIL;
 					break;
-- 
2.16.4

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

[Bart Van Assche]

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.

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

[Hannes Reinecke]

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
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: [PATCH 0/4] scsi: remove devices in ALUA transitioning status

[Martin K. Petersen]

Hannes,

> during testing we found that there is an issue with dev_loss_tmo and
> devices in ALUA transitioning state.  What happens is that I/O gets
> requeued via BLK_STS_RESOURCE for these devices, so when dev_loss_tmo
> triggers the SCSI core cannot flush the request list as I/O is simply
> requeued.
>
> So when the driver is trying to re-establish the device it'll wait for
> that last reference to drop in order to re-attach the device, but as
> I/O is still outstanding on the (old) device it'll wait for ever.
>
> Fix this by returning 'BLK_STS_AGAIN' from scsi_dh_alua when the
> device is in ALUA transitioning, and also set the 'transitioning'
> state when scsi_dh_alua is receiving a sense code, and not only after
> scsi_dh_alua successfully received the response to a REPORT TARGET
> PORT GROUPS command.

It would be good to get this revived/reviewed.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/4] scsi: remove devices in ALUA transitioning status

[Ewan D. Milne]

On Wed, 2020-09-30 at 10:02 +0200, Hannes Reinecke wrote:
> Hi all,
> 
> during testing we found that there is an issue with dev_loss_tmo and
> devices in ALUA transitioning state.
> What happens is that I/O gets requeued via BLK_STS_RESOURCE for these
> devices, so when dev_loss_tmo triggers the SCSI core cannot flush the
> request list as I/O is simply requeued.
> 
> So when the driver is trying to re-establish the device it'll wait
> for
> that last reference to drop in order to re-attach the device, but as
> I/O
> is still outstanding on the (old) device it'll wait for ever.
> 
> Fix this by returning 'BLK_STS_AGAIN' from scsi_dh_alua when the
> device
> is in ALUA transitioning, and also set the 'transitioning' state when
> scsi_dh_alua is receiving a sense code, and not only after
> scsi_dh_alua
> successfully received the response to a REPORT TARGET PORT GROUPS
> command.
> 
> Hannes Reinecke (4):
>   block: return status code in blk_mq_end_request()
>   scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state
>   scsi_dh_alua: set 'transitioning' state on unit attention
>   scsi: return BLK_STS_AGAIN for ALUA transitioning
> 
>  block/blk-mq.c                             |  2 +-
>  drivers/scsi/device_handler/scsi_dh_alua.c | 10 +++++++++-
>  drivers/scsi/scsi_lib.c                    |  8 ++++++++
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 

We had a report of I/O hangs during storage controller resets
and analysis of the kernel state showed the sdev in ALUA transitioning.

The patch set fixes the ALUA transitioning issue, it looks good.
There was a reproducible test case.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

-Ewan

Re: [PATCH 0/4] scsi: remove devices in ALUA transitioning status

[Martin K. Petersen]

Ewan, Hannes,

>> during testing we found that there is an issue with dev_loss_tmo and
>> devices in ALUA transitioning state.  What happens is that I/O gets
>> requeued via BLK_STS_RESOURCE for these devices, so when dev_loss_tmo
>> triggers the SCSI core cannot flush the request list as I/O is simply
>> requeued.

Applied to 5.11/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering