Re: [PATCH net-next] dibs: Avoid inconsistent lockstate warning in dibs_lo_move_data()

[Paolo Abeni <pabeni@redhat.com>]

On 9/8/26 7:04 PM, Alexandra Winter wrote:
> On 08.09.26 14:45, netdev-bot+sashiko@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential
>> issue(s) to consider:
>>
>> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>>
>> - [Medium] The patch fixes an observable lockdep "inconsistent lock
>>   state" splat produced by the plain spin_lock(&dibs->lock) in…
>>
>> --
>>
>> Here is the report.
>>
>> dibs: Avoid inconsistent lockstate warning in dibs_lo_move_data()
>>
>> This converts the plain spin_lock()/spin_unlock() around the
>> dibs->subs[]->ops->handle_irq() call in dibs_lo_move_data() to
>> spin_lock_irqsave()/spin_unlock_irqrestore(), because the same lock is
>> also taken from hardirq context in ism_handle_irq().  The commit message
>> notes this is not a real deadlock since dibs_lo devices have no interrupt
>> context.
>>
>>> diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
>>> index 649e4e375be33..44a2e74c2efcb 100644
>>> --- a/drivers/dibs/dibs_loopback.c
>>> +++ b/drivers/dibs/dibs_loopback.c
>>
>> [ ... ]
>>
>>> @@ -267,12 +268,12 @@ static int dibs_lo_move_data(struct dibs_dev *dibs, u64 dmb_tok,
>>>  	if (!sf)
>>>  		return 0;
>>>  
>>> -	spin_lock(&dibs->lock);
>>> +	spin_lock_irqsave(&dibs->lock, flags);
>>>  	client_id = dibs->dmb_clientid_arr[sba_idx];
>>>  	s_mask = ror16(0x1000, idx);
>>>  	if (likely(client_id != NO_DIBS_CLIENT && dibs->subs[client_id]))
>>>  		dibs->subs[client_id]->ops->handle_irq(dibs, sba_idx, s_mask);
>>> -	spin_unlock(&dibs->lock);
>>> +	spin_unlock_irqrestore(&dibs->lock, flags);
>>
>> [Severity: Medium]
>> Should this carry a Fixes: tag?  The lockdep splat quoted in the commit
>> message is reproducible on shipped kernels, and git blame attributes the
>> converted spin_lock()/spin_unlock() pair in dibs_lo_move_data() to:
>>
>> cc21191b584c6f ("dibs: Move data path to dibs layer")
>>
>> The hardirq-side acquisition of the same lock is in ism_handle_irq()
>> (drivers/s390/net/ism_drv.c):
>>
>> 	spin_lock(&dibs->lock);
>>
>> which matches the {IN-HARDIRQ-W} registration in the report, so the
>> warning can be hit by ordinary SMC traffic through
>> smc_close_active()->smcd_cdc_msg_send()->smcd_tx_ism_write()->dibs_lo_move_data()
>> on a lockdep-enabled kernel.
>>
>> The splat also ends with "INFO: lockdep is turned off", so further lock
>> checking is lost for the rest of that boot on affected kernels.
>>
>> Would adding the following help backporters and stable tooling identify
>> which trees need this change?
>>
>> Fixes: cc21191b584c6f ("dibs: Move data path to dibs layer")
>>
> 
> 
> As mentioned in the commit message the paths reported by lockdep belong to two
> different device types (dibs-lo and ism). So not a real deadlock.
> So unless lockdep-warnings by themselves warrant a Fixes-tag (?), I think net-next is correct here.
> 
> Paolo applied this already to net-next.
> For the records: Sahiko identified the right patch to blame.
The current guidance is no fixes tag for net-next patches address old
'issues'. Since this is a false positive, I deemed it as a 'non issue',
hence I agreed with the net-next target (and thus no fixes tag).

/P