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

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

On 2026-10-12 16:26:28 [-0400], Alan Stern wrote:
> On Mon, Oct 12, 2026 at 09:49:02PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2026-10-09 13:31:17 [+0200], Greg Kroah-Hartman wrote:
> > > > As far as I am aware, there aren't really any guarantees on the 
> > > > context of a USB URB-completion callback.  The kerneldoc for struct urb 
> > > > in include/linux/usb.h says "The completion callback is made 
> > > > in_interrupt()", but that is most definitely out of date.
> > > 
> > > I don't think so, I think some platforms still have those callbacks in
> > > irq context, unless we changed to threaded irq handlers everywhere?  I
> > > could have missed that, but we should still write the callbacks to
> > > assume that and then we should be fine even if we aren't in irq context,
> > > right?
> > 
> > The HCD invokes the completion callback usually in IRQ context. I think
> > the VHCI is an exception here.
> > For the forced-threaded mode we disable BH before the IRQ handler is
> > invoked. I think even dummy_hcd invokes them from the timer if my memory
> > is right.
> > 
> > > > Drivers shouldn't rely on any particular context guarantees.  Not even 
> > > > whether local irqs are enabled/disabled.
> > > 
> > > Agreed.
> > 
> > I would suggest to make VHCI invoke the completion callback with BH
> > disabled. usbnet's tx_complete() does also defer_bh() which schedules a
> > BH worker via queue_work(). While this works without disabling BH it
> > leads to waking ksoftirqd and an additional context switch which we
> > could avoid by dealing with it directly.
> 
> Today you fix VHCI, tomorrow some other virtual HCD will cause problems.

We don't have other virtual HCDs, do we?
The other driver is drivers/net/mctp/mctp-usb.c.
mctp_usb_out_complete() -> … -> mctp_usblib_tx_stats_update() does
get_cpu_ptr() for stats. But mctp_usb_in_complete() has 
mctp_usblib_rx_complete() -> mctp_usblib_rx_stats_single_drop() and
mctp_usblib_rx() which do just this_cpu_ptr(). Plus the queue wakes &
netif_rx() which wake ksoftirqd.

That makes two drivers from a quick grep. But I guess it is just
networking for the per-CPU stats.

> Should we document any requirements that all HCDs will have to follow 
> and that all completion handlers can rely on?

It used to complete in IRQ context. Then you added the possibility to
complete BH for those HCDs marked as such (except for the RH). I think
it was always reasonable to assume that it is either IRQ-context
(force-threaded or not for !HCD_BH) or BH (for HCD_BH). It makes hardly
a difference, BH is always off and networking uses to rely on it.
So yes, documenting would make sense.

Do you want to document it yourself and have me do the VHCI bits?

> Alan Stern

Sebastian