Re: [PATCH v2] scsi: core: Rework the DEF_SCSI_QCMD() implementation

[John Garry <john.g.garry@oracle.com>]

On 31/08/2026 23:19, Bart Van Assche wrote:
> In DEF_SCSI_QCMD(), make the following changes:
>    - Assign cmd->device->host to shost because func_name##_lck()
>      implementations will be annotated with
>      __must_hold(&cmd->device->host->host_lock). This change prepares for

Why not annotate with __must_hold(&host->host_lock)? That is what we do 
in scsi_host_set_state().

>      enabling lock context annotations for SCSI drivers.
>    - Do not save and restore the IRQ flags. Since support for the legacy
>      block layer has been removed, .queue_rq() implementations are always
>      called from thread context and with interrupts enabled. Hence, saving
>      and restoring the interrupt state is not necessary.

That is really a separate change.

> 
> Signed-off-by: Bart Van Assche<bvanassche@acm.org>

It would help the maintainer to mention that this conflicts with my 
host_lock patch

> ---
> 
> Changes compared to v1:
>    - Fixed a use-after-free bug.
>    - Added __maybe_unused to the shost argument.
> 
>    include/scsi/scsi_host.h | 15 +++++++++++----
>    1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 16243ec376cd..d62f1e1fbd0c 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -528,15 +528,22 @@ struct scsi_host_template {
>     *
>     */
>    #define DEF_SCSI_QCMD(func_name) \
> -	enum scsi_qc_status func_name(struct Scsi_Host *shost,		\
> +	enum scsi_qc_status func_name(struct Scsi_Host *shost		\
> +				      __maybe_unused,			\
>    				      struct scsi_cmnd *cmd)		\
>    	{								\
> -		unsigned long irq_flags;				\
>    		enum scsi_qc_status rc;					\
>    									\
> -		spin_lock_irqsave(shost->host_lock, irq_flags);		\
> +		/*							\
> +		 * Help compiler context analysis by using		\
> +		 * cmd->device->host instead of the shost argument	\
> +		 * for the func_name##_lck() implementations annotated	\
> +		 * with __must_hold(cmd->device->host->host_lock).	\
> +		 */							\
> +		shost = cmd->device->host;				\
> +		spin_lock_irq(shost->host_lock);			\
>    		rc = func_name##_lck(cmd);				\
> -		spin_unlock_irqrestore(shost->host_lock, irq_flags);	\
> +		spin_unlock_irq(shost->host_lock);			\
>    		return rc;						\
>    	}
>