[PATCH tip/core/rcu 0/6] RCU fixes for 3.9

[PATCH tip/core/rcu 0/6] RCU fixes for 3.9

[Paul E. McKenney]

Hello!

The following fixes are intended for 3.9:

1.	Fix int/long type confusion in trace_rcu_start_batch().
2.	Declare rcu_is_cpu_rrupt_from_idle() static, courtesy of
	Josh Triplett.
3.	Make rcu_eqs_enter_common() trace the new nesting value instead
	of zero, courtesy of Li Zhong.
4.	Silence a gcc array-out-of-bounds false positive in rcu_init_one().
5.	Code style fix in rcu_torture_barrier_init(), courtesy of Sasha Levin.

								Thanx, Paul


 b/include/trace/events/rcu.h |    6 +++---
 b/kernel/rcutiny.c           |    2 +-
 b/kernel/rcutorture.c        |    2 +-
 b/kernel/rcutree.c           |   10 +++++++---
 4 files changed, 12 insertions(+), 8 deletions(-)

[PATCH tip/core/rcu 1/6] rcu: Fix blimit type for trace_rcu_batch_start()

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

When the type of global variable blimit changed from int to long, the
type of the blimit argument of trace_rcu_batch_start() needed to have
changed.  This commit fixes this issue.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/trace/events/rcu.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
index d4f559b..f919498 100644
--- a/include/trace/events/rcu.h
+++ b/include/trace/events/rcu.h
@@ -393,7 +393,7 @@ TRACE_EVENT(rcu_kfree_callback,
  */
 TRACE_EVENT(rcu_batch_start,
 
-	TP_PROTO(char *rcuname, long qlen_lazy, long qlen, int blimit),
+	TP_PROTO(char *rcuname, long qlen_lazy, long qlen, long blimit),
 
 	TP_ARGS(rcuname, qlen_lazy, qlen, blimit),
 
@@ -401,7 +401,7 @@ TRACE_EVENT(rcu_batch_start,
 		__field(char *, rcuname)
 		__field(long, qlen_lazy)
 		__field(long, qlen)
-		__field(int, blimit)
+		__field(long, blimit)
 	),
 
 	TP_fast_assign(
@@ -411,7 +411,7 @@ TRACE_EVENT(rcu_batch_start,
 		__entry->blimit = blimit;
 	),
 
-	TP_printk("%s CBs=%ld/%ld bl=%d",
+	TP_printk("%s CBs=%ld/%ld bl=%ld",
 		  __entry->rcuname, __entry->qlen_lazy, __entry->qlen,
 		  __entry->blimit)
 );
-- 
1.7.8

[PATCH tip/core/rcu 2/6] rcu: Make rcu_is_cpu_rrupt_from_idle helper functions static

[Paul E. McKenney]

From: Josh Triplett <josh@joshtriplett.org>

Both rcutiny and rcutree define a helper function named
rcu_is_cpu_rrupt_from_idle(), each used exactly once, later in the
same file.  This commit therefore declares these helper functions static.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutiny.c |    2 +-
 kernel/rcutree.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
index e7dce58..9f72a0f 100644
--- a/kernel/rcutiny.c
+++ b/kernel/rcutiny.c
@@ -193,7 +193,7 @@ EXPORT_SYMBOL(rcu_is_cpu_idle);
  * interrupts don't count, we must be running at the first interrupt
  * level.
  */
-int rcu_is_cpu_rrupt_from_idle(void)
+static int rcu_is_cpu_rrupt_from_idle(void)
 {
 	return rcu_dynticks_nesting <= 1;
 }
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index e441b77..cceda76 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -727,7 +727,7 @@ EXPORT_SYMBOL_GPL(rcu_lockdep_current_cpu_online);
  * interrupt from idle, return true.  The caller must have at least
  * disabled preemption.
  */
-int rcu_is_cpu_rrupt_from_idle(void)
+static int rcu_is_cpu_rrupt_from_idle(void)
 {
 	return __get_cpu_var(rcu_dynticks).dynticks_nesting <= 1;
 }
-- 
1.7.8

[PATCH tip/core/rcu 3/6] rcu: Use new nesting value for rcu_dyntick trace in rcu_eqs_enter_common

[Paul E. McKenney]

From: Li Zhong <zhong@linux.vnet.ibm.com>

This patch uses the real new value of dynticks_nesting instead of 0 in
rcu_eqs_enter_common().

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index cceda76..d145796 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -336,7 +336,7 @@ static struct rcu_node *rcu_get_root(struct rcu_state *rsp)
 static void rcu_eqs_enter_common(struct rcu_dynticks *rdtp, long long oldval,
 				bool user)
 {
-	trace_rcu_dyntick("Start", oldval, 0);
+	trace_rcu_dyntick("Start", oldval, rdtp->dynticks_nesting);
 	if (!user && !is_idle_task(current)) {
 		struct task_struct *idle = idle_task(smp_processor_id());
 
-- 
1.7.8

[PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

It turns out that gcc 4.8 warns on array indexes being out of bounds
unless it can prove otherwise.  It gives this warning on some RCU
initialization code.  Because this is far from any fastpath, add
an explicit check for array bounds and panic if so.  This gives the
compiler enough information to figure out that the array index is never
out of bounds.

However, if a similar false positive occurs on a fastpath, it will
probably be necessary to tell the compiler to keep its array-index
anxieties to itself.  ;-)

Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index d145796..e0d9815 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
 
 	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
 
+	/* Silence gcc 4.8 warning about array index out of range. */
+	if (rcu_num_lvls > RCU_NUM_LVLS)
+		panic("rcu_init_one: rcu_num_lvls overflow");
+
 	/* Initialize the level-tracking arrays. */
 
 	for (i = 0; i < rcu_num_lvls; i++)
-- 
1.7.8

[PATCH tip/core/rcu 5/6] rcutorture: don't compare ptr with 0

[Paul E. McKenney]

From: Sasha Levin <sasha.levin@oracle.com>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutorture.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 31dea01..0249800 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -1749,7 +1749,7 @@ static int rcu_torture_barrier_init(void)
 	barrier_cbs_wq =
 		kzalloc(n_barrier_cbs * sizeof(barrier_cbs_wq[0]),
 			GFP_KERNEL);
-	if (barrier_cbs_tasks == NULL || barrier_cbs_wq == 0)
+	if (barrier_cbs_tasks == NULL || !barrier_cbs_wq)
 		return -ENOMEM;
 	for (i = 0; i < n_barrier_cbs; i++) {
 		init_waitqueue_head(&barrier_cbs_wq[i]);
-- 
1.7.8

[PATCH tip/core/rcu 6/6] rcu: Correct 'optimized' to 'optimize' in header comment

[Paul E. McKenney]

From: Cody P Schafer <cody@linux.vnet.ibm.com>

Small grammar fix in rcutree comment regarding 'rcu_scheduler_active'
var.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index e0d9815..d78ba60 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -105,7 +105,7 @@ int rcu_num_nodes __read_mostly = NUM_RCU_NODES; /* Total # rcu_nodes in use. */
  * The rcu_scheduler_active variable transitions from zero to one just
  * before the first task is spawned.  So when this variable is zero, RCU
  * can assume that there is but one task, allowing RCU to (for example)
- * optimized synchronize_sched() to a simple barrier().  When this variable
+ * optimize synchronize_sched() to a simple barrier().  When this variable
  * is one, RCU must actually do all the hard work required to detect real
  * grace periods.  This variable is also used to suppress boot-time false
  * positives from lockdep-RCU error checking.
-- 
1.7.8

Re: [PATCH tip/core/rcu 0/6] RCU fixes for 3.9

[Paul E. McKenney]

On Sat, Jan 05, 2013 at 09:09:20AM -0800, Paul E. McKenney wrote:
> Hello!
> 
> The following fixes are intended for 3.9:
> 
> 1.	Fix int/long type confusion in trace_rcu_start_batch().
> 2.	Declare rcu_is_cpu_rrupt_from_idle() static, courtesy of
> 	Josh Triplett.
> 3.	Make rcu_eqs_enter_common() trace the new nesting value instead
> 	of zero, courtesy of Li Zhong.
> 4.	Silence a gcc array-out-of-bounds false positive in rcu_init_one().
> 5.	Code style fix in rcu_torture_barrier_init(), courtesy of Sasha Levin.

and:

6.	Grammar fix to rcu_scheduler_active comment, courtesy of Cody
	Schafer.

> 								Thanx, Paul
> 
> 
>  b/include/trace/events/rcu.h |    6 +++---
>  b/kernel/rcutiny.c           |    2 +-
>  b/kernel/rcutorture.c        |    2 +-
>  b/kernel/rcutree.c           |   10 +++++++---
>  4 files changed, 12 insertions(+), 8 deletions(-)

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Josh Triplett]

On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> It turns out that gcc 4.8 warns on array indexes being out of bounds
> unless it can prove otherwise.  It gives this warning on some RCU
> initialization code.  Because this is far from any fastpath, add
> an explicit check for array bounds and panic if so.  This gives the
> compiler enough information to figure out that the array index is never
> out of bounds.
> 
> However, if a similar false positive occurs on a fastpath, it will
> probably be necessary to tell the compiler to keep its array-index
> anxieties to itself.  ;-)
> 
> Markus Trippelsdorf <markus@trippelsdorf.de>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutree.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index d145796..e0d9815 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
>  
>  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
>  
> +	/* Silence gcc 4.8 warning about array index out of range. */
> +	if (rcu_num_lvls > RCU_NUM_LVLS)
> +		panic("rcu_init_one: rcu_num_lvls overflow");

Why not write this as BUG_ON(rcu_num_lvls > RCU_NUM_LVLS)?  Given that
the condition in question can never happen, you don't really need an
explanatory message.

I do find it surprising, though, that the compiler can't figure this one
out, given that rcu_num_lvls gets initialized right before this in the
same file (and likely inlined into the same function).  I wonder if it
thought some other code might change it unexpectedly, since rcu_num_lvls
doesn't get declared as static?  Unfortunately, the loop macros in
rcutree.h make it difficult to make rcu_num_lvls static, but as far as I
can tell only one use of those macros ever gets expanded outside of
rcutree.c: the one in rcutree_trace.c.  If you compile out tracing, and
declare rcu_num_lvls static, does the warning go away?

- Josh Triplett

Re: [PATCH tip/core/rcu 0/6] RCU fixes for 3.9

[Josh Triplett]

On Sat, Jan 05, 2013 at 09:19:16AM -0800, Paul E. McKenney wrote:
> On Sat, Jan 05, 2013 at 09:09:20AM -0800, Paul E. McKenney wrote:
> > Hello!
> > 
> > The following fixes are intended for 3.9:
> > 
> > 1.	Fix int/long type confusion in trace_rcu_start_batch().
> > 2.	Declare rcu_is_cpu_rrupt_from_idle() static, courtesy of
> > 	Josh Triplett.
> > 3.	Make rcu_eqs_enter_common() trace the new nesting value instead
> > 	of zero, courtesy of Li Zhong.
> > 4.	Silence a gcc array-out-of-bounds false positive in rcu_init_one().
> > 5.	Code style fix in rcu_torture_barrier_init(), courtesy of Sasha Levin.
> 
> and:
> 
> 6.	Grammar fix to rcu_scheduler_active comment, courtesy of Cody
> 	Schafer.

One comment on patch 4/6, but in any case,
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
for the whole series.

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Paul E. McKenney]

On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > unless it can prove otherwise.  It gives this warning on some RCU
> > initialization code.  Because this is far from any fastpath, add
> > an explicit check for array bounds and panic if so.  This gives the
> > compiler enough information to figure out that the array index is never
> > out of bounds.
> > 
> > However, if a similar false positive occurs on a fastpath, it will
> > probably be necessary to tell the compiler to keep its array-index
> > anxieties to itself.  ;-)
> > 
> > Markus Trippelsdorf <markus@trippelsdorf.de>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutree.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index d145796..e0d9815 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> >  
> >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> >  
> > +	/* Silence gcc 4.8 warning about array index out of range. */
> > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > +		panic("rcu_init_one: rcu_num_lvls overflow");
> 
> Why not write this as BUG_ON(rcu_num_lvls > RCU_NUM_LVLS)?  Given that
> the condition in question can never happen, you don't really need an
> explanatory message.

Good point, will do!

> I do find it surprising, though, that the compiler can't figure this one
> out, given that rcu_num_lvls gets initialized right before this in the
> same file (and likely inlined into the same function).  I wonder if it
> thought some other code might change it unexpectedly, since rcu_num_lvls
> doesn't get declared as static?  Unfortunately, the loop macros in
> rcutree.h make it difficult to make rcu_num_lvls static, but as far as I
> can tell only one use of those macros ever gets expanded outside of
> rcutree.c: the one in rcutree_trace.c.  If you compile out tracing, and
> declare rcu_num_lvls static, does the warning go away?

I found it quite surprising also, hence the "array-index anxieties" above.

I added Marcus on CC for his thoughts on this.

							Thanx, Paul

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Paul E. McKenney]

On Mon, Jan 07, 2013 at 09:16:02AM -0800, Paul E. McKenney wrote:
> On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> > On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > 
> > > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > > unless it can prove otherwise.  It gives this warning on some RCU
> > > initialization code.  Because this is far from any fastpath, add
> > > an explicit check for array bounds and panic if so.  This gives the
> > > compiler enough information to figure out that the array index is never
> > > out of bounds.
> > > 
> > > However, if a similar false positive occurs on a fastpath, it will
> > > probably be necessary to tell the compiler to keep its array-index
> > > anxieties to itself.  ;-)
> > > 
> > > Markus Trippelsdorf <markus@trippelsdorf.de>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > ---
> > >  kernel/rcutree.c |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > index d145796..e0d9815 100644
> > > --- a/kernel/rcutree.c
> > > +++ b/kernel/rcutree.c
> > > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> > >  
> > >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> > >  
> > > +	/* Silence gcc 4.8 warning about array index out of range. */
> > > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > > +		panic("rcu_init_one: rcu_num_lvls overflow");
> > 
> > Why not write this as BUG_ON(rcu_num_lvls > RCU_NUM_LVLS)?  Given that
> > the condition in question can never happen, you don't really need an
> > explanatory message.
> 
> Good point, will do!

