Re: [PATCH] rcu: declare rcu_data variables in the section they are defined in

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Tue, May 05, 2015 at 08:40:55AM +0800, Nicolas Iooss wrote:
> On 05/05/2015 04:33 AM, Paul E. McKenney wrote:
> > On Sun, May 03, 2015 at 12:27:02PM -0700, Josh Triplett wrote:
> >> On Sun, May 03, 2015 at 05:57:53PM +0800, Nicolas Iooss wrote:
> >>> Commit 11bbb235c26f ("rcu: Use DEFINE_PER_CPU_SHARED_ALIGNED for
> >>> rcu_data") replaced DEFINE_PER_CPU by DEFINE_PER_CPU_SHARED_ALIGNED in
> >>> the definition of rcu_sched and rcu_bh without updating
> >>> kernel/rcu/tree.h.
> >>>
> >>> This makes clang report a section mismatch (-Wsection warning) when
> >>> building LLVMLinux because the variables are declared in .data..percpu
> >>> but defined in .data..percpu..shared_aligned.
> >>>
> >>> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> >>
> >> Good catch.
> >> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > Agreed, good catch!  But don't we also need to worry about
> > rcu_preempt_data?
> 
> Yes, I missed it because I didn't know that allyesconfig/allmodconfig
> does not select CONFIG_PREEMPT (it selects CONFIG_PREEMPT_NONE instead).
> 
> > Also, given that tree_trace.c now uses iterators
> > rather than direct access via the per-CPU variables, wouldn't the
> > following be more appropriate?  (-Very- lightly tested.)
> 
> This doesn't work with CONFIG_TREE_RCU_TRACE, because
> kernel/rcu/tree_trace.c uses rcu_preempt_state (in v4.1-rc2).  I've
> successfully built an allmodconfig+CONFIG_PREEMPT kernel for x86_64 with
> the following patch (I have not tested the result).
> 
> Anyway, thanks for your quick replies.

Good point, and your patch does look better.

If you resend with proper changelog and Signed-off-by, and if Josh is
good with the change, I will queue it.

							Thanx, Paul

> Nicolas
> 
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -91,7 +91,7 @@ static const char *tp_##sname##_varname __used
> __tracepoint_string = sname##_var
> 
>  #define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
>  DEFINE_RCU_TPS(sname) \
> -DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, sname##_data); \
> +static DEFINE_PER_CPU_SHARED_ALIGNED(struct rcu_data, sname##_data); \
>  struct rcu_state sname##_state = { \
>  	.level = { &sname##_state.node[0] }, \
>  	.rda = &sname##_data, \
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -519,14 +519,11 @@ extern struct list_head rcu_struct_flavors;
>   * RCU implementation internal declarations:
>   */
>  extern struct rcu_state rcu_sched_state;
> -DECLARE_PER_CPU(struct rcu_data, rcu_sched_data);
> 
>  extern struct rcu_state rcu_bh_state;
> -DECLARE_PER_CPU(struct rcu_data, rcu_bh_data);
> 
>  #ifdef CONFIG_PREEMPT_RCU
>  extern struct rcu_state rcu_preempt_state;
> -DECLARE_PER_CPU(struct rcu_data, rcu_preempt_data);
>  #endif /* #ifdef CONFIG_PREEMPT_RCU */
> 
>  #ifdef CONFIG_RCU_BOOST
>