lkml.org 
[lkml]   [2021]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH RFC] r8152: Ensure that napi_schedule() is handled
On Sat, 15 May 2021 01:23:02 +0200 Thomas Gleixner wrote:
> On Fri, May 14 2021 at 14:41, Jakub Kicinski wrote:
> >> This is not related to force_irqthreads at all. This very driver invokes
> >> it from plain thread context.
> >
> > I see, but a driver calling __napi_schedule_irqoff() from its IRQ
> > handler _would_ be an issue, right? Or do irq threads trigger softirq
> > processing on exit?
>
> Yes, they do. See irq_forced_thread_fn(). It has a local_bh_disable() /
> local_bh_ enable() pair around the invocation to ensure that.

Ah, excellent!

> >> You could have napi_schedule_intask() or something like that which would
> >> do the local_bh_disable()/enable() dance around the invocation of
> >> napi_schedule(). That would also document it clearly in the drivers. A
> >> quick grep shows a bunch of instances which could be replaced:
> >>
> >> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c-5704- local_bh_disable();
> >> drivers/net/ethernet/mellanox/mlx4/en_netdev.c-1830- local_bh_disable();
> >> drivers/net/usb/r8152.c-1552- local_bh_disable();
> >> drivers/net/virtio_net.c-1355- local_bh_disable();
> >> drivers/net/wireless/intel/iwlwifi/pcie/rx.c-1650- local_bh_disable();
> >> drivers/net/wireless/intel/iwlwifi/pcie/rx.c-2015- local_bh_disable();
> >> drivers/net/wireless/intel/iwlwifi/pcie/rx.c-2225- local_bh_disable();
> >> drivers/net/wireless/intel/iwlwifi/pcie/rx.c-2235- local_bh_disable();
> >> drivers/s390/net/qeth_core_main.c-3515- local_bh_disable();
> >
> > Very well aware, I've just sent a patch for mlx5 last week :)
> >
> > My initial reaction was the same as yours - we should add lockdep
> > check, and napi_schedule_intask(). But then I started wondering
> > if it's all for nothing on rt or with force_irqthreads, and therefore
> > we should just eat the extra check.
>
> We can make that work but sure I'm not going to argue when you decide to
> just go for raise_softirq_irqsoff().
>
> I just hacked that check up which is actually useful beyond NAPI. It's
> straight forward except for that flush_smp_call_function_from_idle()
> oddball, which immeditately triggered that assert because block mq uses
> __raise_softirq_irqsoff() in a smp function call...
>
> See below. Peter might have opinions though :)

Looks good to me, since my thinking that RT complicates things here was
wrong I'm perfectly happy with the lockdep + napi_schedule_intask().

\
 
 \ /
  Last update: 2021-05-15 01:36    [W:0.085 / U:0.436 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site