Re: [PATCH 13/13] scsi: core: Add implicit ALUA support

[Benjamin Marzinski <bmarzins@redhat.com>]

On Mon, Mar 23, 2026 at 12:52:18PM +0000, John Garry wrote:
> On 23/03/2026 01:58, Benjamin Marzinski wrote:
> > On Tue, Mar 17, 2026 at 12:07:03PM +0000, John Garry wrote:
> > > For when no device handler is used, add ALUA support.
> > > 
> > > This will be equivalent to when native SCSI multipathing is used.
> > > 
> > > Essentially all the same handling is available as DH alua driver for
> > > rescan, request prep, sense handling.
> > > 
> > > Signed-off-by: John Garry <john.g.garry@oracle.com>
> > > ---
> > >   drivers/scsi/scsi_alua.c  | 93 +++++++++++++++++++++++++++++++++++++++
> > >   drivers/scsi/scsi_error.c |  7 +++
> > >   drivers/scsi/scsi_lib.c   |  7 +++
> > >   drivers/scsi/scsi_scan.c  |  2 +
> > >   drivers/scsi/scsi_sysfs.c |  4 +-
> > >   include/scsi/scsi_alua.h  | 14 ++++++
> > >   6 files changed, 126 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/scsi/scsi_alua.c b/drivers/scsi/scsi_alua.c
> > > index d3fcd887e5018..ee0229b1a9d12 100644
> > > --- a/drivers/scsi/scsi_alua.c
> > > +++ b/drivers/scsi/scsi_alua.c
> > > @@ -562,6 +562,90 @@ int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize)
> > >   }
> > >   EXPORT_SYMBOL_GPL(scsi_alua_stpg_run);
> > > +enum scsi_disposition scsi_alua_check_sense(struct scsi_device *sdev,
> > > +					      struct scsi_sense_hdr *sense_hdr)
> > 
> > This seems like it should be shareable with scsi_dh_alua as well.  In
> > might need to take a function to call for rescanning and have
> > alua_check_sense() be a wrapper around it, but since the force argument
> > to alua_check() is now always set to true in scsi_dh_alua, it's
> > unnecessary, so both it and scsi_device_alua_rescan() can have the
> > same arguments.
> 
> Yeah, I tried it and I just thought that adding the rescan callback was a
> bit messy. I can go with the single function if we think it's better.

I would defer to the opinion of an acutal SCSI maintainer (which I am
not) on this.

> 
> > 
> > > +{
> > > +	switch (sense_hdr->sense_key) {
> > > +	case NOT_READY:
> > > +		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0a) {
> > > +			/*
> > > +			 * LUN Not Accessible - ALUA state transition
> > > +			 */
> > > +			scsi_alua_handle_state_transition(sdev);
> > > +			return NEEDS_RETRY;
> > > +		}
> > > +		break;
> > > +	case UNIT_ATTENTION:
> > > +		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0a) {
> > > +			/*
> > > +			 * LUN Not Accessible - ALUA state transition
> > > +			 */
> > > +			scsi_alua_handle_state_transition(sdev);
> > > +			return NEEDS_RETRY;
> > > +		}
> > > +		if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00) {
> > > +			/*
> > > +			 * Power On, Reset, or Bus Device Reset.
> > > +			 * Might have obscured a state transition,
> > > +			 * so schedule a recheck.
> > > +			 */
> > > +			scsi_device_alua_rescan(sdev);
> > > +			return ADD_TO_MLQUEUE;
> > > +		}
> > > +		if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x04)
> > > +			/*
> > > +			 * Device internal reset
> > > +			 */
> > > +			return ADD_TO_MLQUEUE;
> > > +		if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x01)
> > > +			/*
> > > +			 * Mode Parameters Changed
> > > +			 */
> > > +			return ADD_TO_MLQUEUE;
> > > +		if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x06) {
> > > +			/*
> > > +			 * ALUA state changed
> > > +			 */
> > > +			scsi_device_alua_rescan(sdev);
> > > +			return ADD_TO_MLQUEUE;
> > > +		}
> > > +		if (sense_hdr->asc == 0x2a && sense_hdr->ascq == 0x07) {
> > > +			/*
> > > +			 * Implicit ALUA state transition failed
> > > +			 */
> > > +			scsi_device_alua_rescan(sdev);
> > > +			return ADD_TO_MLQUEUE;
> > > +		}
> > > +		if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x03)
> > > +			/*
> > > +			 * Inquiry data has changed
> > > +			 */
> > > +			return ADD_TO_MLQUEUE;
> > > +		if (sense_hdr->asc == 0x3f && sense_hdr->ascq == 0x0e)
> > > +			/*
> > > +			 * REPORTED_LUNS_DATA_HAS_CHANGED is reported
> > > +			 * when switching controllers on targets like
> > > +			 * Intel Multi-Flex. We can just retry.
> > > +			 */
> > > +			return ADD_TO_MLQUEUE;
> > > +		break;
> > > +	}
> > > +
> > > +	return SCSI_RETURN_NOT_HANDLED;
> > > +}
> > > +
> > > +static void alua_rtpg_work(struct work_struct *work)
> > > +{
> > > +	struct alua_data *alua =
> > > +		container_of(work, struct alua_data, work.work);
> > > +	int ret;
> > > +
> > > +	ret = scsi_alua_rtpg_run(alua->sdev);
> > > +
> > > +	if (ret == -EAGAIN)
> > > +		queue_delayed_work(kalua_wq, &alua->work, alua->interval * HZ);
> > > +}
> > > +
> > >   int scsi_alua_sdev_init(struct scsi_device *sdev)
> > >   {
> > >   	int rel_port, ret, tpgs;
> > > @@ -591,6 +675,7 @@ int scsi_alua_sdev_init(struct scsi_device *sdev)
> > >   		goto out_free_data;
> > >   	}
> > > +	INIT_DELAYED_WORK(&sdev->alua->work, alua_rtpg_work);
> > >   	sdev->alua->sdev = sdev;
> > >   	sdev->alua->tpgs = tpgs;
> > >   	spin_lock_init(&sdev->alua->lock);
> > > @@ -638,6 +723,14 @@ bool scsi_device_alua_implicit(struct scsi_device *sdev)
> > >   	return sdev->alua->tpgs & TPGS_MODE_IMPLICIT;
> > >   }
> > > +void scsi_device_alua_rescan(struct scsi_device *sdev)
> > > +{
> > > +	struct alua_data *alua = sdev->alua;
> > > +
> > > +	queue_delayed_work(kalua_wq, &alua->work,
> > > +				msecs_to_jiffies(ALUA_RTPG_DELAY_MSECS));
> > 
> > This code doesn't support triggering a new rtpg while the current one is
> > running.  I'll leave it to people with more scsi expertise to say how
> > important that is, but the scsi_dh_alua code now will always trigger a
> > new rtpg in this case (or at least it would, with the issues from patch
> > 12 fixed).
> > 
> 
> If the work is running and we call queue_delayed_work() on the same
> work_struct, then it is enqueued again. If the work is pending and we call
> queue_delayed_work(), then it is not requeued (as it is already queued).

Oops. You're correct.

-Ben

> Thanks,
> John