no-CB CPU changes for 3.8
PATCH 1/1 no-CB CPU changes for 3.8
Hello!
This patch series contains a couple of fixes to the no-CBs CPU feature, both courtesy of Paul Gortmaker:
1. Prevent soft-lockup complaints from no-CBs kthreads.
2. Make rcu_nocb_poll and early_param instead of a module parameter.
There is probably at least one more fix in this area.
Thanx, Paul
b/Documentation/kernel-parameters.txt | 2 +- b/kernel/rcutree_plugin.h | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-)
PATCH 1/2 rcu: Prevent soft-lockup complaints about no-CBs CPUs
From: Paul Gortmaker <[email protected]>
The wait_event() at the head of the rcu_nocb_kthread() can result in soft-lockup complaints if the CPU in question does not register RCU callbacks for an extended period. This commit therefore changes the wait_event() to a wait_event_interruptible().
--- kernel/rcutree_plugin.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
kernel/rcutree_plugin.h +2 -1
@@ -2366,10 +2366,11 @@ static int rcu_nocb_kthread(void *arg)for (;;) {/* If not polling, wait for next batch of callbacks. */ if (!rcu_nocb_poll) -wait_event (rdp->nocb_wq, rdp->nocb_head); +wait_event_interruptible (rdp->nocb_wq, rdp->nocb_head); list =ACCESS_ONCE (rdp->nocb_head);if (!list) {schedule_timeout_interruptible (1 ); +flush_signals (current);continue ; }
Signature · 1 line
1.7.8
PATCH 2/2 rcu: Make rcu_nocb_poll an early_param instead of module_param
From: Paul Gortmaker <[email protected]>
The as-documented rcu_nocb_poll will fail to enable this feature for two reasons. (1) there is an extra "s" in the documented name which is not in the code, and (2) since it uses module_param, it really is expecting a prefix, akin to "rcutree.fanout_leaf" and the prefix isn't documented.
However, there are several reasons why we might not want to simply fix the typo and add the prefix:
1) we'd end up with rcutree.rcu_nocb_poll, and rather probably make a change to rcutree.nocb_poll
2) if we did #1, then the prefix wouldn't be consistent with the rcu_nocbs=<cpumap> parameter (i.e. one with, one without prefix)
3) the use of module_param in a header file is less than desired, since it isn't immediately obvious that it will get processed via rcutree.c and get the prefix from that (although use of module_param_named() could clarify that.)
4) the implied export of /sys/module/rcutree/parameters/rcu_nocb_poll data to userspace via module_param() doesn't really buy us anything, as it is read-only and we can tell if it is enabled already without it, since there is a printk at early boot telling us so.
In light of all that, just change it from a module_param() to an early_setup() call, and worry about adding it to /sys later on if we decide to allow a dynamic setting of it.
Also change the variable to be tagged as read_mostly, since it will only ever be fiddled with at most, once at boot.
--- Documentation/kernel-parameters.txt | 2 +- kernel/rcutree_plugin.h | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-)
Documentation/kernel-parameters.txt +1 -1
@@ -2438,7 +2438,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. real-time workloads. It can also improve energy efficiency for asymmetric multiprocessors. - rcu_nocbs_poll [KNL,BOOT] + rcu_nocb_poll [KNL,BOOT] Rather than requiring that offloaded CPUs (specified by rcu_nocbs= above) explicitly awaken the corresponding "rcuoN" kthreads,
kernel/rcutree_plugin.h +8 -2
@@ -40,8 +40,7 @@#ifdef CONFIG_RCU_NOCB_CPU static cpumask_var_t rcu_nocb_mask;/* CPUs to have callbacks offloaded. */ static bool have_rcu_nocb_mask;/* Was rcu_nocb_mask allocated? */ -static bool rcu_nocb_poll;/* Offload kthread are to poll. */ -module_param (rcu_nocb_poll, bool,0444 ); +static bool __read_mostly rcu_nocb_poll;/* Offload kthread are to poll. */ static char __initdata nocb_buf[NR_CPUS *5 ];#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
@@ -2159,6 +2158,13 @@ static int __init rcu_nocb_setup(char *str) }__setup ("rcu_nocbs=" , rcu_nocb_setup); +static int __init parse_rcu_nocb_poll (char *arg) +{ + rcu_nocb_poll =1 ; +return 0 ; +} +early_param ("rcu_nocb_poll" , parse_rcu_nocb_poll); +/* Is the specified CPU a no-CPUs CPU? */ static bool is_nocb_cpu (int cpu) {
Signature · 1 line
1.7.8
Re: [PATCH tip/core/urgent 1/2] rcu: Prevent soft-lockup complaints about no-CBs CPUs
In reply to Paul E. McKenneyHi Paul,
2013/1/5 Paul E. McKenney <[email protected]>:
Quoted 19 lines
From: Paul Gortmaker <[email protected]>
The
wait_event()at the head of thercu_nocb_kthread()can result in soft-lockup complaints if the CPU in question does not register RCU callbacks for an extended period. This commit therefore changes thewait_event()to await_event_interruptible().Reported-by Frederic Weisbecker <[email protected]>Signed-off-by Paul Gortmaker <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree_plugin.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)kernel/rcutree_plugin.h +0
-0
kernel/rcutree_plugin.h static int rcu_nocb_kthread(void *arg) · line 2373
Quoted 9 lines
kernel/rcutree_plugin.h +2
-1@@ -2366,10 +2366,11 @@ static int rcu_nocb_kthread(void *arg)for (;;) {/* If not polling, wait for next batch of callbacks. */ if (!rcu_nocb_poll) -wait_event (rdp->nocb_wq, rdp->nocb_head); +wait_event_interruptible (rdp->nocb_wq, rdp->nocb_head); list =ACCESS_ONCE (rdp->nocb_head);if (!list) {schedule_timeout_interruptible (1 ); +flush_signals (current);
Why is that needed?
Re: [PATCH tip/core/urgent 1/2] rcu: Prevent soft-lockup complaints about no-CBs CPUs
Quoted 17 lines · Frederic Weisbecker
Hi Paul,
2013/1/5 Paul E. McKenney <[email protected]>:
Quoted 14 lines
From: Paul Gortmaker <[email protected]>
The
wait_event()at the head of thercu_nocb_kthread()can result in soft-lockup complaints if the CPU in question does not register RCU callbacks for an extended period. This commit therefore changes thewait_event()to await_event_interruptible().Reported-by Frederic Weisbecker <[email protected]>Signed-off-by Paul Gortmaker <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree_plugin.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
Quoted 16 lines
Quoted 5 lines
kernel/rcutree_plugin.h +0
-0kernel/rcutree_plugin.h static int rcu_nocb_kthread(void *arg) · line 2373
Quoted 9 lines
kernel/rcutree_plugin.h +2
-1@@ -2366,10 +2366,11 @@ static int rcu_nocb_kthread(void *arg)for (;;) {/* If not polling, wait for next batch of callbacks. */ if (!rcu_nocb_poll) -wait_event (rdp->nocb_wq, rdp->nocb_head); +wait_event_interruptible (rdp->nocb_wq, rdp->nocb_head); list =ACCESS_ONCE (rdp->nocb_head);if (!list) {schedule_timeout_interruptible (1 ); +flush_signals (current);Why is that needed?
To satisfy my paranoia. ;-) And in case someone ever figures out some way to send a signal to a kthread.
Thanx, Paul
Re: [PATCH tip/core/urgent 1/2] rcu: Prevent soft-lockup complaints about no-CBs CPUs
2013/1/5 Paul E. McKenney <[email protected]>:
Quoted 35 lines · Frederic Weisbecker
Quoted 17 lines
Hi Paul,
2013/1/5 Paul E. McKenney <[email protected]>:
Quoted 14 lines
From: Paul Gortmaker <[email protected]>
The
wait_event()at the head of thercu_nocb_kthread()can result in soft-lockup complaints if the CPU in question does not register RCU callbacks for an extended period. This commit therefore changes thewait_event()to await_event_interruptible().Reported-by Frederic Weisbecker <[email protected]>Signed-off-by Paul Gortmaker <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>--- kernel/rcutree_plugin.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)Quoted 16 lines
Quoted 5 lines
kernel/rcutree_plugin.h +0
-0kernel/rcutree_plugin.h static int rcu_nocb_kthread(void *arg) · line 2373
Quoted 9 lines
kernel/rcutree_plugin.h +2
-1@@ -2366,10 +2366,11 @@ static int rcu_nocb_kthread(void *arg)for (;;) {/* If not polling, wait for next batch of callbacks. */ if (!rcu_nocb_poll) -wait_event (rdp->nocb_wq, rdp->nocb_head); +wait_event_interruptible (rdp->nocb_wq, rdp->nocb_head); list =ACCESS_ONCE (rdp->nocb_head);if (!list) {schedule_timeout_interruptible (1 ); +flush_signals (current);Why is that needed?
Quoted 2 lines
To satisfy my paranoia. ;-) And in case someone ever figures out some way to send a signal to a kthread.
Ok. I don't want to cause any insomnia to anyone, so I won't insist ;)