Re: [PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-10-14 15:22:51 [-0400], Alan Stern wrote:
> > "local interrupts disabled" as is local_irq_disable()? If so, that is
> > not the case. If it completes in BH context (due to HCD_BH) then
> > interrupts remain enabled.
> 
> Yes, but we can call local_irq_disable() before invoking the completion 
> handler -- if we decide that would be a good thing to do.

Oh please don't. This will break PREEMPT_RT badly.

> Otherwise I would say that we don't guarantee anything.

Okay.

> > > Of course, in special cases you might know more about how a particular 
> > > HCD works.  But except for platform-specific settings, it's hard to be 
> > > certain that your driver will always be used with that one particular 
> > > HCD.
> > > 
> > > If this is okay with you, I can write a documentation patch.
> > 
> > Sure. The kernel doc for usb_hcd_giveback_urb() says BH or caller's
> > context. I think you need to clarify that part and once we have tha, we
> > can proceed with the drivers.
> 
> What do you think?  Should local interrupts always be disabled, or 
> should we offer no guarantee?

No guarantees in this regard, please. Disabling interrupts will break
PREEMPT_RT here.

> Completion handler routines will have to be conservative, so if we don't 
> guarantee anything then they will need to disable interrupts themselves 
> whenever they require it (for example, by using spin_lock_irqsave() 
> instead of spin_lock()).  The core could save them this extra effort, at 
> the cost of unnecessarily disabling interrupts sometimes.

The problem is spin_lock_irqsave() will not disable interrupts on
PREEMPT_RT while local_irq_save() will. For this reason I would prefer
to skip it.

> USB isn't a tremendously fast bus, so the overhead probably doesn't 
> matter much.  Which tradeoff do you prefer?

So if we don't guarantee that BH is disabled then I would say nothing is
guaranteed, anything is possible.

A little note: cdns and dwc3 do disable BH in their voluntary threaded
interrupt handler. The reason is that the networking driver expect this.
I know this is the gadget and not HCD side but sort of the same issue.
While BH is disabled for interrupt handler (and forced threaded handler)
it is not if requested explicitly.

This would mean that the completion handler of the networking driver
would have to do local_bh_disable() themself. I could live with this ;)

> Alan Stern

Sebastian