scsi: Core ALUA driver

[PATCH 07/13] scsi: alua: Add scsi_alua_stpg_run()

Posted by John Garry 2 weeks, 6 days ago

Add a function to run stpg and handle error codes - it does equivalent
handling as in alua_rtpg_work() from scsi_dh_alua.c

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/scsi/scsi_alua.c | 20 +++++++++++++++++++-
 include/scsi/scsi_alua.h |  5 +++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_alua.c b/drivers/scsi/scsi_alua.c
index e4cb43ba645fa..4e20a537a4ad6 100644
--- a/drivers/scsi/scsi_alua.c
+++ b/drivers/scsi/scsi_alua.c
@@ -428,7 +428,6 @@ EXPORT_SYMBOL_GPL(scsi_alua_rtpg_run);
  * a re-evaluation of the target group state or SCSI_DH_OK
  * if no further action needs to be taken.
  */
-__maybe_unused
 static int scsi_alua_stpg(struct scsi_device *sdev, bool optimize)
 {
 	struct alua_data *alua = sdev->alua;
@@ -480,6 +479,25 @@ static int scsi_alua_stpg(struct scsi_device *sdev, bool optimize)
 	return -EAGAIN;//SCSI_DH_RETRY;
 }
 
+int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize)
+{
+	struct alua_data *alua = sdev->alua;
+	unsigned long flags;
+	int err;
+
+	err = scsi_alua_stpg(sdev, optimize);
+	spin_lock_irqsave(&alua->lock, flags);
+	if (err == EAGAIN) {
+		alua->interval = 0;
+		spin_unlock_irqrestore(&alua->lock, flags);
+		return -EAGAIN;
+	}
+	spin_unlock_irqrestore(&alua->lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(scsi_alua_stpg_run);
+
 int scsi_alua_sdev_init(struct scsi_device *sdev)
 {
 	int rel_port, ret, tpgs;
diff --git a/include/scsi/scsi_alua.h b/include/scsi/scsi_alua.h
index 1eb5481f40bd4..6e4f262bbfbc0 100644
--- a/include/scsi/scsi_alua.h
+++ b/include/scsi/scsi_alua.h
@@ -31,6 +31,7 @@ int scsi_alua_sdev_init(struct scsi_device *sdev);
 void scsi_alua_sdev_exit(struct scsi_device *sdev);
 
 int scsi_alua_rtpg_run(struct scsi_device *sdev);
+int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize);
 
 int scsi_alua_init(void);
 void scsi_exit_alua(void);
@@ -40,6 +41,10 @@ static inline int scsi_alua_rtpg_run(struct scsi_device *sdev)
 {
 	return 0;
 }
+static inline int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize)
+{
+	return 0;
+}
 static inline int scsi_alua_sdev_init(struct scsi_device *sdev)
 {
 	return 0;
-- 
2.43.5

Re: [PATCH 07/13] scsi: alua: Add scsi_alua_stpg_run()

Posted by Hannes Reinecke 2 weeks, 5 days ago

On 3/17/26 13:06, John Garry wrote:
> Add a function to run stpg and handle error codes - it does equivalent
> handling as in alua_rtpg_work() from scsi_dh_alua.c
> 
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>   drivers/scsi/scsi_alua.c | 20 +++++++++++++++++++-
>   include/scsi/scsi_alua.h |  5 +++++
>   2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_alua.c b/drivers/scsi/scsi_alua.c
> index e4cb43ba645fa..4e20a537a4ad6 100644
> --- a/drivers/scsi/scsi_alua.c
> +++ b/drivers/scsi/scsi_alua.c
> @@ -428,7 +428,6 @@ EXPORT_SYMBOL_GPL(scsi_alua_rtpg_run);
>    * a re-evaluation of the target group state or SCSI_DH_OK
>    * if no further action needs to be taken.
>    */
> -__maybe_unused
>   static int scsi_alua_stpg(struct scsi_device *sdev, bool optimize)
>   {
>   	struct alua_data *alua = sdev->alua;
> @@ -480,6 +479,25 @@ static int scsi_alua_stpg(struct scsi_device *sdev, bool optimize)
>   	return -EAGAIN;//SCSI_DH_RETRY;
>   }
>   
> +int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize)
> +{
> +	struct alua_data *alua = sdev->alua;
> +	unsigned long flags;
> +	int err;
> +
> +	err = scsi_alua_stpg(sdev, optimize);
> +	spin_lock_irqsave(&alua->lock, flags);
> +	if (err == EAGAIN) {
> +		alua->interval = 0;
> +		spin_unlock_irqrestore(&alua->lock, flags);
> +		return -EAGAIN;
> +	}
> +	spin_unlock_irqrestore(&alua->lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(scsi_alua_stpg_run);
> +
>   int scsi_alua_sdev_init(struct scsi_device *sdev)
>   {
>   	int rel_port, ret, tpgs;
> diff --git a/include/scsi/scsi_alua.h b/include/scsi/scsi_alua.h
> index 1eb5481f40bd4..6e4f262bbfbc0 100644
> --- a/include/scsi/scsi_alua.h
> +++ b/include/scsi/scsi_alua.h
> @@ -31,6 +31,7 @@ int scsi_alua_sdev_init(struct scsi_device *sdev);
>   void scsi_alua_sdev_exit(struct scsi_device *sdev);
>   
>   int scsi_alua_rtpg_run(struct scsi_device *sdev);
> +int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize);
>   
>   int scsi_alua_init(void);
>   void scsi_exit_alua(void);
> @@ -40,6 +41,10 @@ static inline int scsi_alua_rtpg_run(struct scsi_device *sdev)
>   {
>   	return 0;
>   }
> +static inline int scsi_alua_stpg_run(struct scsi_device *sdev, bool optimize)
> +{
> +	return 0;
> +}
>   static inline int scsi_alua_sdev_init(struct scsi_device *sdev)
>   {
>   	return 0;

No. STPG handling should be done in scsi_dh_alua _only_. We really
should not attempt this in the scsi core.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 07/13] scsi: alua: Add scsi_alua_stpg_run()

Posted by John Garry 2 weeks, 5 days ago

On 18/03/2026 07:57, Hannes Reinecke wrote:
>> +static inline int scsi_alua_stpg_run(struct scsi_device *sdev, bool 
>> optimize)
>> +{
>> +    return 0;
>> +}
>>   static inline int scsi_alua_sdev_init(struct scsi_device *sdev)
>>   {
>>       return 0;
> 
> No. STPG handling should be done in scsi_dh_alua _only_. We really
> should not attempt this in the scsi core.

It's not so nice to have the functionality spread out. The way I see it 
is that drivers/scsi/scsi_alua.c is mostly a library, but also has 
functionality to "drive" ALUA for native SCSI multipathing.

Anyway, can you confirm which of the following do you think from this 
series should be in scsi_dh_alua.c:

- scsi_alua_stpg_run()
- scsi_alua_stpg()
- submit_stpg()

You already said scsi_alua_stpg_run() should be.

Thanks,
John

Re: [PATCH 07/13] scsi: alua: Add scsi_alua_stpg_run()

Posted by Hannes Reinecke 2 weeks, 5 days ago

On 3/18/26 09:59, John Garry wrote:
> On 18/03/2026 07:57, Hannes Reinecke wrote:
>>> +static inline int scsi_alua_stpg_run(struct scsi_device *sdev, bool 
>>> optimize)
>>> +{
>>> +    return 0;
>>> +}
>>>   static inline int scsi_alua_sdev_init(struct scsi_device *sdev)
>>>   {
>>>       return 0;
>>
>> No. STPG handling should be done in scsi_dh_alua _only_. We really
>> should not attempt this in the scsi core.
> 
> It's not so nice to have the functionality spread out. The way I see it 
> is that drivers/scsi/scsi_alua.c is mostly a library, but also has 
> functionality to "drive" ALUA for native SCSI multipathing.
> 
> Anyway, can you confirm which of the following do you think from this 
> series should be in scsi_dh_alua.c:
> 
> - scsi_alua_stpg_run()
> - scsi_alua_stpg()
> - submit_stpg()
> 
> You already said scsi_alua_stpg_run() should be.
> 
Gnaa. Misread that one (blame lack of coffee).
stpg should be handled in scsi_dh_alua. Arguable
we could move the utility functions (submit_stpg
and maybe scsi_alua_stpg) in the core alua code,
but scsi_alua_stpg_run() should be kept in
scsi_dh_alua.

If that makes sense ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.com                               +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 07/13] scsi: alua: Add scsi_alua_stpg_run()

Posted by John Garry 2 weeks ago

On 18/03/2026 09:24, Hannes Reinecke wrote:
>>
>> It's not so nice to have the functionality spread out. The way I see 
>> it is that drivers/scsi/scsi_alua.c is mostly a library, but also has 
>> functionality to "drive" ALUA for native SCSI multipathing.
>>
>> Anyway, can you confirm which of the following do you think from this 
>> series should be in scsi_dh_alua.c:
>>
>> - scsi_alua_stpg_run()
>> - scsi_alua_stpg()
>> - submit_stpg()
>>
>> You already said scsi_alua_stpg_run() should be.
>>
> Gnaa. Misread that one (blame lack of coffee).
> stpg should be handled in scsi_dh_alua. Arguable
> we could move the utility functions (submit_stpg
> and maybe scsi_alua_stpg) in the core alua code,
> but scsi_alua_stpg_run() should be kept in
> scsi_dh_alua.
> 
> If that makes sense ...

scsi_alua_stpg_run() hardly does anything - scsi_alua_stpg() has the 
bulk of the functionality. I think that it's nicer to co-locate this 
functionality (with the rtpg code), as when we separate we have 
different code read/writing alua_data structure.

However, I have been hearing that SCSI core code only needs implicit 
support, so I can try that (which is keep everything STPG in scsi_dh_alua.c)

Thanks,
John