Ah, wait, BUG_ON() sometimes compiles to nothingness:

#ifndef HAVE_ARCH_BUG_ON
#define BUG_ON(condition) do { if (condition) ; } while(0)
#endif

So I do need the explicit "if".  :-(

							Thanx, Paul

> > I do find it surprising, though, that the compiler can't figure this one
> > out, given that rcu_num_lvls gets initialized right before this in the
> > same file (and likely inlined into the same function).  I wonder if it
> > thought some other code might change it unexpectedly, since rcu_num_lvls
> > doesn't get declared as static?  Unfortunately, the loop macros in
> > rcutree.h make it difficult to make rcu_num_lvls static, but as far as I
> > can tell only one use of those macros ever gets expanded outside of
> > rcutree.c: the one in rcutree_trace.c.  If you compile out tracing, and
> > declare rcu_num_lvls static, does the warning go away?
> 
> I found it quite surprising also, hence the "array-index anxieties" above.
> 
> I added Marcus on CC for his thoughts on this.
> 
> 							Thanx, Paul

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Markus Trippelsdorf]

On 2013.01.07 at 09:16 -0800, Paul E. McKenney wrote:
> On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> > On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > 
> > > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > > unless it can prove otherwise.  It gives this warning on some RCU
> > > initialization code.  Because this is far from any fastpath, add
> > > an explicit check for array bounds and panic if so.  This gives the
> > > compiler enough information to figure out that the array index is never
> > > out of bounds.
> > > 
> > > However, if a similar false positive occurs on a fastpath, it will
> > > probably be necessary to tell the compiler to keep its array-index
> > > anxieties to itself.  ;-)
> > > 
> > > Markus Trippelsdorf <markus@trippelsdorf.de>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > ---
> > >  kernel/rcutree.c |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > index d145796..e0d9815 100644
> > > --- a/kernel/rcutree.c
> > > +++ b/kernel/rcutree.c
> > > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> > >  
> > >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> > >  
> > > +	/* Silence gcc 4.8 warning about array index out of range. */
> > > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > > +		panic("rcu_init_one: rcu_num_lvls overflow");
> 
> > I do find it surprising, though, that the compiler can't figure this one
> > out, given that rcu_num_lvls gets initialized right before this in the
> > same file (and likely inlined into the same function).  I wonder if it
> > thought some other code might change it unexpectedly, since rcu_num_lvls
> > doesn't get declared as static?  Unfortunately, the loop macros in
> > rcutree.h make it difficult to make rcu_num_lvls static, but as far as I
> > can tell only one use of those macros ever gets expanded outside of
> > rcutree.c: the one in rcutree_trace.c.  If you compile out tracing, and
> > declare rcu_num_lvls static, does the warning go away?
> 
> I found it quite surprising also, hence the "array-index anxieties" above.

