Re: [net,PATCH v3 1/2] net: ks8851: Reinstate disabling of BHs around IRQ handler

Marek Vasut
2026-04-15T23:14:38Z

On 4/14/26 4:52 PM, Sebastian Andrzej Siewior wrote:
> On 2026-04-14 16:20:46 [+0200], Marek Vasut wrote:
>>> This is what happens since commit 0913ec336a6c0 ("net: ks8851: Fix
>>> deadlock with the SPI chip variant"). Before that commit the softirq
>>> execution will be picked up by netdev_alloc_skb_ip_align() and requires
>>> PREEMPT_RT and a RX packet in #1 to trigger the deadlock.
>>
>> Do you want me to add this into the V4 commit message ?
>
> The description does not match the code since the commit mentioned
> above.

I hope the V4 commit message is a bit better.

>>> The backtrace here and the description is based on an older kernel.
>>> However
>> I actually did update the backtrace in V3 with the one from current next
>> 20260413 .
>
> That would be from yesterday and the change is merged since v6.10. But
> why is the softirq starting from __netdev_alloc_skb() instead of
> spin_unlock_bh(&ks->statelock)? After that unlock, the softirq must be
> processed and __netdev_alloc_skb() _could_ observe pending softirqs but
> not from ks8851.
Because __netdev_alloc_skb() also enables/disables BH , see the "else"
branch:

759 struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
unsigned int len,
760 gfp_t gfp_mask)
761 {
...
786 if (in_hardirq() || irqs_disabled()) {
787 nc = this_cpu_ptr(&netdev_alloc_cache);
788 data = page_frag_alloc(nc, len, gfp_mask);
789 pfmemalloc = page_frag_cache_is_pfmemalloc(nc);
790 } else {
791 local_bh_disable();
792 local_lock_nested_bh(&napi_alloc_cache.bh_lock);
...
798 local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
799 local_bh_enable();
...