RCU fixes for 3.9
cover RCU fixes for 3.9
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 3/6 rcu: Use new nesting value for rcu_dyntick trace in rcu_eqs_enter_common
In reply to Paul E. McKenneyFrom: Li Zhong <[email protected]>
This patch uses the real new value of dynticks_nesting instead of 0 in rcu_eqs_enter_common().
--- kernel/rcutree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree.c +1 -1
@@ -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 ());
Signature · 1 line
1.7.8
PATCH 5/6 rcutorture: don't compare ptr with 0
In reply to Paul E. McKenneyFrom: Sasha Levin <[email protected]>
--- kernel/rcutorture.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutorture.c +1 -1
@@ -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]);
Signature · 1 line
1.7.8
PATCH 4/6 rcu: Silence compiler array out-of-bounds false positive
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
kernel/rcutree.c +4 -0
@@ -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++)
Signature · 1 line
1.7.8
PATCH 1/6 rcu: Fix blimit type for trace_rcu_batch_start()
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
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.
--- include/trace/events/rcu.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
include/trace/events/rcu.h +3 -3
@@ -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) );
Signature · 1 line
1.7.8
PATCH 6/6 rcu: Correct 'optimized' to 'optimize' in header comment
From: Cody P Schafer <[email protected]>
Small grammar fix in rcutree comment regarding 'rcu_scheduler_active' var.
--- kernel/rcutree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/rcutree.c +1 -1
@@ -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.
Signature · 1 line
1.7.8
PATCH 2/6 rcu: Make rcu_is_cpu_rrupt_from_idle helper functions static
In reply to Paul E. McKenneyFrom: Josh Triplett <[email protected]>
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.
--- kernel/rcutiny.c | 2 +- kernel/rcutree.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
kernel/rcutiny.c +1 -1
@@ -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 ; }
kernel/rcutree.c +1 -1
@@ -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 ; }
Signature · 1 line
1.7.8
Quoted 11 lines · Paul E. McKenney
Hello!
The following fixes are intended for 3.9:
1. Fix int/long type confusion in
trace_rcu_start_batch().
2. Declarercu_is_cpu_rrupt_from_idle()static, courtesy of
Josh Triplett.
3. Makercu_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 inrcu_init_one().
5. Code style fix inrcu_torture_barrier_init(), courtesy of Sasha Levin.
and:
6. Grammar fix to rcu_scheduler_active comment, courtesy of Cody
Schafer.
Quoted 8 lines
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
In reply to Paul E. McKenneyQuoted 24 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0
kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 6 lines
kernel/rcutree.c +3
-0@@ -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
Josh Triplett
Reviewed-byQuoted 13 lines · Paul E. McKenney
Quoted 11 lines · Paul E. McKenney
Hello!
The following fixes are intended for 3.9:
1. Fix int/long type confusion in
trace_rcu_start_batch().
2. Declarercu_is_cpu_rrupt_from_idle()static, courtesy of
Josh Triplett.
3. Makercu_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 inrcu_init_one().
5. Code style fix inrcu_torture_barrier_init(), courtesy of Sasha Levin.
Quoted 4 lines
and:
6. Grammar fix to
rcu_scheduler_activecomment, courtesy of Cody
Schafer.
One comment on patch 4/6, but in any case,
for the whole series.
Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive
In reply to Josh TriplettQuoted 28 lines · Josh Triplett
Quoted 27 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0@@ -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! */
Quoted 7 lines
kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 3 lines
kernel/rcutree.c +3
-0+/* 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!
Quoted 9 lines
I do find it surprising, though, that the compiler can't figure this one out, given that
rcu_num_lvlsgets 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, sincercu_num_lvlsdoesn't get declared as static? Unfortunately, the loop macros inrcutree.hmake it difficult to makercu_num_lvlsstatic, but as far as I can tell only one use of those macros ever gets expanded outside ofrcutree.c: the one inrcutree_trace.c. If you compile out tracing, and declarercu_num_lvlsstatic, 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
Quoted 37 lines · Paul E. McKenney
Quoted 28 lines · Josh Triplett
Quoted 27 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0@@ -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! */ Quoted 7 lines
kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 3 lines
kernel/rcutree.c +3
-0+/* 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.
Quoted 1 line
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
Quoted 15 lines
Quoted 9 lines
I do find it surprising, though, that the compiler can't figure this one out, given that
rcu_num_lvlsgets 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, sincercu_num_lvlsdoesn't get declared as static? Unfortunately, the loop macros inrcutree.hmake it difficult to makercu_num_lvlsstatic, but as far as I can tell only one use of those macros ever gets expanded outside ofrcutree.c: the one inrcutree_trace.c. If you compile out tracing, and declarercu_num_lvlsstatic, 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
In reply to Paul E. McKenneyQuoted 43 lines · Paul E. McKenney
Quoted 28 lines · Josh Triplett
Quoted 27 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0@@ -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! */ Quoted 13 lines
kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 3 lines
kernel/rcutree.c +3
-0+/* 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_lvlsgets 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, sincercu_num_lvlsdoesn't get declared as static? Unfortunately, the loop macros inrcutree.hmake it difficult to makercu_num_lvlsstatic, but as far as I can tell only one use of those macros ever gets expanded outside ofrcutree.c: the one inrcutree_trace.c. If you compile out tracing, and declarercu_num_lvlsstatic, does the warning go away?
Quoted 1 line
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.
Signature · 1 line
Markus
Re: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive
Quoted 44 lines · Markus Trippelsdorf
Quoted 43 lines · Paul E. McKenney
Quoted 41 lines · Josh Triplett
Quoted 24 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 6 lines
kernel/rcutree.c +3
-0@@ -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_lvlsgets 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, sincercu_num_lvlsdoesn't get declared as static? Unfortunately, the loop macros inrcutree.hmake it difficult to makercu_num_lvlsstatic, but as far as I can tell only one use of those macros ever gets expanded outside ofrcutree.c: the one inrcutree_trace.c. If you compile out tracing, and declarercu_num_lvlsstatic, does the warning go away?
Quoted 4 lines
Quoted 1 line
I found it quite surprising also, hence the "array-index anxieties" above.
Yes, declaring
rcu_num_lvlsstatic 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
Quoted 45 lines · Josh Triplett
Quoted 44 lines · Markus Trippelsdorf
Quoted 43 lines · Paul E. McKenney
Quoted 41 lines · Josh Triplett
Quoted 24 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 6 lines
kernel/rcutree.c +3
-0@@ -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_lvlsgets 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, sincercu_num_lvlsdoesn't get declared as static? Unfortunately, the loop macros inrcutree.hmake it difficult to makercu_num_lvlsstatic, but as far as I can tell only one use of those macros ever gets expanded outside ofrcutree.c: the one inrcutree_trace.c. If you compile out tracing, and declarercu_num_lvlsstatic, does the warning go away?
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
I found it quite surprising also, hence the "array-index anxieties" above.
Yes, declaring
rcu_num_lvlsstatic would fix the issue. See the following gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529Seems potentially worth restructuring the code a bit to fold in the one
rcutree_trace.cbit that needs it and then makingrcu_num_lvlsstatic and internal torcutree.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
Steven Rostedt
maintainerRe: [PATCH tip/core/rcu 4/6] rcu: Silence compiler array out-of-bounds false positive
In reply to Paul E. McKenneyQuoted 40 lines · Paul E. McKenney
Quoted 37 lines · Paul E. McKenney
Quoted 28 lines · Josh Triplett
Quoted 27 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0@@ -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! */ Quoted 7 lines
kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 3 lines
kernel/rcutree.c +3
-0+/* 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.Quoted 1 line
Good point, will do!
Quoted 7 lines
Ah, wait,
BUG_ON()sometimes compiles to nothingness:#ifndef
HAVE_ARCH_BUG_ON
#defineBUG_ON(condition)do { if (condition) ; } while(0) #endifSo 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
Quoted 43 lines · Steven Rostedt
Quoted 42 lines · Paul E. McKenney
Quoted 37 lines · Paul E. McKenney
Quoted 28 lines · Josh Triplett
Quoted 27 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
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 <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)kernel/rcutree.c +0
-0@@ -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! */ Quoted 7 lines
kernel/rcutree.c static void __init rcu_init_one(struct rcu_state *rsp, · line 2943
Quoted 3 lines
kernel/rcutree.c +3
-0+/* 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.Quoted 1 line
Good point, will do!
Ah, wait,
BUG_ON()sometimes compiles to nothingness:
Quoted 8 lines
Quoted 5 lines
#ifndef
HAVE_ARCH_BUG_ON
#defineBUG_ON(condition)do { if (condition) ; } while(0) #endifSo 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 :-)
;-) ;-) ;-)
Quoted 2 lines
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