[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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch uses `__context_unsafe` to silence a valid static analysis warning about a cross-thread mutex unlock in `do_pe_handler_work`, hiding a severe locking API violation.
--

--- Patch [1]: [PATCH 1/2] s390/dasd: Add __context_unsafe() attribute to various functions ---
commit 0764dc821ba7e47d621de5b2696256130f3eee09
Author: Heiko Carstens <hca@linux.ibm.com>

s390/dasd: Add __context_unsafe() attribute to various functions

This patch disables context analysis for various functions to avoid compile
time warnings using clang caused by conditional locking. The __context_unsafe()
attribute is used to provide a short comment on why it is disabled.

> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index 74fe73b5738ab..e908e2f29919e 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
[ ... ]
> @@ -1432,6 +1432,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;

[ ... ]

> @@ -1465,6 +1466,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;

[Severity: High]
This is a pre-existing issue, but does silencing this warning hide a
legitimate cross-thread mutex unlock?

When kzalloc fails in dasd_eckd_pe_handler(), it calls mutex_trylock(),
establishing the current interrupted task as the lock owner:

dasd_eckd_pe_handler() {
    ...
    data = kzalloc_obj(*data, GFP_ATOMIC | GFP_DMA);
    if (!data) {
        if (mutex_trylock(&dasd_pe_handler_mutex)) {
            data = pe_handler_worker;
            data->isglobal = 1;
    ...
    schedule_work(&data->worker);
}

The lock is then released from a different task (the workqueue worker thread)
in do_pe_handler_work():

do_pe_handler_work() {
    ...
    if (data->isglobal)
        mutex_unlock(&dasd_pe_handler_mutex);
    ...
}

Unlocking a struct mutex from a task other than the one that acquired it
strictly violates mutex semantics, which could cause debug warnings or
panics.

Additionally, mutex_trylock() in atomic context (inferred from GFP_ATOMIC
usage) violates lockdep wait-context rules and PREEMPT_RT requirements.

Could this code be restructured to avoid the cross-thread unlock rather than
silencing the warning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806130050.2057443-1-hca@linux.ibm.com?part=1

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

[Heiko Carstens]

On Thu, Aug 06, 2026 at 01:09:16PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
...
> >  static int dasd_eckd_pe_handler(struct dasd_device *device,
> >  				__u8 tbvpm, __u8 fcsecpm)
> > +__context_unsafe(/* Conditional locking */)
> >  {
> >  	struct pe_handler_work_data *data;
> 
> [Severity: High]
> This is a pre-existing issue, but does silencing this warning hide a
> legitimate cross-thread mutex unlock?
> 
> When kzalloc fails in dasd_eckd_pe_handler(), it calls mutex_trylock(),
> establishing the current interrupted task as the lock owner:
> 
> dasd_eckd_pe_handler() {
>     ...
>     data = kzalloc_obj(*data, GFP_ATOMIC | GFP_DMA);
>     if (!data) {
>         if (mutex_trylock(&dasd_pe_handler_mutex)) {
>             data = pe_handler_worker;
>             data->isglobal = 1;
>     ...
>     schedule_work(&data->worker);
> }
> 
> The lock is then released from a different task (the workqueue worker thread)
> in do_pe_handler_work():
> 
> do_pe_handler_work() {
>     ...
>     if (data->isglobal)
>         mutex_unlock(&dasd_pe_handler_mutex);
>     ...
> }
> 
> Unlocking a struct mutex from a task other than the one that acquired it
> strictly violates mutex semantics, which could cause debug warnings or
> panics.
> 
> Additionally, mutex_trylock() in atomic context (inferred from GFP_ATOMIC
> usage) violates lockdep wait-context rules and PREEMPT_RT requirements.
> 
> Could this code be restructured to avoid the cross-thread unlock rather than
> silencing the warning?

This looks indeed like a valid finding. Looking at the code it looks to me
like there is no need for a mutex at all. Instead a simple integer which
indicates if the global pe_handler_work_data is used should be
sufficient. Exclusive access would then be tried to get with xchg().

Jens, Stefan, should this be addressed before or after this small series?
I would prefer after this series, since in the meantime I dislike those
"pre-existing" bugs, which pile up :)

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

[Stefan Haberland]

Am 06.08.26 um 16:06 schrieb Heiko Carstens:
> On Thu, Aug 06, 2026 at 01:09:16PM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> ...
>>>  static int dasd_eckd_pe_handler(struct dasd_device *device,
>>>  				__u8 tbvpm, __u8 fcsecpm)
>>> +__context_unsafe(/* Conditional locking */)
>>>  {
>>>  	struct pe_handler_work_data *data;
>>
>> [Severity: High]
>> This is a pre-existing issue, but does silencing this warning hide a
>> legitimate cross-thread mutex unlock?
>>
>> When kzalloc fails in dasd_eckd_pe_handler(), it calls mutex_trylock(),
>> establishing the current interrupted task as the lock owner:
>>
>> dasd_eckd_pe_handler() {
>>     ...
>>     data = kzalloc_obj(*data, GFP_ATOMIC | GFP_DMA);
>>     if (!data) {
>>         if (mutex_trylock(&dasd_pe_handler_mutex)) {
>>             data = pe_handler_worker;
>>             data->isglobal = 1;
>>     ...
>>     schedule_work(&data->worker);
>> }
>>
>> The lock is then released from a different task (the workqueue worker thread)
>> in do_pe_handler_work():
>>
>> do_pe_handler_work() {
>>     ...
>>     if (data->isglobal)
>>         mutex_unlock(&dasd_pe_handler_mutex);
>>     ...
>> }
>>
>> Unlocking a struct mutex from a task other than the one that acquired it
>> strictly violates mutex semantics, which could cause debug warnings or
>> panics.
>>
>> Additionally, mutex_trylock() in atomic context (inferred from GFP_ATOMIC
>> usage) violates lockdep wait-context rules and PREEMPT_RT requirements.
>>
>> Could this code be restructured to avoid the cross-thread unlock rather than
>> silencing the warning?
> 
> This looks indeed like a valid finding. Looking at the code it looks to me
> like there is no need for a mutex at all. Instead a simple integer which
> indicates if the global pe_handler_work_data is used should be
> sufficient. Exclusive access would then be tried to get with xchg().
> 
> Jens, Stefan, should this be addressed before or after this small series?
> I would prefer after this series, since in the meantime I dislike those
> "pre-existing" bugs, which pile up :)

Yes, looks valid to me.
I already have a certain amount of findings which I have to evaluate and
address. I can put this on the list as well.

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 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 :)

[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 2/2] s390/block: Enable CONTEXT_ANALYSIS

[sashiko-bot]

> 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>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806130050.2057443-1-hca@linux.ibm.com?part=2

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 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