Yes, declaring rcu_num_lvls static would fix the issue. See the
following gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529

Please note that this was the only warning of this kind that occurred
with an allyesconfig.

-- 
Markus

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Josh Triplett]

On Mon, Jan 07, 2013 at 07:08:55PM +0100, Markus Trippelsdorf wrote:
> On 2013.01.07 at 09:16 -0800, Paul E. McKenney wrote:
> > On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> > > On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > 
> > > > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > > > unless it can prove otherwise.  It gives this warning on some RCU
> > > > initialization code.  Because this is far from any fastpath, add
> > > > an explicit check for array bounds and panic if so.  This gives the
> > > > compiler enough information to figure out that the array index is never
> > > > out of bounds.
> > > > 
> > > > However, if a similar false positive occurs on a fastpath, it will
> > > > probably be necessary to tell the compiler to keep its array-index
> > > > anxieties to itself.  ;-)
> > > > 
> > > > Markus Trippelsdorf <markus@trippelsdorf.de>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > ---
> > > >  kernel/rcutree.c |    4 ++++
> > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > > index d145796..e0d9815 100644
> > > > --- a/kernel/rcutree.c
> > > > +++ b/kernel/rcutree.c
> > > > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> > > >  
> > > >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> > > >  
> > > > +	/* Silence gcc 4.8 warning about array index out of range. */
> > > > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > > > +		panic("rcu_init_one: rcu_num_lvls overflow");
> > 
> > > I do find it surprising, though, that the compiler can't figure this one
> > > out, given that rcu_num_lvls gets initialized right before this in the
> > > same file (and likely inlined into the same function).  I wonder if it
> > > thought some other code might change it unexpectedly, since rcu_num_lvls
> > > doesn't get declared as static?  Unfortunately, the loop macros in
> > > rcutree.h make it difficult to make rcu_num_lvls static, but as far as I
> > > can tell only one use of those macros ever gets expanded outside of
> > > rcutree.c: the one in rcutree_trace.c.  If you compile out tracing, and
> > > declare rcu_num_lvls static, does the warning go away?
> > 
> > I found it quite surprising also, hence the "array-index anxieties" above.
> 
> Yes, declaring rcu_num_lvls static would fix the issue. See the
> following gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529

