netdev - Re: [PATCH 2/3] softirq: avoid spurious stalls due to need

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 5 Mar 2023 14:42:11 -0800
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Jakub Kicinski <kuba@...nel.org>, peterz@...radead.org,
        jstultz@...gle.com, edumazet@...gle.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Frederic Weisbecker <frederic@...nel.org>
Subject: Re: [PATCH 2/3] softirq: avoid spurious stalls due to need_resched()

On Sun, Mar 05, 2023 at 09:43:23PM +0100, Thomas Gleixner wrote:
> On Fri, Mar 03 2023 at 13:31, Jakub Kicinski wrote:
> > On Fri, 03 Mar 2023 14:30:46 +0100 Thomas Gleixner wrote:

[ . . . ]

> > Where 512 is the ORed pending mask over all iterations
> > 512 == 1 << RCU_SOFTIRQ.
> >
> > And they usually take less than 100us to consume the 10 iterations.
> > Histogram of usecs consumed when we run out of loop iterations:
> >
> > [16, 32)               3 |                                                    |
> > [32, 64)            4786 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
> > [64, 128)            871 |@@@@@@@@@                                           |
> > [128, 256)            34 |                                                    |
> > [256, 512)             9 |                                                    |
> > [512, 1K)            262 |@@                                                  |
> > [1K, 2K)              35 |                                                    |
> > [2K, 4K)               1 |                                                    |
> >
> > Paul, is this expected? Is RCU not trying too hard to be nice?
> >
> > # cat /sys/module/rcutree/parameters/blimit
> > 10
> >
> > Or should we perhaps just raise the loop limit? Breaking after less 
> > than 100usec seems excessive :(
> 
> No. Can we please stop twiddling a parameter here and there and go and
> fix this whole problem space properly. Increasing the loop count for RCU
> might work for your particular usecase and cause issues in other
> scenarios.
> 
> Btw, RCU seems to be a perfect candidate to delegate batches from softirq
> into a seperate scheduler controllable entity.

Indeed, as you well know, CONFIG_RCU_NOCB_CPU=y in combination with the
rcutree.use_softirq kernel boot parameter in combination with either the
nohz_full or rcu_nocbs kernel boot parameter and then the callbacks are
invoked within separate kthreads so that the scheduler has full control.
In addition, this dispenses with all of the heuristics that are otherwise
necessary to avoid invoking too many callbacks in one shot.

Back in the day, I tried making this the default (with an eye towards
making it the sole callback-execution scheme), but this resulted in
some ugly performance regressions.  This was in part due to the extra
synchronization required to queue a callback and in part due to the
higher average cost of a wakeup compared to a raise_softirq().

So I changed to the current non-default arrangement.

And of course, you can do it halfway by booting kernel built with
CONFIG_RCU_NOCB_CPU=n with the rcutree.use_softirq kernel boot parameter.
But then the callback-invocation-limit heuristics are still used, but
this time to prevent callback invocation from preventing the CPU from
reporting quiescent states.  But if this was the only case, simpler
heuristics would suffice.

In short, it is not hard to make RCU avoid using softirq, but doing so
is not without side effects.  ;-)

							Thanx, Paul