(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 <[email protected]> > > > > 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 <[email protected]> > > Reviewed-by: Sebastian Andrzej Siewior <[email protected]> > > Cc: "Paul E. McKenney" <[email protected]> > > Signed-off-by: Anna-Maria Gleixner <[email protected]> > > 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