Seems potentially worth restructuring the code a bit to fold in the one
rcutree_trace.c bit that needs it and then making rcu_num_lvls static
and internal to rcutree.c.

- Josh Triplett

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Paul E. McKenney]

On Mon, Jan 07, 2013 at 10:24:17AM -0800, Josh Triplett wrote:
> On Mon, Jan 07, 2013 at 07:08:55PM +0100, Markus Trippelsdorf wrote:
> > On 2013.01.07 at 09:16 -0800, Paul E. McKenney wrote:
> > > On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> > > > On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > > 
> > > > > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > > > > unless it can prove otherwise.  It gives this warning on some RCU
> > > > > initialization code.  Because this is far from any fastpath, add
> > > > > an explicit check for array bounds and panic if so.  This gives the
> > > > > compiler enough information to figure out that the array index is never
> > > > > out of bounds.
> > > > > 
> > > > > However, if a similar false positive occurs on a fastpath, it will
> > > > > probably be necessary to tell the compiler to keep its array-index
> > > > > anxieties to itself.  ;-)
> > > > > 
> > > > > Markus Trippelsdorf <markus@trippelsdorf.de>
> > > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > > ---
> > > > >  kernel/rcutree.c |    4 ++++
> > > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > > > index d145796..e0d9815 100644
> > > > > --- a/kernel/rcutree.c
> > > > > +++ b/kernel/rcutree.c
> > > > > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> > > > >  
> > > > >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> > > > >  
> > > > > +	/* Silence gcc 4.8 warning about array index out of range. */
> > > > > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > > > > +		panic("rcu_init_one: rcu_num_lvls overflow");
> > > 
> > > > I do find it surprising, though, that the compiler can't figure this one
> > > > out, given that rcu_num_lvls gets initialized right before this in the
> > > > same file (and likely inlined into the same function).  I wonder if it
> > > > thought some other code might change it unexpectedly, since rcu_num_lvls
> > > > doesn't get declared as static?  Unfortunately, the loop macros in
> > > > rcutree.h make it difficult to make rcu_num_lvls static, but as far as I
> > > > can tell only one use of those macros ever gets expanded outside of
> > > > rcutree.c: the one in rcutree_trace.c.  If you compile out tracing, and
> > > > declare rcu_num_lvls static, does the warning go away?
> > > 
> > > I found it quite surprising also, hence the "array-index anxieties" above.
> > 
> > Yes, declaring rcu_num_lvls static would fix the issue. See the
> > following gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529
> 
> Seems potentially worth restructuring the code a bit to fold in the one
> rcutree_trace.c bit that needs it and then making rcu_num_lvls static
> and internal to rcutree.c.

