[PATCH 29/62] fnic: Make fnic_queuecommand() easier to analyze

[PATCH 29/62] fnic: Make fnic_queuecommand() easier to analyze

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

Move a spin_unlock_irqrestore() call such that the io_lock_acquired
variable can be eliminated. This patch prepares for enabling the Clang
thread-safety analyzer.

Cc: Satish Kharat <satishkh@cisco.com>
Cc: Sesidhar Baddela <sebaddel@cisco.com>
Cc: Karan Tilak Kumar <kartilak@cisco.com>
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/fnic/fnic_scsi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 29d7aca06958..f47c92dfbfd0 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -471,7 +471,6 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
 	int sg_count = 0;
 	unsigned long flags = 0;
 	unsigned long ptr;
-	int io_lock_acquired = 0;
 	uint16_t hwq = 0;
 	struct fnic_tport_s *tport = NULL;
 	struct rport_dd_data_s *rdd_data;
@@ -636,7 +635,6 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
 	spin_lock_irqsave(&fnic->wq_copy_lock[hwq], flags);
 
 	/* initialize rest of io_req */
-	io_lock_acquired = 1;
 	io_req->port_id = rport->port_id;
 	io_req->start_time = jiffies;
 	fnic_priv(sc)->state = FNIC_IOREQ_CMD_PENDING;
@@ -689,6 +687,9 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
 		/* REVISIT: Use per IO lock in the final code */
 		fnic_priv(sc)->flags |= FNIC_IO_ISSUED;
 	}
+
+	spin_unlock_irqrestore(&fnic->wq_copy_lock[hwq], flags);
+
 out:
 	cmd_trace = ((u64)sc->cmnd[0] << 56 | (u64)sc->cmnd[7] << 40 |
 			(u64)sc->cmnd[8] << 32 | (u64)sc->cmnd[2] << 24 |
@@ -699,10 +700,6 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
 		   mqtag, sc, io_req, sg_count, cmd_trace,
 		   fnic_flags_and_state(sc));
 
-	/* if only we issued IO, will we have the io lock */
-	if (io_lock_acquired)
-		spin_unlock_irqrestore(&fnic->wq_copy_lock[hwq], flags);
-
 	atomic_dec(&fnic->in_flight);
 	atomic_dec(&tport->in_flight);
 

RE: [PATCH 29/62] fnic: Make fnic_queuecommand() easier to analyze

[Karan Tilak Kumar (kartilak)]

On Monday, February 23, 2026 2:00 PM, Bart Van Assche <bart.vanassche@linux.dev> wrote:
>
>
> Move a spin_unlock_irqrestore() call such that the io_lock_acquired
> variable can be eliminated. This patch prepares for enabling the Clang
> thread-safety analyzer.
>
> Cc: Satish Kharat <satishkh@cisco.com>
> Cc: Sesidhar Baddela <sebaddel@cisco.com>
> Cc: Karan Tilak Kumar <kartilak@cisco.com>
> Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: linux-scsi@vger.kernel.org
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/fnic/fnic_scsi.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
> index 29d7aca06958..f47c92dfbfd0 100644
> --- a/drivers/scsi/fnic/fnic_scsi.c
> +++ b/drivers/scsi/fnic/fnic_scsi.c
> @@ -471,7 +471,6 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
>       int sg_count = 0;
>       unsigned long flags = 0;
>       unsigned long ptr;
> -     int io_lock_acquired = 0;
>       uint16_t hwq = 0;
>       struct fnic_tport_s *tport = NULL;
>       struct rport_dd_data_s *rdd_data;
> @@ -636,7 +635,6 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
>       spin_lock_irqsave(&fnic->wq_copy_lock[hwq], flags);
>
>       /* initialize rest of io_req */
> -     io_lock_acquired = 1;
>       io_req->port_id = rport->port_id;
>       io_req->start_time = jiffies;
>       fnic_priv(sc)->state = FNIC_IOREQ_CMD_PENDING;
> @@ -689,6 +687,9 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
>               /* REVISIT: Use per IO lock in the final code */
>               fnic_priv(sc)->flags |= FNIC_IO_ISSUED;
>       }
> +
> +     spin_unlock_irqrestore(&fnic->wq_copy_lock[hwq], flags);
> +
>  out:
>       cmd_trace = ((u64)sc->cmnd[0] << 56 | (u64)sc->cmnd[7] << 40 |
>                       (u64)sc->cmnd[8] << 32 | (u64)sc->cmnd[2] << 24 |
> @@ -699,10 +700,6 @@ enum scsi_qc_status fnic_queuecommand(struct Scsi_Host *shost,
>                  mqtag, sc, io_req, sg_count, cmd_trace,
>                  fnic_flags_and_state(sc));
>
> -     /* if only we issued IO, will we have the io lock */
> -     if (io_lock_acquired)
> -             spin_unlock_irqrestore(&fnic->wq_copy_lock[hwq], flags);
> -
>       atomic_dec(&fnic->in_flight);
>       atomic_dec(&tport->in_flight);
>
>

Thanks for this change, Bart.

Reviewed-by: Karan Tilak Kumar <kartilak@cisco.com>

Regards,
Karan

Re: [PATCH 29/62] fnic: Make fnic_queuecommand() easier to analyze

[Martin K. Petersen]

Bart,

> Move a spin_unlock_irqrestore() call such that the io_lock_acquired
> variable can be eliminated. This patch prepares for enabling the Clang
> thread-safety analyzer.

Applied to 7.1/scsi-staging, thanks!

-- 
Martin K. Petersen

[PATCH 30/62] megaraid: Protect more code with instance->reset_mutex

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

megasas_get_device_list() and megasas_get_snapdump_properties() may
unlock instance->reset_mutex indirectly. Hence, hold reset_mutex while
calling these functions.

Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Sumit Saxena <sumit.saxena@broadcom.com>
Cc: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
Cc: Chandrakanth patil <chandrakanth.patil@broadcom.com>
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: megaraidlinux.pdl@broadcom.com
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index ac71ea4898b2..ecd365d78ae3 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -6365,11 +6365,13 @@ static int megasas_init_fw(struct megasas_instance *instance)
 
 	megasas_setup_jbod_map(instance);
 
-	if (megasas_get_device_list(instance) != SUCCESS) {
-		dev_err(&instance->pdev->dev,
-			"%s: megasas_get_device_list failed\n",
-			__func__);
-		goto fail_get_ld_pd_list;
+	scoped_guard(mutex, &instance->reset_mutex) {
+		if (megasas_get_device_list(instance) != SUCCESS) {
+			dev_err(&instance->pdev->dev,
+				"%s: megasas_get_device_list failed\n",
+				__func__);
+			goto fail_get_ld_pd_list;
+		}
 	}
 
 	/* stream detection initialization */
@@ -6468,7 +6470,8 @@ static int megasas_init_fw(struct megasas_instance *instance)
 	}
 
 	if (instance->snapdump_wait_time) {
-		megasas_get_snapdump_properties(instance);
+		scoped_guard(mutex, &instance->reset_mutex)
+			megasas_get_snapdump_properties(instance);
 		dev_info(&instance->pdev->dev, "Snap dump wait time\t: %d\n",
 			 instance->snapdump_wait_time);
 	}