[PATCH 0/2] s390/block: Enable CONTEXT_ANALYSIS

[PATCH 0/2] s390/block: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

Enable CONTEXT_ANALYSYS for drivers/s390/block.

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/block. In order to avoid false positives due to
conditional locking, disable context analysis for specific functions of
the dasd driver.

Disabling context analysis for specific functions keeps analysis enabled
for the rest of a file.

Jens, if you and Stefan agree to the dasd patches, I think it would be
best if this would be routed via your tree.

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):
  s390/dasd: Add __context_unsafe() attribute to various functions
  s390/block: Enable CONTEXT_ANALYSIS

 drivers/s390/block/Makefile    | 2 ++
 drivers/s390/block/dasd_eckd.c | 7 +++++++
 2 files changed, 9 insertions(+)

-- 
2.53.0

[PATCH 1/2] s390/dasd: Add __context_unsafe() attribute to various functions

[Heiko Carstens]

Disable context analysis for various functions to get rid of context
analysis compile time warnings using clang caused by conditional
locking like e.g.:

drivers/s390/block/dasd_eckd.c:1462:3:
  warning: releasing mutex 'dasd_pe_handler_mutex' that was not held [-Wthread-safety-analysis]
 1462 |                 mutex_unlock(&dasd_pe_handler_mutex);
      |                 ^

Use __context_unsafe() to provide a short comment why context analysis is
disabled for each function. It doesn't look like those functions can be
easily reworked to get rid of conditional locking.
Therefore disable context analysis for (only) those functions.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/block/dasd_eckd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index bacf770c0e1f..b4dcef06edaa 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -1468,6 +1468,7 @@ static void dasd_eckd_path_available_action(struct dasd_device *device,
 }
 
 static void do_pe_handler_work(struct work_struct *work)
