Skip to contentSkip to conversation

scsi: zfcp: Enable CONTEXT_ANALYSIS

PATCH Applied Heiko Carstens 4 participants 7 messages Aug 6 – Aug 7

Subsystems (3)
SCSI s390 s390 zfcp driver
lore
Share & tools
lore

Heiko Carstens

author maintainer
#0 lore raw

cover scsi: zfcp: Enable CONTEXT_ANALYSIS

Enable CONTEXT_ANALYSYS for the zfcp driver.

Static code checking for acquiring and releasing locks used to be done with sparse. That was removed with [1] and replaced with a clang based approach [2]. The new approach requires that each subsystem needs to be explicitly enabled for checking.

Do that for drivers/s390/scsi. Add a __must_hold() attribute to zfcp_qdio_sbal_get() to address the only valid warning. Then enable CONTEXT_ANALYSIS, similar to other patches for drivers/s390 I'm currently working on.

Thanks,
Heiko

[1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
[2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")

Heiko Carstens (2):
scsi: zfcp: Add __must_hold() attribute to zfcp_qdio_sbal_get()
scsi: zfcp: Enable CONTEXT_ANALYSIS

 drivers/s390/scsi/Makefile    | 2 ++
 drivers/s390/scsi/zfcp_ext.h  | 3 ++-
 drivers/s390/scsi/zfcp_qdio.c | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)
Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH 1/2 scsi: zfcp: Add __must_hold() attribute to zfcp_qdio_sbal_get()

Add __must_hold() attribute to zfcp_qdio_sbal_get() in order to let clang's context analysis know that qdio->req_q_wq must be held on function entry. This is also documented above the function. Without this annotation this leads to a valid warning when context analysis is enabled:

drivers/s390/scsi/zfcp_qdio.c:287:8: warning:
expecting spinlock '->req_q_lock' to be held at start of each loop [-Wthread-safety-analysis]
287 | ret = wait_event_interruptible_lock_irq_timeout(qdio->req_q_wq,
| ^

Signed-off-by Heiko Carstens <[email protected]>
---
 drivers/s390/scsi/zfcp_ext.h  | 3 ++-
 drivers/s390/scsi/zfcp_qdio.c | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)
drivers/s390/scsi/zfcp_ext.h +2 -1
index 40bd597fb4cd..0eb9bc9d03e4 100644
@@ -154,7 +154,8 @@ extern ssize_t zfcp_fsf_scnprint_fc_security(char *buf, size_t size,
 /* zfcp_qdio.c */
 extern int zfcp_qdio_setup(struct zfcp_adapter *);
 extern void zfcp_qdio_destroy(struct zfcp_qdio *);
-extern int zfcp_qdio_sbal_get(struct zfcp_qdio *);
+extern int zfcp_qdio_sbal_get(struct zfcp_qdio *qdio)
+	__must_hold(qdio->req_q_lock);
 extern int zfcp_qdio_send(struct zfcp_qdio *, struct zfcp_qdio_req *);
 extern int zfcp_qdio_sbals_from_sg(struct zfcp_qdio *, struct zfcp_qdio_req *,
 				   struct scatterlist *);
drivers/s390/scsi/zfcp_qdio.c +1 -0
index ce1af72d9b61..bac8385e069f 100644
@@ -281,6 +281,7 @@ static int zfcp_qdio_sbal_check(struct zfcp_qdio *qdio)
  * Returns: 0 on success, -EIO if there is no free sbal after waiting.
  */
 int zfcp_qdio_sbal_get(struct zfcp_qdio *qdio)
+	__must_hold(qdio->req_q_lock)
 {
 	long ret;
Signature · 1 line
2.53.0
AI review
#2 lore raw

Re: [PATCH 1/2] scsi: zfcp: Add __must_hold() attribute to zfcp_qdio_sbal_get()

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 6 lines

Add __must_hold() attribute to zfcp_qdio_sbal_get() in order to let clang's context analysis know that qdio->req_q_wq must be held on function entry. This is also documented above the function. Without this annotation this leads to a valid warning when context analysis is enabled:

Quoted 6 lines

drivers/s390/scsi/zfcp_qdio.c:287:8: warning:
expecting spinlock '->req_q_lock' to be held at start of each loop [-Wthread-safety-analysis]
287 | ret = wait_event_interruptible_lock_irq_timeout(qdio->req_q_wq,
| ^

Signed-off-by Heiko Carstens <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#3 lore raw

PATCH 2/2 scsi: zfcp: Enable CONTEXT_ANALYSIS

In reply to Heiko Carstens

The zfcp driver passes clang's compile time context analysis. Therefore enable CONTEXT_ANALYSIS.

Signed-off-by Heiko Carstens <[email protected]>
---
 drivers/s390/scsi/Makefile | 2 ++
 1 file changed, 2 insertions(+)
drivers/s390/scsi/Makefile +2 -0
index 352056eb0dd1..01b41f7c6860 100644
@@ -3,6 +3,8 @@
 # Makefile for the S/390 specific device drivers
 #
 
+CONTEXT_ANALYSIS := y
+
 zfcp-objs := zfcp_aux.o zfcp_ccw.o zfcp_dbf.o zfcp_erp.o \
 	     zfcp_fc.o zfcp_fsf.o zfcp_qdio.o zfcp_scsi.o zfcp_sysfs.o \
 	     zfcp_unit.o zfcp_diag.o
Signature · 1 line
2.53.0
AI review
#4 lore raw
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 4 lines

The zfcp driver passes clang's compile time context analysis. Therefore enable CONTEXT_ANALYSIS.

Signed-off-by Heiko Carstens <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#5 lore raw
In reply to Heiko Carstens
Quoted 11 lines · Heiko Carstens

Enable CONTEXT_ANALYSYS for the zfcp driver.

Static code checking for acquiring and releasing locks used to be done with sparse. That was removed with [1] and replaced with a clang based approach [2]. The new approach requires that each subsystem needs to be explicitly enabled for checking.

Do that for drivers/s390/scsi. Add a __must_hold() attribute to zfcp_qdio_sbal_get() to address the only valid warning. Then enable CONTEXT_ANALYSIS, similar to other patches for drivers/s390 I'm currently working on.

These two patches could have been combined into a single patch. Anyway, both patches look good to me.

Thanks,

Bart.