Skip to contentSkip to conversation

Enable lock context analysis in the SCSI core and UFS driver

PATCH Superseded Bart Van Assche 5 participants 19 messages Jul 31 – Aug 4

Subsystems (6)

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Bart Van Assche

author reviewer
#0 lore raw

v4 cover Enable lock context analysis in the SCSI core and UFS driver

Hi Martin,

This patch series enables lock context analysis for the SCSI core and the UFS driver. The advantages are as follows:
- The compiler (only Clang) verifies whether the lock and unlock calls match what has been declared via __must_hold(), __acquires() or __releases(). This is useful for catching locking bugs in error paths.
- Support for __guarded_by() is enabled. If a member variable is annotated with __guarded_by(lock), the compiler will issue a warning if that member variable is accessed without holding 'lock'.

Additionally, a patch is included that suppresses KCSAN complaints about SCSI host state changes.

More information about lock context analysis is available in the cover letter of [PATCH v5 00/36] Compiler-Based Context- and Locking-Analysis
(https://lore.kernel.org/lkml/[email protected]/).

Please consider this patch series for the next merge window.

Thanks,

Bart.

Changes compared to v3:
- Left out all changes except those for the SCSI core and the UFS driver.

Changes compared to v2:
- Added patch "scsi: core: Pass the SCSI host pointer directly".
- Corrected several annotations that had missing or extra ampersands (&).
- Reduced the number of __assume_ctx_lock() statements.

Changes compared to v1:
- Split patch "Enable lock context analysis" into one patch per driver.

Bart Van Assche (6):
ufs: core: Set task state before io_schedule_timeout()
ufs: core: Enable lock context analysis
scsi: core: Pass the SCSI host pointer directly
scsi: core: Add lock context annotations
scsi: core: Protect host state changes with the host lock
scsi: core: Enable lock context analysis

 drivers/scsi/Makefile                     | 22 ++++++++++
 drivers/scsi/device_handler/Makefile      |  3 ++
 drivers/scsi/hosts.c                      | 16 ++++---
 drivers/scsi/megaraid/megaraid_sas_base.c |  2 +-
 drivers/scsi/mpi3mr/mpi3mr_os.c           |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c      |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c             |  6 +--
 drivers/scsi/scsi_lib.c                   |  3 +-
 drivers/scsi/scsi_scan.c                  | 52 ++++++++++++++---------
 drivers/scsi/scsi_sysfs.c                 |  7 +--
 drivers/ufs/core/Makefile                 |  2 +
 drivers/ufs/core/ufs-debugfs.c            |  8 +++-
 drivers/ufs/core/ufshcd.c                 | 15 +++++++
 drivers/ufs/host/Makefile                 |  2 +
 include/scsi/scsi_host.h                  | 23 +++++++---
 15 files changed, 116 insertions(+), 49 deletions(-)
author reviewer
#1 lore raw

PATCH v4 1/6 ufs: core: Set task state before io_schedule_timeout()

Set the task state to TASK_UNINTERRUPTIBLE before calling io_schedule_timeout() in ufshcd_wait_for_pending_cmds(). Without setting the task state, io_schedule_timeout() returns immediately because the task state remains TASK_RUNNING. This results in a busy loop that wastes CPU cycles.

Fixes 2000bc309703 ("scsi: ufs: core: Reduce the clock scaling latency")
---
 drivers/ufs/core/ufshcd.c | 1 +
 1 file changed, 1 insertion(+)
drivers/ufs/core/ufshcd.c +1 -0
index 1a5a851d43b2..5f99aad26d57 100644
@@ -1317,6 +1317,7 @@ static int ufshcd_wait_for_pending_cmds(struct ufs_hba *hba,
 			break;
 		}
 
+		__set_current_state(TASK_UNINTERRUPTIBLE);
 		io_schedule_timeout(msecs_to_jiffies(20));
 		if (ktime_to_us(ktime_sub(ktime_get(), start)) >
 		    wait_timeout_us) {
author reviewer
#2 lore raw

PATCH v4 2/6 ufs: core: Enable lock context analysis

Annotate functions that modify the state of a synchronization object. Remove the struct semaphore annotations because lock context annotations are not supported for semaphores.

---
 drivers/ufs/core/Makefile      |  2 ++
 drivers/ufs/core/ufs-debugfs.c |  8 ++++++--
 drivers/ufs/core/ufshcd.c      | 14 ++++++++++++++
 drivers/ufs/host/Makefile      |  2 ++
 4 files changed, 24 insertions(+), 2 deletions(-)
drivers/ufs/core/Makefile +2 -0
index ce7d16d2cf35..67ab9ffbdf5d 100644
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_SCSI_UFSHCD)		+= ufshcd-core.o
 ufshcd-core-y				+= ufshcd.o ufs-sysfs.o ufs-mcq.o ufs-txeq.o
 ufshcd-core-$(CONFIG_RPMB)		+= ufs-rpmb.o
drivers/ufs/core/ufs-debugfs.c +6 -2
index e3dd81d6fe82..814816810388 100644
@@ -65,8 +65,10 @@ static int ee_usr_mask_get(void *data, u64 *val)
 	return 0;
 }
 
+token_context_lock(ufs_debugfs);
+
 static int ufs_debugfs_get_user_access(struct ufs_hba *hba)
-__acquires(&hba->host_sem)
+	__cond_acquires(0, ufs_debugfs)
 {
 	down(&hba->host_sem);
 	if (!ufshcd_is_user_access_allowed(hba)) {
@@ -74,14 +76,16 @@ __acquires(&hba->host_sem)
 		return -EBUSY;
 	}
 	ufshcd_rpm_get_sync(hba);
+	__acquire(ufs_debugfs);
 	return 0;
 }
 
 static void ufs_debugfs_put_user_access(struct ufs_hba *hba)
-__releases(&hba->host_sem)
+	__releases(ufs_debugfs)
 {
 	ufshcd_rpm_put_sync(hba);
 	up(&hba->host_sem);
+	__release(ufs_debugfs);
 }
 
 static int ee_usr_mask_set(void *data, u64 val)
