Re: [PATCH net v3] usbnet: fix EVENT_RX_PAUSED race in usbnet_skb_return()

[Jakub Kicinski <kuba@kernel.org>]

On Sun, 11 Oct 2026 22:02:33 +0000 netdev-bot+sashiko@kernel.org wrote:
> Will lockdep complain now that this lock is taken with hardirqs enabled?
> 
> Before this patch, rxq_pause.lock was only taken through skb_queue_tail()
> and skb_dequeue(). Both use spin_lock_irqsave(). Here the lock is taken
> with spin_lock_bh(). usbnet_resume_rx() below takes it with plain
> spin_lock() inside local_bh_disable().
> 
> usbnet_probe() initializes all four queues with the same inline helper,
> so they appear to share one lockdep class:
> 
> usbnet_probe() {
>     ...
> 	skb_queue_head_init (&dev->rxq);
> 	skb_queue_head_init (&dev->txq);
> 	skb_queue_head_init (&dev->done);
> 	skb_queue_head_init(&dev->rxq_pause);
>     ...
> }
> 
> defer_bh() relies on this too. It uses spin_lock_nested() for
> dev->done.lock while it holds rxq.lock or txq.lock.
> 
> Some host controllers do not set HCD_BH (OHCI, UHCI, musb and others).
> On those, usb_hcd_giveback_urb() runs the completion directly from the
> HCD interrupt handler, so rx_complete()->defer_bh() takes that lock class
> in hardirq:
> 
> defer_bh() {
> 	spin_lock_irqsave(&list->lock, flags);
> 	...
> 	spin_lock_nested(&dev->done.lock, SINGLE_DEPTH_NESTING);
>     ...
> }
> 
> With CONFIG_PROVE_LOCKING, wouldn't the first qualifying MTU increase
> through usbnet_change_mtu()->usbnet_resume_rx() then trigger an
> "inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage" report and turn
> lockdep off?
> 
> No code takes rxq_pause.lock itself from hardirq today, so no real
> deadlock is known. Using spin_lock_irqsave()/spin_unlock_irqrestore() in
> both new critical sections would match every other sk_buff_head lock use
> in usbnet.c.

Good suggestion.