[PATCH] RCU: remove alignment padding from rcu_data on 64 bit builds

[PATCH] RCU: remove alignment padding from rcu_data on 64 bit builds

[Richard Kennedy]

Reorder rcu_data to remove 8 bytes of alignment padding on 64 bit builds
so saving a total of 16 bytes per cpu, (rcu_sched_data & rcu_bh_data).
    
When CONFIG_NO_HZ is set, it shrinks the size of this structure from 264
to 256 bytes allowing it to fit into one fewer cache lines.

Signed-off-by: Richard Kennedy <richard@rsk.demon.co.uk>
---

patch against v2.6.39-rc6
compiled & tested on x86_64

I've been running with this patch for some weeks and have not seen any
issues.

regards
Richard



diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index e8f057e..fcced0f 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -176,6 +176,9 @@ struct rcu_data {
 	bool		qs_pending;	/* Core waits for quiesc state. */
 	bool		beenonline;	/* CPU online at least once. */
 	bool		preemptable;	/* Preemptable RCU? */
+
+	int cpu;
+
 	struct rcu_node *mynode;	/* This CPU's leaf of hierarchy */
 	unsigned long grpmask;		/* Mask to apply to leaf qsmask. */
 
@@ -238,8 +241,6 @@ struct rcu_data {
 	unsigned long n_rp_gp_started;
 	unsigned long n_rp_need_fqs;
 	unsigned long n_rp_need_nothing;
-
-	int cpu;
 };
 
 /* Values for signaled field in struct rcu_state. */

Re: [PATCH] RCU: remove alignment padding from rcu_data on 64 bit builds

[Eric Dumazet]

Le jeudi 05 mai 2011 à 16:50 +0100, Richard Kennedy a écrit :
> Reorder rcu_data to remove 8 bytes of alignment padding on 64 bit builds
> so saving a total of 16 bytes per cpu, (rcu_sched_data & rcu_bh_data).
>     
> When CONFIG_NO_HZ is set, it shrinks the size of this structure from 264
> to 256 bytes allowing it to fit into one fewer cache lines.
> 
> Signed-off-by: Richard Kennedy <richard@rsk.demon.co.uk>
> ---
> 
> patch against v2.6.39-rc6
> compiled & tested on x86_64
> 
> I've been running with this patch for some weeks and have not seen any
> issues.
> 
> regards
> Richard
> 
> 
> 
> diff --git a/kernel/rcutree.h b/kernel/rcutree.h
> index e8f057e..fcced0f 100644
> --- a/kernel/rcutree.h
> +++ b/kernel/rcutree.h
> @@ -176,6 +176,9 @@ struct rcu_data {
>  	bool		qs_pending;	/* Core waits for quiesc state. */
>  	bool		beenonline;	/* CPU online at least once. */
>  	bool		preemptable;	/* Preemptable RCU? */
> +
> +	int cpu;
> +
>  	struct rcu_node *mynode;	/* This CPU's leaf of hierarchy */
>  	unsigned long grpmask;		/* Mask to apply to leaf qsmask. */
>  
> @@ -238,8 +241,6 @@ struct rcu_data {
>  	unsigned long n_rp_gp_started;
>  	unsigned long n_rp_need_fqs;
>  	unsigned long n_rp_need_nothing;
> -
> -	int cpu;
>  };
>  

We could shrink this structure more, converting some 64bit fields to
32bit ones.

Rationale is the algo is working well on 32bit arches, no need to use
64bit fields.

candidates : completed, gpnum, passed_quiesc_completed, qlen,
qlen_last_fqs_check, blimit.

Counters might be converted too.

Re: [PATCH] RCU: remove alignment padding from rcu_data on 64 bit builds

[Richard Kennedy]

On Fri, 2011-05-06 at 08:20 +0200, Eric Dumazet wrote:
...
> We could shrink this structure more, converting some 64bit fields to
> 32bit ones.
> 
> Rationale is the algo is working well on 32bit arches, no need to use
> 64bit fields.
> 
> candidates : completed, gpnum, passed_quiesc_completed, qlen,
> qlen_last_fqs_check, blimit.
> 
> Counters might be converted too.

I don't know the code well enough to make that decision ;)
Is it possible to shrink it enough to free up another cache line ?
(16 longs to ints).

CONFIG_NO_HZ adds 24 bytes so even for users with !CONFIG_NO_HZ the
variables you suggest are not enough to free a cache line.  

regards
Richard

Re: [PATCH] RCU: remove alignment padding from rcu_data on 64 bit builds

[Paul E. McKenney]

On Fri, May 06, 2011 at 01:13:56PM +0100, Richard Kennedy wrote:
> On Fri, 2011-05-06 at 08:20 +0200, Eric Dumazet wrote:
> ...
> > We could shrink this structure more, converting some 64bit fields to
> > 32bit ones.
> > 
> > Rationale is the algo is working well on 32bit arches, no need to use
> > 64bit fields.
> > 
> > candidates : completed, gpnum, passed_quiesc_completed, qlen,
> > qlen_last_fqs_check, blimit.
> > 
> > Counters might be converted too.
> 
> I don't know the code well enough to make that decision ;)
> Is it possible to shrink it enough to free up another cache line ?
> (16 longs to ints).

A number of these could be changed from long to int, though appropriate
adjustments need to be made.  Some of the fields could be placed under
#ifdef CONFIG_RCU_TRACE, though again, corresponding adjustments would
need to be made.

> CONFIG_NO_HZ adds 24 bytes so even for users with !CONFIG_NO_HZ the
> variables you suggest are not enough to free a cache line.  

Yep.  And recent work introducing RCU priority boosting adds some more.

But it is not like this structure is allocated as a unit, so are you
really all that sensitive to the exact size?

							Thanx, Paul