One thing to keep in mind...  The added "if" is in initialization code
that runs once, and whose memory is reclaimed.  Restructuring the runtime
code will increase complexity and probably result in some degradation.

So I am not convinced that a change is really needed.

							Thanx, Paul

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Steven Rostedt]

On Mon, 2013-01-07 at 09:19 -0800, Paul E. McKenney wrote:
> On Mon, Jan 07, 2013 at 09:16:02AM -0800, Paul E. McKenney wrote:
> > On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> > > On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > 
> > > > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > > > unless it can prove otherwise.  It gives this warning on some RCU
> > > > initialization code.  Because this is far from any fastpath, add
> > > > an explicit check for array bounds and panic if so.  This gives the
> > > > compiler enough information to figure out that the array index is never
> > > > out of bounds.
> > > > 
> > > > However, if a similar false positive occurs on a fastpath, it will
> > > > probably be necessary to tell the compiler to keep its array-index
> > > > anxieties to itself.  ;-)
> > > > 
> > > > Markus Trippelsdorf <markus@trippelsdorf.de>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > ---
> > > >  kernel/rcutree.c |    4 ++++
> > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > > index d145796..e0d9815 100644
> > > > --- a/kernel/rcutree.c
> > > > +++ b/kernel/rcutree.c
> > > > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> > > >  
> > > >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> > > >  
> > > > +	/* Silence gcc 4.8 warning about array index out of range. */
> > > > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > > > +		panic("rcu_init_one: rcu_num_lvls overflow");
> > > 
> > > Why not write this as BUG_ON(rcu_num_lvls > RCU_NUM_LVLS)?  Given that
> > > the condition in question can never happen, you don't really need an
> > > explanatory message.
> > 
> > Good point, will do!
> 
> Ah, wait, BUG_ON() sometimes compiles to nothingness:
> 
> #ifndef HAVE_ARCH_BUG_ON
> #define BUG_ON(condition) do { if (condition) ; } while(0)
> #endif
> 
> So I do need the explicit "if".  :-(


Bah, those archs shouldn't be bothered with. If they don't want to bug,
then that's there problem :-)

Lots of places in the kernel have BUG_ON() where they require it to
panic.

-- Steve

Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive

[Paul E. McKenney]

On Mon, Jan 07, 2013 at 05:18:37PM -0500, Steven Rostedt wrote:
> On Mon, 2013-01-07 at 09:19 -0800, Paul E. McKenney wrote:
> > On Mon, Jan 07, 2013 at 09:16:02AM -0800, Paul E. McKenney wrote:
> > > On Mon, Jan 07, 2013 at 07:50:02AM -0800, Josh Triplett wrote:
> > > > On Sat, Jan 05, 2013 at 09:09:36AM -0800, Paul E. McKenney wrote:
> > > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > > > 
> > > > > It turns out that gcc 4.8 warns on array indexes being out of bounds
> > > > > unless it can prove otherwise.  It gives this warning on some RCU
> > > > > initialization code.  Because this is far from any fastpath, add
> > > > > an explicit check for array bounds and panic if so.  This gives the
> > > > > compiler enough information to figure out that the array index is never
> > > > > out of bounds.
> > > > > 
> > > > > However, if a similar false positive occurs on a fastpath, it will
> > > > > probably be necessary to tell the compiler to keep its array-index
> > > > > anxieties to itself.  ;-)
> > > > > 
> > > > > Markus Trippelsdorf <markus@trippelsdorf.de>
> > > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > > ---
> > > > >  kernel/rcutree.c |    4 ++++
> > > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > > > index d145796..e0d9815 100644
> > > > > --- a/kernel/rcutree.c
> > > > > +++ b/kernel/rcutree.c
> > > > > @@ -2938,6 +2938,10 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> > > > >  
> > > > >  	BUILD_BUG_ON(MAX_RCU_LVLS > ARRAY_SIZE(buf));  /* Fix buf[] init! */
> > > > >  
> > > > > +	/* Silence gcc 4.8 warning about array index out of range. */
> > > > > +	if (rcu_num_lvls > RCU_NUM_LVLS)
> > > > > +		panic("rcu_init_one: rcu_num_lvls overflow");
> > > > 
> > > > Why not write this as BUG_ON(rcu_num_lvls > RCU_NUM_LVLS)?  Given that
> > > > the condition in question can never happen, you don't really need an
> > > > explanatory message.
> > > 
> > > Good point, will do!
> > 
> > Ah, wait, BUG_ON() sometimes compiles to nothingness:
> > 
> > #ifndef HAVE_ARCH_BUG_ON
> > #define BUG_ON(condition) do { if (condition) ; } while(0)
> > #endif
> > 
> > So I do need the explicit "if".  :-(
> 
> Bah, those archs shouldn't be bothered with. If they don't want to bug,
> then that's there problem :-)

;-) ;-) ;-)

> Lots of places in the kernel have BUG_ON() where they require it to
> panic.

Fair point, but that doesn't mean that I want them complaining to
me when as a result of the compiler's array-index anxieties.

							Thanx, Paul