Re: [PATCH v2] I/O errors for ALUA state transitions

Damien Le Moal
2024-05-07T09:12:19Z

On 5/4/24 04:56, Martin Wilck wrote:
> When a host is configured with a few LUNs and IO is running,
> injecting FC faults repeatedly leads to path recovery problems.
> The LUNs have 4 paths each and 3 of them come back active after
> say an FC fault which makes two of the paths go down, instead of
> all 4. This happens after several iterations of continuous FC faults.
>
> Reason here is that we're returning an I/O error whenever we're
> encountering sense code 06/04/0a (LOGICAL UNIT NOT ACCESSIBLE,
> ASYMMETRIC ACCESS STATE TRANSITION) instead of retrying.
>
> mwilck: Resending a modified version of this patch, which was originally
> authored by Rajashekhar M A from Netapp, and submitted in 2021.
> Moved the changes to alua_check_sense() as suggested by Mike Christie [1].
> Evan Milne had raised the question whether pg->state should be set to
> transitioning in the UA case [2]. I believe that doing this is
> correct. SCSI_ACCESS_STATE_TRANSITIONING by itself doesn't cause I/O
> errors. Our handler schedules an RTPG, which will only result in an I/O
> error condition if the transitioning timeout expires.
>
> [1] https://lore.kernel.org/all/0bc96e82-fdda-4187-148d-5b34f81d4942@oracle.com/
> [2] https://lore.kernel.org/all/CAGtn9r=kicnTDE2o7Gt5Y=yoidHYD7tG8XdMHEBJTBraVEoOCw@mail.gmail.com/
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Co-authored-by: Rajashekhar M A <rajs@netapp.com>
> ---
> drivers/scsi/device_handler/scsi_dh_alua.c | 34 +++++++++++++---------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index a226dc1b65d7..682d5bb53d14 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -414,28 +414,34 @@ static char print_alua_state(unsigned char state)
> }
> }
>
> -static enum scsi_disposition alua_check_sense(struct scsi_device *sdev,
> - struct scsi_sense_hdr *sense_hdr)
> +static enum scsi_disposition alua_handle_state_transition(struct scsi_device *sdev)
> {
> struct alua_dh_data *h = sdev->handler_data;
> struct alua_port_group *pg;
>
> + /*
> + * 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;
> +}
> +
> +static enum scsi_disposition alua_check_sense(struct scsi_device *sdev,
> + struct scsi_sense_hdr *sense_hdr)
> +{
> 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;
> - }
> + if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0a)

Please keep the comment that spells out what this asc/ascq is.

> + return alua_handle_state_transition(sdev);
> break;
> case UNIT_ATTENTION:
> + if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0a)
> + return alua_handle_state_transition(sdev);
> if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00) {
> /*
> * Power On, Reset, or Bus Device Reset.

--
Damien Le Moal
Western Digital Research