Re: [RFC PATCH 0/2] net: threadable napi poll loop

[Paolo Abeni]

On Fri, 2016-05-13 at 10:36 -0700, Eric Dumazet wrote:
> On Fri, May 13, 2016 at 10:19 AM, Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
> 
> > The difference is small, in the noise range:
> >
> > [with this patch applied]
> > super_netperf 100 -H 192.168.122.1 -t UDP_STREAM -l 60 -- -m 1
> > 9.00
> >
> > [adding the test into __local_bh_enable_ip(), too]
> > super_netperf 100 -H 192.168.122.1 -t UDP_STREAM -l 60 -- -m 1
> > 9.14
> >
> > but reproducible, in my experiments.
> > I have similar data for different number of flows.
> >
> >> I believe I did this so that we factorize the logic in do_softirq()
> >> and keep the code local to kernel/softirq.c
> >>
> >> Otherwise, netif_rx_ni() could also process softirq while ksoftirqd
> >> was scheduled,
> >> so I would have to  'export' the ksoftirqd_running(void) helper in an
> >> include file.
> >
> > The idea could be to add the test in __local_bh_enable_ip(), maintaining
> > the test also in do_softirq() (as currently done, i.e for
> > local_softirq_pending())
> >
> 
> Then I guess even the !in_interrupt() test we do is expensive and
> could be avoided,
> since do_softirq() is doing it again in the unlikely case it really is needed.
> 
> @@ -162,7 +170,8 @@ void __local_bh_enable_ip(unsigned long ip,
> unsigned int cnt)
>          */
>         preempt_count_sub(cnt - 1);
> 
> -       if (unlikely(!in_interrupt() && local_softirq_pending())) {
> +       if (unlikely(local_softirq_pending()) &&
> +                    !ksoftirqd_running()) {
>                 /*
>                  * Run softirq if any pending. And do it in its own stack
>                  * as we may be calling this deep in a task call stack already.

I'm sorry for the not-so-prompt reply. I had to use a different H/W, so
I had to re-run the tests with all the patch flavors to get comparable
results.

While I can confirm that adding the '!ksoftirqd_running()' condition
improves the throughput a little, but in a reproducible way, removing
the '!in_interrupt()' don't change the result measurably, in my
environment.

While running the test against a kernel with the above chunk applied I
got a couple of:

[  702.791025] NOHZ: local_softirq_pending 08

Not seen with the other versions of this patch.

Cheers,

Paolo