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

Am 06.08.26 um 16:06 schrieb Heiko Carstens:
> On Thu, Aug 06, 2026 at 01:09:16PM +0000, [email protected] 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.