drivers/ufs/core/ufshcd.c +14 -0
index 5f99aad26d57..b34e2da4c320 100644
@@ -1354,6 +1354,8 @@ static int ufshcd_wait_for_pending_cmds(struct ufs_hba *hba,
  * On failure, all acquired locks are released and the tagset is unquiesced.
  */
 int ufshcd_pause_command_processing(struct ufs_hba *hba, u64 timeout_us)
+	__cond_acquires(0, &hba->host->scan_mutex)
+	__cond_acquires(0, &hba->clk_scaling_lock)
 {
 	int ret = 0;
 
@@ -1378,6 +1380,8 @@ int ufshcd_pause_command_processing(struct ufs_hba *hba, u64 timeout_us)
  * This function resumes command submissions.
  */
 void ufshcd_resume_command_processing(struct ufs_hba *hba)
+	__releases(&hba->clk_scaling_lock)
+	__releases(&hba->host->scan_mutex)
 {
 	up_write(&hba->clk_scaling_lock);
 	blk_mq_unquiesce_tagset(&hba->host->tag_set);
@@ -1447,6 +1451,9 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, u32 target_gear, bool scale_up
  * Return: 0 upon success; -EBUSY upon timeout.
  */
 static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
+	__cond_acquires(0, &hba->host->scan_mutex)
+	__cond_acquires(0, &hba->wb_mutex)
+	__cond_acquires(0, &hba->clk_scaling_lock)
 {
 	int ret = 0;
 	/*
@@ -1476,6 +1483,9 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
 }
 
 static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err)
+	__releases(&hba->clk_scaling_lock)
+	__releases(&hba->wb_mutex)
+	__releases(&hba->host->scan_mutex)
 {
 	up_write(&hba->clk_scaling_lock);
 	mutex_unlock(&hba->wb_mutex);
@@ -3283,6 +3293,8 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 }
 
 static void ufshcd_dev_man_lock(struct ufs_hba *hba)
+	__acquires(&hba->dev_cmd.lock)
+	__acquires_shared(&hba->clk_scaling_lock)
 {
 	ufshcd_hold(hba);
 	mutex_lock(&hba->dev_cmd.lock);
@@ -3290,6 +3302,8 @@ static void ufshcd_dev_man_lock(struct ufs_hba *hba)
 }
 
 static void ufshcd_dev_man_unlock(struct ufs_hba *hba)
+	__releases_shared(&hba->clk_scaling_lock)
+	__releases(&hba->dev_cmd.lock)
 {
 	up_read(&hba->clk_scaling_lock);
 	mutex_unlock(&hba->dev_cmd.lock);
drivers/ufs/host/Makefile +2 -0
index 65d8bb23ab7b..7d8db67eb23c 100644
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o tc-dwc-g210.o
 obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-dwc-g210.o
 obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o
author reviewer
#4 lore raw

PATCH v4 3/6 scsi: core: Pass the SCSI host pointer directly

In reply to Bart Van Assche

In the functions scsi_probe_and_add_lun(), scsi_sequential_lun_scan(), scsi_report_lun_scan() and __scsi_scan_target() the SCSI host pointer is derived from the SCSI target pointer. Pass the SCSI host pointer directly. This patch prepares for enabling lock context analysis. No functionality has been changed.

---
 drivers/scsi/scsi_scan.c | 47 ++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 21 deletions(-)
drivers/scsi/scsi_scan.c +26 -21
index e27da038603a..d7e0e9393194 100644
@@ -1169,6 +1169,7 @@ static unsigned char *scsi_inq_str(unsigned char *buf, unsigned char *inq,
 
 /**
  * scsi_probe_and_add_lun - probe a LUN, if a LUN is found add it
+ * @shost:      SCSI host pointer
  * @starget:	pointer to target device structure
  * @lun:	LUN of target device
  * @bflagsp:	store bflags here if not NULL
@@ -1188,7 +1189,8 @@ static unsigned char *scsi_inq_str(unsigned char *buf, unsigned char *inq,
  *         attached at the LUN
  *   - SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
  **/
-static int scsi_probe_and_add_lun(struct scsi_target *starget,
+static int scsi_probe_and_add_lun(struct Scsi_Host *shost,
+				  struct scsi_target *starget,
 				  u64 lun, blist_flags_t *bflagsp,
 				  struct scsi_device **sdevp,
 				  enum scsi_scan_mode rescan,
@@ -1198,7 +1200,6 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 	unsigned char *result;
 	blist_flags_t bflags;
 	int res = SCSI_SCAN_NO_RESPONSE, result_len = 256;
-	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
 
 	/*
 	 * The rescan flag is used as an optimization, the first scan of a
@@ -1334,6 +1335,7 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 
 /**
  * scsi_sequential_lun_scan - sequentially scan a SCSI target
+ * @shost:      SCSI host pointer
  * @starget:	pointer to target structure to scan
  * @bflags:	black/white list flag for LUN 0
  * @scsi_level: Which version of the standard does this device adhere to
@@ -1346,13 +1348,13 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
  *
  *     Modifies sdevscan->lun.
  **/
-static void scsi_sequential_lun_scan(struct scsi_target *starget,
+static void scsi_sequential_lun_scan(struct Scsi_Host *shost,
+				     struct scsi_target *starget,
 				     blist_flags_t bflags, int scsi_level,
 				     enum scsi_scan_mode rescan)
 {
 	uint max_dev_lun;
 	u64 sparse_lun, lun;
-	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
 
 	SCSI_LOG_SCAN_BUS(3, starget_printk(KERN_INFO, starget,
 		"scsi scan: Sequential scan\n"));
@@ -1412,14 +1414,15 @@ static void scsi_sequential_lun_scan(struct scsi_target *starget,
 	 * sparse_lun.
 	 */
 	for (lun = 1; lun < max_dev_lun; ++lun)
-		if ((scsi_probe_and_add_lun(starget, lun, NULL, NULL, rescan,
-					    NULL) != SCSI_SCAN_LUN_PRESENT) &&
+		if (scsi_probe_and_add_lun(shost, starget, lun, NULL, NULL,
+				rescan, NULL) != SCSI_SCAN_LUN_PRESENT &&
 		    !sparse_lun)
 			return;
 }
 
 /**
  * scsi_report_lun_scan - Scan using SCSI REPORT LUN results
+ * @shost: SCSI host pointer
  * @starget: which target
  * @bflags: Zero or a mix of BLIST_NOLUN, BLIST_REPORTLUN2, or BLIST_NOREPORTLUN
  * @rescan: nonzero if we can skip code only needed on first scan
@@ -1438,8 +1441,9 @@ static void scsi_sequential_lun_scan(struct scsi_target *starget,
  *     0: scan completed (or no memory, so further scanning is futile)
  *     1: could not scan with REPORT LUN
  **/
-static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflags,
-				enum scsi_scan_mode rescan)
+static int scsi_report_lun_scan(struct Scsi_Host *shost,
+			struct scsi_target *starget, blist_flags_t bflags,
+			enum scsi_scan_mode rescan)
 {
 	unsigned char scsi_cmd[MAX_COMMAND_SIZE];
 	unsigned int length;
@@ -1448,7 +1452,6 @@ static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflag
 	int result;
 	struct scsi_lun *lunp, *lun_data;
 	struct scsi_device *sdev;
-	struct Scsi_Host *shost = dev_to_shost(&starget->dev);
 	struct scsi_failure failure_defs[] = {
 		{
 			.sense = UNIT_ATTENTION,
@@ -1594,7 +1597,7 @@ static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflag
 		} else {
 			int res;
 
-			res = scsi_probe_and_add_lun(starget,
+			res = scsi_probe_and_add_lun(shost, starget,
 				lun, NULL, NULL, rescan, NULL);
 			if (res == SCSI_SCAN_NO_RESPONSE) {
 				/*
@@ -1641,7 +1644,7 @@ struct scsi_device *__scsi_add_device(struct Scsi_Host *shost, uint channel,
 		scsi_complete_async_scans();
 
 	if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
-		scsi_probe_and_add_lun(starget, lun, NULL, &sdev,
+		scsi_probe_and_add_lun(shost, starget, lun, NULL, &sdev,
 				       SCSI_SCAN_RESCAN, hostdata);
 		scsi_autopm_put_host(shost);
 	}
@@ -1763,10 +1766,10 @@ int scsi_rescan_device(struct scsi_device *sdev)
 }
 EXPORT_SYMBOL(scsi_rescan_device);
 
-static void __scsi_scan_target(struct device *parent, unsigned int channel,
-		unsigned int id, u64 lun, enum scsi_scan_mode rescan)
+static void __scsi_scan_target(struct Scsi_Host *shost, struct device *parent,
+			       unsigned int channel, unsigned int id, u64 lun,
+			       enum scsi_scan_mode rescan)
 {
-	struct Scsi_Host *shost = dev_to_shost(parent);
 	blist_flags_t bflags = 0;
 	int res;
 	struct scsi_target *starget;
@@ -1786,7 +1789,8 @@ static void __scsi_scan_target(struct device *parent, unsigned int channel,
 		/*
 		 * Scan for a specific host/chan/id/lun.
 		 */
-		scsi_probe_and_add_lun(starget, lun, NULL, NULL, rescan, NULL);
+		scsi_probe_and_add_lun(shost, starget, lun, NULL, NULL, rescan,
+				       NULL);
 		goto out_reap;
 	}
 
@@ -1794,14 +1798,15 @@ static void __scsi_scan_target(struct device *parent, unsigned int channel,
 	 * Scan LUN 0, if there is some response, scan further. Ideally, we
 	 * would not configure LUN 0 until all LUNs are scanned.
 	 */
-	res = scsi_probe_and_add_lun(starget, 0, &bflags, NULL, rescan, NULL);
+	res = scsi_probe_and_add_lun(shost, starget, 0, &bflags, NULL, rescan,
+				     NULL);
 	if (res == SCSI_SCAN_LUN_PRESENT || res == SCSI_SCAN_TARGET_PRESENT) {
-		if (scsi_report_lun_scan(starget, bflags, rescan) != 0)
+		if (scsi_report_lun_scan(shost, starget, bflags, rescan) != 0)
 			/*
 			 * The REPORT LUN did not scan the target,
 			 * do a sequential scan.
 			 */
-			scsi_sequential_lun_scan(starget, bflags,
+			scsi_sequential_lun_scan(shost, starget, bflags,
 						 starget->scsi_level, rescan);
 	}
 
@@ -1851,7 +1856,7 @@ void scsi_scan_target(struct device *parent, unsigned int channel,
 		scsi_complete_async_scans();
 
 	if (scsi_host_scan_allowed(shost) && scsi_autopm_get_host(shost) == 0) {
-		__scsi_scan_target(parent, channel, id, lun, rescan);
+		__scsi_scan_target(shost, parent, channel, id, lun, rescan);
 		scsi_autopm_put_host(shost);
 	}
 	mutex_unlock(&shost->scan_mutex);
@@ -1882,11 +1887,11 @@ static void scsi_scan_channel(struct Scsi_Host *shost, unsigned int channel,
 				order_id = shost->max_id - id - 1;
 			else
 				order_id = id;
-			__scsi_scan_target(&shost->shost_gendev, channel,
+			__scsi_scan_target(shost, &shost->shost_gendev, channel,
 					order_id, lun, rescan);
 		}
 	else
-		__scsi_scan_target(&shost->shost_gendev, channel,
+		__scsi_scan_target(shost, &shost->shost_gendev, channel,
 				id, lun, rescan);
 }
 
#5 lore raw

Re: [PATCH v4 3/6] scsi: core: Pass the SCSI host pointer directly

On 31/07/2026 22:52, Bart Van Assche wrote:

"Pass the SCSI host pointer directly" - that's too vague a title.

Quoted 4 lines

In the functions scsi_probe_and_add_lun(), scsi_sequential_lun_scan(), scsi_report_lun_scan() and __scsi_scan_target() the SCSI host pointer is derived from the SCSI target pointer. Pass the SCSI host pointer directly. This patch prepares for enabling lock context analysis.

How?

Quoted 4 lines

No
functionality has been changed.

author reviewer
#6 lore raw

Re: [PATCH v4 3/6] scsi: core: Pass the SCSI host pointer directly

Quoted 3 lines · John Garry

On 31/07/2026 22:52, Bart Van Assche wrote:

"Pass the SCSI host pointer directly" - that's too vague a title.

Huh? This patch passes the SCSI host pointer directly to several functions instead of deriving the SCSI host pointer from the SCSI target pointer (dev_to_shost(starget->dev.parent)). Hence, I think the title is
accurate.

Quoted 6 lines
Quoted 4 lines

In the functions scsi_probe_and_add_lun(), scsi_sequential_lun_scan(), scsi_report_lun_scan() and __scsi_scan_target() the SCSI host pointer is derived from the SCSI target pointer. Pass the SCSI host pointer directly. This patch prepares for enabling lock context analysis.

How?

With this patch, lock context annotations can refer to the SCSI host pointer directly (__must_hold(&shost->scan_mutex)). Without this patch, the following lock context annotation would have to be used instead:

__must_hold(&dev_to_shost(starget->dev.parent)->scan_mutex)

Additionally, in code that locks shost->scan_mutex, the following would have to be added to help the compiler understand that shost == dev_to_shost(starget->dev.parent):

__assume_ctx_lock(&dev_to_shost(starget->dev.parent)->scan_mutex);

Christoph Hellwig made it clear during the LSF/MM/BPF summit that he is doesn't like __assume_ctx_lock() statements being added and also that he prefers to add arguments to functions if that eliminates the need for introducing __assume_ctx_lock() statements. Hence this patch.

Thanks,

Bart.

#7 lore raw

Re: [PATCH v4 3/6] scsi: core: Pass the SCSI host pointer directly

Quoted 6 lines · Bart Van Assche

On 8/3/26 6: 01 AM, John Garry wrote: > On 31/07/2026 22: 52, Bart Van Assche
wrote: > > "Pass the SCSI host pointer directly" - that's too vague a title. Huh? This patch passes the SCSI host pointer directly to several functions instead

On 8/3/26 6:01 AM, John Garry wrote:

Quoted 9 lines
Quoted 3 lines · Bart Van Assche

"Pass the SCSI host pointer directly" - that's too vague a title.

Huh? This patch passes the SCSI host pointer directly to several functions instead of deriving the SCSI host pointer from the SCSI target pointer (dev_to_shost(starget->dev.parent)). Hence, I think the title is accurate.

I wrote vague, as in "Pass the SCSI host pointer directly" to and from what? I would have "Pass the SCSI host pointer to scan-related functions" or similar.

Quoted 17 lines
Quoted 6 lines
Quoted 4 lines

In the functions scsi_probe_and_add_lun(), scsi_sequential_lun_scan(), scsi_report_lun_scan() and __scsi_scan_target() the SCSI host pointer is derived from the SCSI target pointer. Pass the SCSI host pointer directly. This patch prepares for enabling lock context analysis.

How?

With this patch, lock context annotations can refer to the SCSI host pointer directly (__must_hold(&shost->scan_mutex)). Without this patch, the following lock context annotation would have to be used instead:

__must_hold(&dev_to_shost(starget->dev.parent)->scan_mutex)

Additionally, in code that locks shost->scan_mutex, the following would have to be added to help the compiler understand that shost == dev_to_shost(starget->dev.parent):

Quoted 7 lines

__assume_ctx_lock(&dev_to_shost(starget->dev.parent)->scan_mutex);

Christoph Hellwig made it clear during the LSF/MM/BPF summit that he is doesn't like __assume_ctx_lock() statements being added and also that he prefers to add arguments to functions if that eliminates the need for introducing __assume_ctx_lock() statements. Hence this patch.

ok, I get it, but it's better to mention the reason briefly, like "context analysis requires checking on passed argument when using preferred annotation form"

Thanks!

author reviewer
#8 lore raw

Re: [PATCH v4 3/6] scsi: core: Pass the SCSI host pointer directly

Quoted 1 line · John Garry

ok, I get it, but it's better to mention the reason briefly, like "context analysis requires checking on passed argument when using preferred annotation form"

I will do this :-)

Bart.

author reviewer
#9 lore raw

PATCH v4 4/6 scsi: core: Add lock context annotations

In reply to Bart Van Assche

Document which functions expect that shost->scan_mutex is held.

---
 drivers/scsi/scsi_scan.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/scsi/scsi_scan.c +5 -0
index d7e0e9393194..3b82e80e807a 100644
@@ -1195,6 +1195,7 @@ static int scsi_probe_and_add_lun(struct Scsi_Host *shost,
 				  struct scsi_device **sdevp,
 				  enum scsi_scan_mode rescan,
 				  void *hostdata)
+	__must_hold(&shost->scan_mutex)
 {
 	struct scsi_device *sdev;
 	unsigned char *result;
@@ -1352,6 +1353,7 @@ static void scsi_sequential_lun_scan(struct Scsi_Host *shost,
 				     struct scsi_target *starget,
 				     blist_flags_t bflags, int scsi_level,
 				     enum scsi_scan_mode rescan)
+	__must_hold(&shost->scan_mutex)
 {
 	uint max_dev_lun;
 	u64 sparse_lun, lun;
@@ -1444,6 +1446,7 @@ static void scsi_sequential_lun_scan(struct Scsi_Host *shost,
 static int scsi_report_lun_scan(struct Scsi_Host *shost,
 			struct scsi_target *starget, blist_flags_t bflags,
 			enum scsi_scan_mode rescan)
+	__must_hold(&shost->scan_mutex)
 {
 	unsigned char scsi_cmd[MAX_COMMAND_SIZE];
 	unsigned int length;
@@ -1769,6 +1772,7 @@ EXPORT_SYMBOL(scsi_rescan_device);
 static void __scsi_scan_target(struct Scsi_Host *shost, struct device *parent,
 			       unsigned int channel, unsigned int id, u64 lun,
 			       enum scsi_scan_mode rescan)
+	__must_hold(&shost->scan_mutex)
 {
 	blist_flags_t bflags = 0;
 	int res;
@@ -1866,6 +1870,7 @@ EXPORT_SYMBOL(scsi_scan_target);
 static void scsi_scan_channel(struct Scsi_Host *shost, unsigned int channel,
 			      unsigned int id, u64 lun,
 			      enum scsi_scan_mode rescan)
+	__must_hold(&shost->scan_mutex)
 {
 	uint order_id;
 
#10 lore raw

Re: [PATCH v4 4/6] scsi: core: Add lock context annotations

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] An integer overflow in the calculation of the REPORT LUNS response length allows a malicious or malformed device to cause an infinite loop or kernel crash. --

--- Patch [4]: [PATCH v4 4/6] scsi: core: Add lock context annotations --- commit 548e6f9dc80e81c45d6f26ae1feeb609869a4064
Author: Bart Van Assche <[email protected]>

scsi: core: Add lock context annotations

This commit documents which functions expect that shost->scan_mutex is held.

Quoted 4 lines
drivers/scsi/scsi_scan.c +0 -0
index d7e0e93931947..3b82e80e807a7 100644

[ ... ]

drivers/scsi/scsi_scan.c static void scsi_sequential_lun_scan(struct Scsi_Host *shost, · line 1452

Quoted 8 lines
drivers/scsi/scsi_scan.c +1 -0
@@ -1444,6 +1446,7 @@ static void scsi_sequential_lun_scan(struct Scsi_Host *shost,
 static int scsi_report_lun_scan(struct Scsi_Host *shost,
 			struct scsi_target *starget, blist_flags_t bflags,
 			enum scsi_scan_mode rescan)
+	__must_hold(&shost->scan_mutex)
 {
 	unsigned char scsi_cmd[MAX_COMMAND_SIZE];
 	unsigned int length;

[Severity: High]
This isn't a bug introduced by this patch, but while looking at this function, I noticed a potential integer overflow when calculating the response length:

drivers/scsi/scsi_scan.c:scsi_report_lun_scan() {
    ...
	if (get_unaligned_be32(lun_data->scsi_lun) +
	    sizeof(struct scsi_lun) > length) {
		length = get_unaligned_be32(lun_data->scsi_lun) +
			 sizeof(struct scsi_lun);
		kfree(lun_data);
		goto retry;
	}
    ...
}

If a malicious or malformed device returns a large value like 0xFFFFFFFF in the first 4 bytes of its response, can the addition of sizeof(struct scsi_lun) overflow?

On 64-bit systems, would this addition yield a 64-bit value that bypasses the greater-than length check, but then truncate when assigned back to the 32-bit unsigned int length variable? Could this cause an infinite retry loop or an invalid pointer dereference if it truncates to zero?

On 32-bit systems, could the addition directly overflow to a small value, fail the greater-than length check, and proceed to parse out-of-bounds luns from the initially allocated buffer?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/68f3346cab2be570839350831f82b6c9233832ed.1785534721.git.bvanassche@acm.org?part=4
author reviewer
#11 lore raw

PATCH v4 5/6 scsi: core: Protect host state changes with the host lock

In reply to Bart Van Assche

Some but not all SCSI host state changes are protected with the SCSI host lock. Annotate the SCSI host state with __guarded_by(host_lock), protect all SCSI host state changes with the SCSI host lock and use READ_ONCE() for all SCSI host state reads. This patch prevents that KCSAN complains about data races when accessing the SCSI host state.

---
 drivers/scsi/hosts.c                      | 16 +++++++++-------
 drivers/scsi/megaraid/megaraid_sas_base.c |  2 +-
 drivers/scsi/mpi3mr/mpi3mr_os.c           |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c      |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c             |  6 ++----
 drivers/scsi/scsi_lib.c                   |  3 +--
 drivers/scsi/scsi_sysfs.c                 |  7 ++++---
 include/scsi/scsi_host.h                  | 23 ++++++++++++++++-------
 8 files changed, 35 insertions(+), 26 deletions(-)
drivers/scsi/hosts.c +9 -7
index d512080268af..190b1932416b 100644
@@ -145,7 +145,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
 		}
 		break;
 	}
-	shost->shost_state = state;
+	WRITE_ONCE(shost->shost_state, state);
 	return 0;
 
  illegal:
@@ -276,7 +276,8 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 	if (error)
 		goto out_disable_runtime_pm;
 
-	scsi_host_set_state(shost, SHOST_RUNNING);
+	scoped_guard(spinlock_irq, shost->host_lock)
+		scsi_host_set_state(shost, SHOST_RUNNING);
 	get_device(shost->shost_gendev.parent);
 
 	device_enable_async_suspend(&shost->shost_dev);
@@ -350,6 +351,7 @@ EXPORT_SYMBOL(scsi_add_host_with_dma);
 static void scsi_host_dev_release(struct device *dev)
 {
 	struct Scsi_Host *shost = dev_to_shost(dev);
+	enum scsi_host_state host_state = scsi_get_host_state(shost);
 	struct device *parent = dev->parent;
 
 	/* Wait for functions invoked through call_rcu(&scmd->rcu, ...) */
@@ -362,7 +364,7 @@ static void scsi_host_dev_release(struct device *dev)
 	if (shost->work_q)
 		destroy_workqueue(shost->work_q);
 
-	if (shost->shost_state == SHOST_CREATED) {
+	if (host_state == SHOST_CREATED) {
 		/*
 		 * Free the shost_dev device name and remove the proc host dir
 		 * here if scsi_host_{alloc,put}() have been called but neither
@@ -378,7 +380,7 @@ static void scsi_host_dev_release(struct device *dev)
 
 	ida_free(&host_index_ida, shost->host_no);
 
-	if (shost->shost_state != SHOST_CREATED)
+	if (host_state != SHOST_CREATED)
 		put_device(parent);
 	kfree(shost);
 }
@@ -411,8 +413,8 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
 		return NULL;
 
 	shost->host_lock = &shost->default_lock;
-	spin_lock_init(shost->host_lock);
-	shost->shost_state = SHOST_CREATED;
+	scoped_guard(spinlock_init, shost->host_lock)
+		shost->shost_state = SHOST_CREATED;
 	INIT_LIST_HEAD(&shost->__devices);
 	INIT_LIST_HEAD(&shost->__targets);
 	INIT_LIST_HEAD(&shost->eh_abort_list);
@@ -598,7 +600,7 @@ EXPORT_SYMBOL(scsi_host_lookup);
  **/
 struct Scsi_Host *scsi_host_get(struct Scsi_Host *shost)
 {
-	if ((shost->shost_state == SHOST_DEL) ||
+	if (scsi_get_host_state(shost) == SHOST_DEL ||
 		!get_device(&shost->shost_gendev))
 		return NULL;
 	return shost;
drivers/scsi/megaraid/megaraid_sas_base.c +1 -1
index ecd365d78ae3..f0152b043e18 100644
@@ -3072,7 +3072,7 @@ static int megasas_reset_bus_host(struct scsi_cmnd *scmd)
 
 	scmd_printk(KERN_INFO, scmd,
 		"SCSI host state: %d  SCSI host busy: %d  FW outstanding: %d\n",
-		scmd->device->host->shost_state,
+		scsi_get_host_state(scmd->device->host),
 		scsi_host_busy(scmd->device->host),
 		atomic_read(&instance->fw_outstanding));
 	/*
drivers/scsi/mpi3mr/mpi3mr_os.c +1 -1
index 402d1f35d214..f80a21ec161b 100644
@@ -5172,7 +5172,7 @@ static enum scsi_qc_status mpi3mr_qcmd(struct Scsi_Host *shost,
 
 	/* Avoid error handling escalation when device is removed or blocked */
 
-	if (scmd->device->host->shost_state == SHOST_RECOVERY &&
+	if (scsi_get_host_state(scmd->device->host) == SHOST_RECOVERY &&
 		scmd->cmnd[0] == TEST_UNIT_READY &&
 		(stgt_priv_data->dev_removed || (dev_handle == MPI3MR_INVALID_DEV_HANDLE))) {
 		scsi_build_sense(scmd, 0, UNIT_ATTENTION, 0x29, 0x07);
drivers/scsi/mpt3sas/mpt3sas_scsih.c +1 -1
index dea78688cc9b..0e12009a87f6 100644
@@ -5472,7 +5472,7 @@ static enum scsi_qc_status scsih_qcmd(struct Scsi_Host *shost,
 	 * Avoid error handling escallation when device is disconnected
 	 */
 	if (handle == MPT3SAS_INVALID_DEVICE_HANDLE || sas_device_priv_data->block) {
-		if (scmd->device->host->shost_state == SHOST_RECOVERY &&
+		if (scsi_get_host_state(scmd->device->host) == SHOST_RECOVERY &&
 		    scmd->cmnd[0] == TEST_UNIT_READY) {
 			scsi_build_sense(scmd, 0, UNIT_ATTENTION, 0x29, 0x07);
 			scsi_done(scmd);
drivers/scsi/qla4xxx/ql4_os.c +2 -4
index d598ab4126f8..c9d9fc7c81fb 100644
@@ -9411,11 +9411,9 @@ static int qla4xxx_eh_target_reset(struct scsi_cmnd *cmd)
  * This routine finds that if reset host is called in EH
  * scenario or from some application like sg_reset
  **/
-static int qla4xxx_is_eh_active(struct Scsi_Host *shost)
+static bool qla4xxx_is_eh_active(struct Scsi_Host *shost)
 {
-	if (shost->shost_state == SHOST_RECOVERY)
-		return 1;
-	return 0;
+	return scsi_get_host_state(shost) == SHOST_RECOVERY;
 }
 
 /**
drivers/scsi/scsi_lib.c +1 -2
index 22e2e3223440..89d2e5a70e9b 100644
@@ -1661,10 +1661,9 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 		goto done;
 	}
 
-	if (unlikely(host->shost_state == SHOST_DEL)) {
+	if (unlikely(scsi_get_host_state(host) == SHOST_DEL)) {
 		cmd->result = (DID_NO_CONNECT << 16);
 		goto done;
-
 	}
 
 	trace_scsi_dispatch_cmd_start(cmd);
drivers/scsi/scsi_sysfs.c +4 -3
index dfc3559e7e04..9480432f650b 100644
@@ -214,8 +214,9 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
 	if (!state)
 		return -EINVAL;
 
-	if (scsi_host_set_state(shost, state))
-		return -EINVAL;
+	scoped_guard(spinlock_irq, shost->host_lock)
+		if (scsi_host_set_state(shost, state))
+			return -EINVAL;
 	return count;
 }
 
@@ -223,7 +224,7 @@ static ssize_t
 show_shost_state(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
-	const char *name = scsi_host_state_name(shost->shost_state);
+	const char *name = scsi_host_state_name(scsi_get_host_state(shost));
 
 	if (!name)
 		return -EINVAL;
include/scsi/scsi_host.h +16 -7
index 7e2011830ba4..69d432fd8e32 100644
@@ -727,7 +727,7 @@ struct Scsi_Host {
 	unsigned int  irq;
 	
 
-	enum scsi_host_state shost_state;
+	enum scsi_host_state shost_state __guarded_by(host_lock);
 
 	/* ldm bits */
 	struct device		shost_gendev, shost_dev;
@@ -785,11 +785,18 @@ static inline struct Scsi_Host *dev_to_shost(struct device *dev)
 	return container_of(dev, struct Scsi_Host, shost_gendev);
 }
 
+static inline enum scsi_host_state scsi_get_host_state(struct Scsi_Host *shost)
+{
+	return context_unsafe(READ_ONCE(shost->shost_state));
+}
+
 static inline int scsi_host_in_recovery(struct Scsi_Host *shost)
 {
-	return shost->shost_state == SHOST_RECOVERY ||
-		shost->shost_state == SHOST_CANCEL_RECOVERY ||
-		shost->shost_state == SHOST_DEL_RECOVERY ||
+	enum scsi_host_state state = scsi_get_host_state(shost);
+
+	return state == SHOST_RECOVERY ||
+		state == SHOST_CANCEL_RECOVERY ||
+		state == SHOST_DEL_RECOVERY ||
 		shost->tmf_in_progress;
 }
 
@@ -835,8 +842,9 @@ static inline struct device *scsi_get_device(struct Scsi_Host *shost)
  **/
 static inline int scsi_host_scan_allowed(struct Scsi_Host *shost)
 {
-	return shost->shost_state == SHOST_RUNNING ||
-	       shost->shost_state == SHOST_RECOVERY;
+	enum scsi_host_state state = scsi_get_host_state(shost);
+
+	return state == SHOST_RUNNING || state == SHOST_RECOVERY;
 }
 
 extern void scsi_unblock_requests(struct Scsi_Host *);
@@ -940,6 +948,7 @@ static inline unsigned char scsi_host_get_guard(struct Scsi_Host *shost)
 	return shost->prot_guard_type;
 }
 
-extern int scsi_host_set_state(struct Scsi_Host *, enum scsi_host_state);
+int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
+	__must_hold(shost->host_lock);
 
 #endif /* _SCSI_SCSI_HOST_H */
#12 lore raw

Re: [PATCH v4 5/6] scsi: core: Protect host state changes with the host lock

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The commit message claims to use READ_ONCE() for all SCSI host state reads, but…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit message claims to use READ_ONCE() for all SCSI host state reads, but scsi_host_set_state() still performs a plain read. --

--- Patch [5]: [PATCH v4 5/6] scsi: core: Protect host state changes with the host lock --- commit 89fe98af4d7aa30cc00f86bd2b550ec8e90c44ff
Author: Bart Van Assche <[email protected]>

scsi: core: Protect host state changes with the host lock

This commit annotates the SCSI host state with __guarded_by(host_lock) and protects state changes with the host lock. It also updates state accesses to use READ_ONCE() and WRITE_ONCE() to prevent data races.

Quoted 5 lines

Some but not all SCSI host state changes are protected with the SCSI host lock. Annotate the SCSI host state with __guarded_by(host_lock), protect all SCSI host state changes with the SCSI host lock and use READ_ONCE() for all SCSI host state reads. This patch prevents that KCSAN complains about data races when accessing the SCSI host state.

[Severity: Low]
This isn't a bug, but the commit message states that READ_ONCE() is used for all SCSI host state reads. However, looking at scsi_host_set_state() in drivers/scsi/hosts.c, the initial state read still uses a plain access:

int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)

{
	enum scsi_host_state oldstate = shost->shost_state;

While this is safe since the host lock is held here, should this read also be updated to match the commit message, or should the commit message be adjusted to reflect this exception?

Quoted 11 lines
drivers/scsi/hosts.c +1 -1
index 08ec144f7bf77..9ea161478a10a 100644
@@ -145,7 +145,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
 		}
 		break;
 	}
-	shost->shost_state = state;
+	WRITE_ONCE(shost->shost_state, state);
 	return 0;

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/68f3346cab2be570839350831f82b6c9233832ed.1785534721.git.bvanassche@acm.org?part=5
#13 lore raw

Re: [PATCH v4 5/6] scsi: core: Protect host state changes with the host lock

In reply to Bart Van Assche

drivers/scsi/hosts.c struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv · line 416

Quoted 7 lines · Bart Van Assche
drivers/scsi/hosts.c +1 -2
@@ -411,8 +413,8 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
  		return NULL;
 
  	shost->host_lock = &shost->default_lock;
-	spin_lock_init(shost->host_lock);
-	shost->shost_state = SHOST_CREATED;
+	scoped_guard(spinlock_init, shost->host_lock)

Please forgive my ignorance, but what does spinlock_init do here in relation to setting shost->shost_state?

Elsewhere we have scoped_guard(spinlock_irq, shost->host_lock) for protecting setting shost->shost_state and this makes sense to me. However I am curious what scoped_guard(spinlock_init, ...) does.

Quoted 1 line
drivers/scsi/hosts.c +1 -0
+		shost->shost_state = SHOST_CREATED;
author reviewer
#14 lore raw

Re: [PATCH v4 5/6] scsi: core: Protect host state changes with the host lock

Quoted 11 lines · John Garry
Quoted 7 lines · Bart Van Assche
(quoted diff) +0 -0
@@ -411,8 +413,8 @@ struct Scsi_Host *scsi_host_alloc(const struct 

scsi_host_template *sht, int priv
          return NULL;
      shost->host_lock = &shost->default_lock; -    spin_lock_init(shost->host_lock);
-    shost->shost_state = SHOST_CREATED;
+    scoped_guard(spinlock_init, shost->host_lock)

Please forgive my ignorance, but what does spinlock_init do here in relation to setting shost->shost_state?

Quoted 1 line

Elsewhere we have scoped_guard(spinlock_irq, shost->host_lock) for protecting setting shost->shost_state and this makes sense to me. However I am curious what scoped_guard(spinlock_init, ...) does.

Support for scoped_guard(*_init, ...) and guard(*_init)(...) was introduced in January of this year. My understanding of commit d084a73714f8 ("compiler-context-analysis: Introduce scoped init guards") is as follows:
* The referenced synchronization object is initialized. In this case that means calling spinlock_init(shost->host_lock).
* Compiler warnings triggered by __guarded_by() are suppressed for the referenced synchronization object. In this case, the compiler is told not to assume that shost->host_lock is acquired at the start of the guarded code block and released at the end of the guarded code block.

Marco, please correct me if I got this wrong.

Thanks,

Bart.

#15 lore raw

Re: [PATCH v4 5/6] scsi: core: Protect host state changes with the host lock

Quoted 29 lines · Bart Van Assche
Quoted 12 lines · John Garry

drivers/scsi/hosts.c struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv · line 416

Quoted 7 lines · Bart Van Assche
(quoted diff) +0 -0
@@ -411,8 +413,8 @@ struct Scsi_Host *scsi_host_alloc(const struct

scsi_host_template *sht, int priv

          return NULL;
      shost->host_lock = &shost->default_lock;
drivers/scsi/hosts.c +1 -2
-    spin_lock_init(shost->host_lock);
-    shost->shost_state = SHOST_CREATED;
+    scoped_guard(spinlock_init, shost->host_lock)

Please forgive my ignorance, but what does spinlock_init do here in relation to setting shost->shost_state?

Quoted 3 lines

Elsewhere we have scoped_guard(spinlock_irq, shost->host_lock) for protecting setting shost->shost_state and this makes sense to me. However I am curious what scoped_guard(spinlock_init, ...) does.

Support for scoped_guard(*_init, ...) and guard(*_init)(...) was introduced in January of this year. My understanding of commit d084a73714f8 ("compiler-context-analysis: Introduce scoped init guards") is as follows:
* The referenced synchronization object is initialized. In this case that means calling spinlock_init(shost->host_lock).
* Compiler warnings triggered by __guarded_by() are suppressed for the referenced synchronization object. In this case, the compiler is told not to assume that shost->host_lock is acquired at the start of the guarded code block and released at the end of the guarded code block.

Quoted 1 line

Marco, please correct me if I got this wrong.

Right - documentation also summarizes this:
https://docs.kernel.org/next/dev-tools/context-analysis.html#supported-kernel-primitives

author reviewer
#16 lore raw

PATCH v4 6/6 scsi: core: Enable lock context analysis

In reply to Bart Van Assche

Enable lock context analysis for those SCSI core files that build without triggering any compiler warnings.

---
 drivers/scsi/Makefile                | 22 ++++++++++++++++++++++
 drivers/scsi/device_handler/Makefile |  3 +++
 2 files changed, 25 insertions(+)
drivers/scsi/Makefile +22 -0
index 842c254bb226..69ea80819f3b 100644
@@ -14,6 +14,28 @@
 # satisfy certain initialization assumptions in the SCSI layer.
 # *!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!
 
+CONTEXT_ANALYSIS_constants.o := y
+CONTEXT_ANALYSIS_scsi.o := y
+CONTEXT_ANALYSIS_scsicam.o := y
+CONTEXT_ANALYSIS_common.o := y
+CONTEXT_ANALYSIS_devinfo.o := y
+CONTEXT_ANALYSIS_dh.o := y
+CONTEXT_ANALYSIS_ioctl.o := y
+CONTEXT_ANALYSIS_lib.o := y
+CONTEXT_ANALYSIS_lib_dma.o := y
+CONTEXT_ANALYSIS_error.o := y
+CONTEXT_ANALYSIS_logging.o := y
+CONTEXT_ANALYSIS_netlink.o := y
+CONTEXT_ANALYSIS_pm.o := y
+CONTEXT_ANALYSIS_scan.o := y
+CONTEXT_ANALYSIS_sysfs.o := y
+CONTEXT_ANALYSIS_trace.o := y
+CONTEXT_ANALYSIS_sd.o := y
+CONTEXT_ANALYSIS_sd_dif.o := y
+CONTEXT_ANALYSIS_sd_zbc.o := y
+CONTEXT_ANALYSIS_sr.o := y
+CONTEXT_ANALYSIS_sr_ioctl.o := y
+CONTEXT_ANALYSIS_proc.o := y
 
 CFLAGS_aha152x.o =   -DAHA152X_STAT -DAUTOCONF
 
drivers/scsi/device_handler/Makefile +3 -0
index 0a603aefd2bb..5aa282a63e24 100644
@@ -2,6 +2,9 @@
 #
 # SCSI Device Handler
 #
+
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_SCSI_DH_RDAC)	+= scsi_dh_rdac.o
 obj-$(CONFIG_SCSI_DH_HP_SW)	+= scsi_dh_hp_sw.o
 obj-$(CONFIG_SCSI_DH_EMC)	+= scsi_dh_emc.o
#17 lore raw

Re: [PATCH v4 6/6] scsi: core: Enable lock context analysis

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Makefile variables intended to enable lock context analysis are incorrectly named due to missing…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Makefile variables intended to enable lock context analysis are incorrectly named due to missing 'scsi_' prefixes, failing to enable analysis for most intended SCSI core files. --

--- Patch [6]: [PATCH v4 6/6] scsi: core: Enable lock context analysis --- commit 72acfbb8a25ff6235de3b56d2113eb7681fd8184
Author: Bart Van Assche <[email protected]>

scsi: core: Enable lock context analysis

Enable lock context analysis for those SCSI core files that build without triggering any compiler warnings.

Quoted 20 lines
drivers/scsi/Makefile +12 -0
index 842c254bb2269..69ea80819f3ba 100644
@@ -14,6 +14,28 @@
 # satisfy certain initialization assumptions in the SCSI layer.
 # *!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!*!
 
+CONTEXT_ANALYSIS_constants.o := y
+CONTEXT_ANALYSIS_scsi.o := y
+CONTEXT_ANALYSIS_scsicam.o := y
+CONTEXT_ANALYSIS_common.o := y
+CONTEXT_ANALYSIS_devinfo.o := y
+CONTEXT_ANALYSIS_dh.o := y
+CONTEXT_ANALYSIS_ioctl.o := y
+CONTEXT_ANALYSIS_lib.o := y
+CONTEXT_ANALYSIS_lib_dma.o := y
+CONTEXT_ANALYSIS_error.o := y
+CONTEXT_ANALYSIS_logging.o := y
+CONTEXT_ANALYSIS_netlink.o := y

drivers/scsi/Makefile line 38

Quoted 10 lines
drivers/scsi/Makefile +10 -0
+CONTEXT_ANALYSIS_pm.o := y
+CONTEXT_ANALYSIS_scan.o := y
+CONTEXT_ANALYSIS_sysfs.o := y
+CONTEXT_ANALYSIS_trace.o := y
+CONTEXT_ANALYSIS_sd.o := y
+CONTEXT_ANALYSIS_sd_dif.o := y
+CONTEXT_ANALYSIS_sd_zbc.o := y
+CONTEXT_ANALYSIS_sr.o := y
+CONTEXT_ANALYSIS_sr_ioctl.o := y
+CONTEXT_ANALYSIS_proc.o := y

[Severity: Medium]
Do these Makefile variables need a "scsi_" prefix to match the actual object files?

For example, drivers/scsi/scsi_ioctl.c generates scsi_ioctl.o, so Kbuild expects CONTEXT_ANALYSIS_scsi_ioctl.o := y.

Since the target stems do not match the variables (like common, devinfo, dh, ioctl, lib, lib_dma, error, logging, netlink, pm, scan, sysfs, trace, and proc), Kbuild will silently ignore them and lock context analysis will not be enabled for these files.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/68f3346cab2be570839350831f82b6c9233832ed.1785534721.git.bvanassche@acm.org?part=6
Reviewed-by on v4 1/6
#18 lore raw

Re: [PATCH v4 1/6] ufs: core: Set task state before io_schedule_timeout()

In reply to Bart Van Assche

On Fri, 2026-07-31 at 14:52 -0700, Bart Van Assche wrote

Quoted 7 lines

Set the task state to TASK_UNINTERRUPTIBLE before calling io_schedule_timeout() in ufshcd_wait_for_pending_cmds(). Without setting the task state, io_schedule_timeout() returns immediately because the task state remains TASK_RUNNING. This results in a busy loop that wastes CPU cycles.

Quoted 5 lines
Fixes 2000bc309703 ("scsi: ufs: core: Reduce the clock scaling

latency")

---