linux-kernel - Re: [patch 59/66] rcu: Convert rcutree to hotplug state machine

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:	Tue, 12 Jul 2016 12:57:35 +0200 (CEST)
From:	Anna-Maria Gleixner <anna-maria@...utronix.de>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [patch 59/66] rcu: Convert rcutree to hotplug state machine

(edit cc: add tglx)

On Mon, 11 Jul 2016, Paul E. McKenney wrote:

> On Mon, Jul 11, 2016 at 12:29:04PM -0000, Anna-Maria Gleixner wrote:
> > From: Thomas Gleixner <tglx@...utronix.de>
> > 
> > Straight forward conversion to the state machine. Though the question arises
> > whether this needs really all these state transitions to work.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> > Reviewed-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> > Cc: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> > Signed-off-by: Anna-Maria Gleixner <anna-maria@...utronix.de>
> 
> I believe that this patch breaks !SMP builds, as it has the side effect
> of pulling a Tree RCU include file into Tiny RCU builds.
>
> Some questions below, and a related patch at the end.  The related patch
> provides exact detection of CPUs coming online, and passes light rcutorture
> testing.

We will take it into account before this change.

> The dying-idle state is still covered by direct function call, correct?
> (The call to rcu_report_dead() from cpuhp_report_idle_dead().)

Yes.

> > --- a/include/linux/rcutree.h
> > +++ b/include/linux/rcutree.h
> > @@ -111,4 +111,19 @@ bool rcu_is_watching(void);
> > 
> >  void rcu_all_qs(void);
> > 
> > +/* RCUtree hotplug events */
> > +#if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU)
> > +int rcutree_prepare_cpu(unsigned int cpu);
> > +int rcutree_online_cpu(unsigned int cpu);
> > +int rcutree_offline_cpu(unsigned int cpu);
> > +int rcutree_dead_cpu(unsigned int cpu);
> > +int rcutree_dying_cpu(unsigned int cpu);
> > +#else
> > +#define rcutree_prepare_cpu	NULL
> > +#define rcutree_online_cpu	NULL
> > +#define rcutree_offline_cpu	NULL
> > +#define rcutree_dead_cpu	NULL
> > +#define rcutree_dying_cpu	NULL
> > +#endif
> 
> This file is included only in CONFIG_TREE_RCU or CONFIG_PREEMPT_RCU
> builds, so you should not need this ifdef.
> 
> The only other option is CONFIG_TINY_RCU, for which CONFIG_HOTPLUG_CPU
> cannot possibly be set.
> 
> > +
> >  #endif /* __LINUX_RCUTREE_H */
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/tick.h>
> >  #include <linux/irq.h>
> >  #include <linux/smpboot.h>
> > +#include <linux/rcutree.h>
> 
> Ah, I see...  ;-)
> 
> I am going to guess that this code was never built for CONFIG_SMP=n...
> I would expect a few build errors.
> 
> I suggest moving the #ifdef from include/linux/rcutree.h to
> include/linux/cpu.h.  That way, you avoid including code intended
> only for Tree RCU into Tiny RCU builds.
>

Is it ok, to leave the defines without ifdef in
include/linux/rcutree.h and remove the include rcutree.h in
kernel/cpu.c ?  Because only if CONFIG_TREE_RCU or CONFIG_PREEMPT_RCU
is defined, rcupdate.h includes rcutree.h . See delta patch below.


8<----------------

--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -112,18 +112,10 @@ bool rcu_is_watching(void);
 void rcu_all_qs(void);
 
 /* RCUtree hotplug events */
-#if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU)
 int rcutree_prepare_cpu(unsigned int cpu);
 int rcutree_online_cpu(unsigned int cpu);
 int rcutree_offline_cpu(unsigned int cpu);
 int rcutree_dead_cpu(unsigned int cpu);
 int rcutree_dying_cpu(unsigned int cpu);
-#else
-#define rcutree_prepare_cpu	NULL
-#define rcutree_online_cpu	NULL
-#define rcutree_offline_cpu	NULL
-#define rcutree_dead_cpu	NULL
-#define rcutree_dying_cpu	NULL
-#endif
 
 #endif /* __LINUX_RCUTREE_H */
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -23,7 +23,6 @@
 #include <linux/tick.h>
 #include <linux/irq.h>
 #include <linux/smpboot.h>
-#include <linux/rcutree.h>
 
 #include <trace/events/power.h>
 #define CREATE_TRACE_POINTS