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
| ||
|
Date: Tue, 5 Jan 2021 14:42:32 +0100 From: Peter Zijlstra <peterz@...radead.org> To: "Paul E. McKenney" <paulmck@...nel.org> Cc: Neeraj Upadhyay <neeraju@...eaurora.org>, josh@...htriplett.org, rostedt@...dmis.org, mathieu.desnoyers@...icios.com, jiangshanlai@...il.com, joel@...lfernandes.org, rcu@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] rcu: Fix dynticks_nmi_nesting underflow check in rcu_is_cpu_rrupt_from_idle On Wed, Dec 23, 2020 at 07:12:31AM -0800, Paul E. McKenney wrote: > On Wed, Dec 23, 2020 at 02:09:37PM +0530, Neeraj Upadhyay wrote: > > For the smp_call_function() optimization, where callbacks can run from > > idle context, in commit 806f04e9fd2c ("rcu: Allow for smp_call_function() > > running callbacks from idle"), an additional check is added in > > rcu_is_cpu_rrupt_from_idle(), for dynticks_nmi_nesting value being 0, > > for these smp_call_function() callbacks running from idle loop. > > However, this commit missed updating a preexisting underflow check > > of dynticks_nmi_nesting, which checks for a non zero positive value. > > Fix this warning and while at it, read the counter only once. > > > > Signed-off-by: Neeraj Upadhyay <neeraju@...eaurora.org> > > --- > > > > Hi, > > > > I was not able to get this warning, with scftorture. > > > > RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 0, > > "RCU dynticks_nmi_nesting counter underflow/zero!"); > > > > Not sure if idle loop smp_call_function() optimization is already present > > in mainline? > > Now that you mention it, I don't see it. kernel/sched/idle.c:do_idle() calls flush_smp_call_function_from_idle(). (nothing x86 specific about it) > > Another thing, which I am not sure of is, maybe lockdep gets disabled > > in the idle loop contexts, where rcu_is_cpu_rrupt_from_idle() is called? > > Was this the original intention, to keep the lockdep based > > RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 0 > > check separate from idle task context nesting value > > WARN_ON_ONCE(!nesting && !is_idle_task(current)) check? > > An easy way to test lockdep is to create a pair of locks, acquire them > in one order then release them both, and finally acquire them in the > other order and then release them both. If lockdep is configured and > enabled, it will complain. IIRC (and this is after not staring at the computer for 2 weeks) lockdep should work just fine in idle, except of course that RCU will be stopped so actually taking locks will scream bloody murder due to tracing etc.. > The only reason I used RCU_LOCKDEP_WARN() was that people were complaining > to me about idle-entry overhead back at that time. So without lockdep, > there is zero overhead. Maybe people have become more tolerant of idle > delays, or perhaps they are not so worried about an extra check of a > cache-hot quantity. Not having checks also saves on $I and branches, in general I think having checks depend on DEBUG features, esp. those we don't really expect to trigger is still sane. > I am tempted to pull this in as is, given the current logical > inconsistency in the checks. Thoughts? Patch looks ok, although I've seen compilers do CSE on __this_cpu_read() (on x86).
Powered by blists - more mailing lists