Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
From: "Ewan D. Milne" <emilne@redhat.com>
To: Hannes Reinecke <hare@suse.de>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>,
	James Bottomley <james.bottomley@hansenpartnership.com>,
	Martin Wilck <martin.wilck@suse.com>,
	linux-scsi@vger.kernel.org, Hannes Reinecke <hare@suse.com>
Subject: Re: [PATCH] scsi_dh_alua: handle RTPG sense code correctly during state transitions
Date: Mon, 07 Oct 2019 16:45:44 -0400
Message-ID: <4ff5beeae5092d313d0e90a83f04a222400180f1.camel@redhat.com> (raw)
In-Reply-To: <20191007135701.32389-1-hare@suse.de>

See below.

On Mon, 2019-10-07 at 15:57 +0200, Hannes Reinecke wrote:
> From: Hannes Reinecke <hare@suse.com>
> 
> Some arrays are not capable of returning RTPG data during state
> transitioning, but rather return an 'LUN not accessible, asymmetric
> access state transition' sense code. In these cases we
> can set the state to 'transitioning' directly and don't need to
> evaluate the RTPG data (which we won't have anyway).
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 4971104b1817..f32da0ca529e 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -512,6 +512,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  	unsigned int tpg_desc_tbl_off;
>  	unsigned char orig_transition_tmo;
>  	unsigned long flags;
> +	bool transitioning_sense = false;
>  
>  	if (!pg->expiry) {
>  		unsigned long transition_tmo = ALUA_FAILOVER_TIMEOUT * HZ;
> @@ -572,13 +573,19 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  			goto retry;
>  		}
>  		/*
> -		 * Retry on ALUA state transition or if any
> -		 * UNIT ATTENTION occurred.
> +		 * If the array returns with 'ALUA state transition'
> +		 * sense code here it cannot return RTPG data during
> +		 * transition. So set the state to 'transitioning' directly.
>  		 */
>  		if (sense_hdr.sense_key == NOT_READY &&
> -		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a)
> -			err = SCSI_DH_RETRY;
> -		else if (sense_hdr.sense_key == UNIT_ATTENTION)
> +		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) {
> +			transitioning_sense = true;
> +			goto skip_rtpg;
> +		}
> +		/*
> +		 * Retry on any other UNIT ATTENTION occurred.
> +		 */
> +		if (sense_hdr.sense_key == UNIT_ATTENTION)
>  			err = SCSI_DH_RETRY;
>  		if (err == SCSI_DH_RETRY &&
>  		    pg->expiry != 0 && time_before(jiffies, pg->expiry)) {
> @@ -666,7 +673,11 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
>  		off = 8 + (desc[7] * 4);
>  	}
>  
> + skip_rtpg:
>  	spin_lock_irqsave(&pg->lock, flags);
> +	if (transitioning_sense)
> +		pg->state = SCSI_ACCESS_STATE_TRANSITIONING;
> +
>  	sdev_printk(KERN_INFO, sdev,
>  		    "%s: port group %02x state %c %s supports %c%c%c%c%c%c%c\n",
>  		    ALUA_DH_NAME, pg->group_id, print_alua_state(pg->state),

The patch itself looks OK, but I was wondering about a couple of things:

  - There are other places in scsi_dh_alua where the ASC/ASCQ 04 0A is checked
    and we retry, I understand that this is a particular case you are solving
    but is the changing of the state to -> transitioning (because that's what
    the device said the state was) applicable in those other cases?
  - The code originally seems to have been under the assumption that the
    transitioning state was a transient event, so the retry would pick up
    the eventual state.  Now, some storage arrays spend a long time in the
    transitioning state, but if we don't send another command are we going to
    get the sense (or the UA) that triggers entry to the eventual ALUA state?

-Ewan


  parent reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-07 13:57 Hannes Reinecke
2019-10-07 14:15 ` Laurence Oberman
2019-10-07 20:45 ` Ewan D. Milne [this message]
2019-10-08  6:21   ` Hannes Reinecke
2019-10-08 15:58     ` Ewan D. Milne
2019-10-09 16:31 ` Bart Van Assche
2019-10-10  2:43 ` Martin K. Petersen

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ff5beeae5092d313d0e90a83f04a222400180f1.camel@redhat.com \
    --to=emilne@redhat.com \
    --cc=hare@suse.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=james.bottomley@hansenpartnership.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=martin.wilck@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git