+__context_unsafe(/* Conditional locking */)
 {
 	struct pe_handler_work_data *data;
 	struct dasd_device *device;
@@ -1501,6 +1502,7 @@ static void do_pe_handler_work(struct work_struct *work)
 
 static int dasd_eckd_pe_handler(struct dasd_device *device,
 				__u8 tbvpm, __u8 fcsecpm)
+__context_unsafe(/* Conditional locking */)
 {
 	struct pe_handler_work_data *data;
 
@@ -1601,6 +1603,7 @@ static int dasd_eckd_read_features(struct dasd_device *device)
 
 /* Read Volume Information - Volume Storage Query */
 static int dasd_eckd_read_vol_info(struct dasd_device *device)
+__context_unsafe(/* Conditional locking */)
 {
 	struct dasd_eckd_private *private = device->private;
 	struct dasd_psf_prssd_data *prssdp;
@@ -5965,6 +5968,7 @@ dasd_eckd_fill_info(struct dasd_device * device,
  */
 static int
 dasd_eckd_release(struct dasd_device *device)
+__context_unsafe(/* Conditional locking */)
 {
 	struct dasd_ccw_req *cqr;
 	int rc;
@@ -6020,6 +6024,7 @@ dasd_eckd_release(struct dasd_device *device)
  */
 static int
 dasd_eckd_reserve(struct dasd_device *device)
+__context_unsafe(/* Conditional locking */)
 {
 	struct dasd_ccw_req *cqr;
 	int rc;
@@ -6074,6 +6079,7 @@ dasd_eckd_reserve(struct dasd_device *device)
  */
 static int
 dasd_eckd_steal_lock(struct dasd_device *device)
+__context_unsafe(/* Conditional locking */)
 {
 	struct dasd_ccw_req *cqr;
 	int rc;
@@ -6129,6 +6135,7 @@ dasd_eckd_steal_lock(struct dasd_device *device)
  */
 static int dasd_eckd_snid(struct dasd_device *device,
 			  void __user *argp)
+__context_unsafe(/* Conditional locking */)
 {
 	struct dasd_ccw_req *cqr;
 	int rc;
-- 
2.53.0

[PATCH 2/2] s390/block: Enable CONTEXT_ANALYSIS

[Heiko Carstens]

All drivers in drivers/s390/block pass clang's compile time context
analysis. Therefore enable CONTEXT_ANALYSIS.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 drivers/s390/block/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/s390/block/Makefile b/drivers/s390/block/Makefile
index a0a54d2f063f..3dd1a3ce7ed1 100644
--- a/drivers/s390/block/Makefile
+++ b/drivers/s390/block/Makefile
@@ -3,6 +3,8 @@
 # S/390 block devices
 #
 
+CONTEXT_ANALYSIS := y
+
 dasd_eckd_mod-objs := dasd_eckd.o dasd_3990_erp.o dasd_alias.o
 dasd_fba_mod-objs  := dasd_fba.o
 dasd_diag_mod-objs := dasd_diag.o
-- 
2.53.0

Re: [PATCH 0/2] s390/block: Enable CONTEXT_ANALYSIS

[Stefan Haberland]

Am 06.08.26 um 15:00 schrieb Heiko Carstens:
> Enable CONTEXT_ANALYSYS for drivers/s390/block.
> 
> 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/block. In order to avoid false positives due to
> conditional locking, disable context analysis for specific functions of
> the dasd driver.
> 
> Disabling context analysis for specific functions keeps analysis enabled
> for the rest of a file.
> 
> Jens, if you and Stefan agree to the dasd patches, I think it would be
> best if this would be routed via your tree.
> 
> Thanks,
> Heiko
> 

Sounds good to me.
Acked-by: Stefan Haberland <sth@linux.ibm.com>

> [1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
> [2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")
> 
> Heiko Carstens (2):
>   s390/dasd: Add __context_unsafe() attribute to various functions
>   s390/block: Enable CONTEXT_ANALYSIS
> 
>  drivers/s390/block/Makefile    | 2 ++
>  drivers/s390/block/dasd_eckd.c | 7 +++++++
>  2 files changed, 9 insertions(+)
> 

Re: [PATCH 0/2] s390/block: Enable CONTEXT_ANALYSIS

[Jens Axboe]

On Thu, 06 Aug 2026 15:00:48 +0200, Heiko Carstens wrote:
> Enable CONTEXT_ANALYSYS for drivers/s390/block.
> 
> 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.
> 
> [...]

Applied, thanks!

[1/2] s390/dasd: Add __context_unsafe() attribute to various functions
      commit: 11d4f5e69fe25bc76e27eca75b4ef6f7aae53214
[2/2] s390/block: Enable CONTEXT_ANALYSIS
      commit: 30df3ab3c92d0e7854c54df6ae66f4aa6eb5b3d2

Best regards,
-- 
Jens Axboe

Re: [PATCH 1/2] s390/dasd: Add __context_unsafe() attribute to various functions

[Bart Van Assche]

On 8/6/26 6:00 AM, Heiko Carstens wrote:
> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index bacf770c0e1f..b4dcef06edaa 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -1468,6 +1468,7 @@ static void dasd_eckd_path_available_action(struct dasd_device *device,
>   }
>   
>   static void do_pe_handler_work(struct work_struct *work)
> +__context_unsafe(/* Conditional locking */)
>   {
>   	struct pe_handler_work_data *data;
>   	struct dasd_device *device;

__context_unsafe() should be avoided if there is a better solution. For
this driver I propose to make the following changes:
- Change dasd_pe_handler_mutex into a semaphore. This driver locks this
   mutex from one kernel thread and unlocks it from another kernel
   thread. That's not allowed!
- Split the dasd_eckd_read_vol_info(), dasd_eckd_release(), ...
   functions. Move the code between the mutex_lock() and mutex_unlock()
   calls into a new function. Rework both functions such that the
   mutex_lock() and mutex_unlock() calls end up in the same branch of an
   if-statement.

Thanks,

Bart.

Re: [PATCH 2/2] s390/block: Enable CONTEXT_ANALYSIS

[Bart Van Assche]

On 8/6/26 6:00 AM, Heiko Carstens wrote:
> --- a/drivers/s390/block/Makefile
> +++ b/drivers/s390/block/Makefile
> @@ -3,6 +3,8 @@
>   # S/390 block devices
>   #
>   
> +CONTEXT_ANALYSIS := y
> +
>   dasd_eckd_mod-objs := dasd_eckd.o dasd_3990_erp.o dasd_alias.o
>   dasd_fba_mod-objs  := dasd_fba.o
>   dasd_diag_mod-objs := dasd_diag.o

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

Re: [PATCH 1/2] s390/dasd: Add __context_unsafe() attribute to various functions

[Heiko Carstens]

On Thu, Aug 06, 2026 at 09:34:06AM -0700, Bart Van Assche wrote:
> On 8/6/26 6:00 AM, Heiko Carstens wrote:
> > diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> > index bacf770c0e1f..b4dcef06edaa 100644
> > --- a/drivers/s390/block/dasd_eckd.c
> > +++ b/drivers/s390/block/dasd_eckd.c
> > @@ -1468,6 +1468,7 @@ static void dasd_eckd_path_available_action(struct dasd_device *device,
> >   }
> >   static void do_pe_handler_work(struct work_struct *work)
> > +__context_unsafe(/* Conditional locking */)
> >   {
> >   	struct pe_handler_work_data *data;
> >   	struct dasd_device *device;
> 
> __context_unsafe() should be avoided if there is a better solution.

Sure, I did that for other code, but this one is up to Stefan.

> For
> this driver I propose to make the following changes:
> - Change dasd_pe_handler_mutex into a semaphore. This driver locks this
>   mutex from one kernel thread and unlocks it from another kernel
>   thread. That's not allowed!

Yes, I did not realize that when adding those attributes, however that
was also reported by Sashiko.

> - Split the dasd_eckd_read_vol_info(), dasd_eckd_release(), ...
>   functions. Move the code between the mutex_lock() and mutex_unlock()
>   calls into a new function. Rework both functions such that the
>   mutex_lock() and mutex_unlock() calls end up in the same branch of an
>   if-statement.

Agreed, all for Stefan's todo list :)