* [PATCH 1/4] block: return status code in blk_mq_end_request()
2020-09-30 8:02 [PATCH 0/4] scsi: remove devices in ALUA transitioning status Hannes Reinecke
@ 2020-09-30 8:02 ` Hannes Reinecke
2020-09-30 8:02 ` [PATCH 2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state Hannes Reinecke
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2020-09-30 8:02 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state
2020-09-30 8:02 [PATCH 0/4] scsi: remove devices in ALUA transitioning status Hannes Reinecke
2020-09-30 8:02 ` [PATCH 1/4] block: return status code in blk_mq_end_request() Hannes Reinecke
@ 2020-09-30 8:02 ` Hannes Reinecke
2020-10-01 2:44 ` Bart Van Assche
2020-09-30 8:02 ` [PATCH 3/4] scsi_dh_alua: set 'transitioning' state on unit attention Hannes Reinecke
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2020-09-30 8:02 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state
2020-09-30 8:02 ` [PATCH 2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state Hannes Reinecke
@ 2020-10-01 2:44 ` Bart Van Assche
2020-10-01 9:58 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Bart Van Assche @ 2020-10-01 2:44 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state
2020-10-01 2:44 ` Bart Van Assche
@ 2020-10-01 9:58 ` Hannes Reinecke
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2020-10-01 9:58 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] scsi_dh_alua: set 'transitioning' state on unit attention
2020-09-30 8:02 [PATCH 0/4] scsi: remove devices in ALUA transitioning status Hannes Reinecke
2020-09-30 8:02 ` [PATCH 1/4] block: return status code in blk_mq_end_request() Hannes Reinecke
2020-09-30 8:02 ` [PATCH 2/4] scsi_dh_alua: return BLK_STS_AGAIN for ALUA transitioning state Hannes Reinecke
@ 2020-09-30 8:02 ` Hannes Reinecke
2020-09-30 8:02 ` [PATCH 4/4] scsi: return BLK_STS_AGAIN for ALUA transitioning Hannes Reinecke
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2020-09-30 8:02 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] scsi: return BLK_STS_AGAIN for ALUA transitioning
2020-09-30 8:02 [PATCH 0/4] scsi: remove devices in ALUA transitioning status Hannes Reinecke
` (2 preceding siblings ...)
2020-09-30 8:02 ` [PATCH 3/4] scsi_dh_alua: set 'transitioning' state on unit attention Hannes Reinecke
@ 2020-09-30 8:02 ` Hannes Reinecke
2020-10-27 0:21 ` [PATCH 0/4] scsi: remove devices in ALUA transitioning status Martin K. Petersen
2020-11-07 0:02 ` Ewan D. Milne
5 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2020-09-30 8:02 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] scsi: remove devices in ALUA transitioning status
2020-09-30 8:02 [PATCH 0/4] scsi: remove devices in ALUA transitioning status Hannes Reinecke
` (3 preceding siblings ...)
2020-09-30 8:02 ` [PATCH 4/4] scsi: return BLK_STS_AGAIN for ALUA transitioning Hannes Reinecke
@ 2020-10-27 0:21 ` Martin K. Petersen
2020-11-07 0:02 ` Ewan D. Milne
5 siblings, 0 replies; 10+ messages in thread
From: Martin K. Petersen @ 2020-10-27 0:21 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, James Bottomley, linux-scsi
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] scsi: remove devices in ALUA transitioning status
2020-09-30 8:02 [PATCH 0/4] scsi: remove devices in ALUA transitioning status Hannes Reinecke
` (4 preceding siblings ...)
2020-10-27 0:21 ` [PATCH 0/4] scsi: remove devices in ALUA transitioning status Martin K. Petersen
@ 2020-11-07 0:02 ` Ewan D. Milne
2020-11-11 3:58 ` Martin K. Petersen
5 siblings, 1 reply; 10+ messages in thread
From: Ewan D. Milne @ 2020-11-07 0:02 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, James Bottomley, linux-scsi
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/4] scsi: remove devices in ALUA transitioning status
2020-11-07 0:02 ` Ewan D. Milne
@ 2020-11-11 3:58 ` Martin K. Petersen
0 siblings, 0 replies; 10+ messages in thread
From: Martin K. Petersen @ 2020-11-11 3:58 UTC (permalink / raw)
To: Ewan D. Milne
Cc: Hannes Reinecke, Martin K. Petersen, Christoph Hellwig,
James Bottomley, linux-scsi
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
^ permalink raw reply [flat|nested] 10+ messages in thread