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

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 <[email protected]>

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/[email protected]?part=1