v2 Fixes for 3.12
Subsystems (4)
cover v2 Fixes for 3.12
Hello!
This series contains a number of miscellaneous fixes:
1. Switch to expedited grace periods during hibernation and
suspend/resume operations (courtesy of Borislav Petkov and Bjørn Mork).
2. Make RCU debugobjects fixup actions leak callbacks rather than
risking corrupting the callback lists.
3. Propagate above callback leaking to debugobjects's callers.
4. Make call_rcu() leak callbacks when debugobjects indicates an error.
5. Avoid needless self-wakeups from the grace-period kthreads.
6. Make list_first_or_null_rcu() use list_entry_rcu() (courtesy of
Tejun Heo).
7. Fix irq_work_queue() build error for TREE_PREEMPT_RCU for some
configurations (courtesy of James Hogan).
8. Simplify _rcu_barrier() processing per Linus Torvalds feedback.
9. Avoid signed-overflow undefined behavior in time_after() and friends.
Changes since v1 (https://lkml.org/lkml/2013/8/17/106):
o Merge v1 patches 1, 2, and 6 into the new patch 1.
o Apply other review comments from Josh Triplett.
Thanx, Paul
b/include/linux/debugobjects.h | 6 +- b/include/linux/jiffies.h | 8 +-- b/include/linux/rculist.h | 5 +- b/init/Kconfig | 1 b/kernel/rcu.h | 10 ++-- b/kernel/rcupdate.c | 100 ----------------------------------------- b/kernel/rcutree.c | 58 +++++++++++++++++++++-- b/lib/debugobjects.c | 20 +++++--- 8 files changed, 84 insertions(+), 124 deletions(-)
PATCH 1/9 rcu: Expedite grace periods during suspend/resume
From: Borislav Petkov <[email protected]>
CONFIG_RCU_FAST_NO_HZ can increase grace-period durations by up to a factor of four, which can result in long suspend and resume times. Thus, this commit temporarily switches to expedited grace periods when suspending the box and return to normal settings when resuming. Similar logic is applied to hibernation.
Because expedited grace periods are of dubious benefit on very large systems, so this commit restricts their automated use during suspend and resume to systems of 256 or fewer CPUs. (Some day a number of Linux-kernel facilities, including RCU's expedited grace periods, will be more scalable, but I need to see bug reports first.)
[ paulmck: This also papers over an audio/irq bug, but hopefully that will
be fixed soon. ]
--- kernel/rcutree.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
kernel/rcutree.c +21 -0
@@ -54,6 +54,7 @@#include <linux/stop_machine.h> #include <linux/random.h> #include <linux/ftrace_event.h> +#include <linux/suspend.h> #include "rcutree.h" #include <trace/events/rcu.h>
@@ -3032,6 +3033,25 @@ static int rcu_cpu_notify(struct notifier_block *self,return NOTIFY_OK ; } +static int rcu_pm_notify (struct notifier_block *self, +unsigned long action,void *hcpu) +{ +switch (action) { +case PM_HIBERNATION_PREPARE : +case PM_SUSPEND_PREPARE : +if (nr_cpu_ids <=256 )/* Expediting bad for large systems. */ + rcu_expedited =1 ; +break ; +case PM_POST_HIBERNATION : +case PM_POST_SUSPEND : + rcu_expedited =0 ; +break ; +default : +break ; + } +return NOTIFY_OK ; +} +/* * Spawn the kthread that handles this RCU flavor's grace periods. */
@@ -3273,6 +3293,7 @@ void __init rcu_init(void)* or the scheduler are operational. */ cpu_notifier (rcu_cpu_notify,0 ); +pm_notifier (rcu_pm_notify,0 );for_each_online_cpu (cpu)rcu_cpu_notify (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); }
Signature · 1 line
1.8.1.5
PATCH 2/9 rcu: Simplify debug-objects fixups
From: "Paul E. McKenney" <[email protected]>
The current debug-objects fixups are complex and heavyweight, and the fixups are not complete: Even with the fixups, RCU's callback lists can still be corrupted. This commit therefore strips the fixups down to their minimal form, eliminating two of the three.
It would be even better if (for example) call_rcu() simply leaked any problematic callbacks, but for that to happen, the debug-objects system would need to inform its caller of suspicious situations. This is the subject of a later commit in this series.
--- kernel/rcupdate.c | 100 ------------------------------------------------------ 1 file changed, 100 deletions(-)
kernel/rcupdate.c +0 -100
@@ -212,43 +212,6 @@ static inline void debug_rcu_head_free(struct rcu_head *head) }/* -* fixup_init is called when: -* - an active object is initialized -*/ -static int rcuhead_fixup_init (void *addr,enum debug_obj_state state) -{ -struct rcu_head *head = addr; - -switch (state) { -case ODEBUG_STATE_ACTIVE : -/* -* Ensure that queued callbacks are all executed. -* If we detect that we are nested in a RCU read-side critical -* section, we should simply fail, otherwise we would deadlock. -* In !PREEMPT configurations, there is no way to tell if we are -* in a RCU read-side critical section or not, so we never -* attempt any fixup and just print a warning. -*/ -#ifndef CONFIG_PREEMPT -WARN_ON_ONCE (1 ); -return 0 ; -#endif -if (rcu_preempt_depth () !=0 ||preempt_count () !=0 || -irqs_disabled ()) { -WARN_ON_ONCE (1 ); -return 0 ; - } -rcu_barrier (); -rcu_barrier_sched (); -rcu_barrier_bh (); -debug_object_init (head, &rcuhead_debug_descr); -return 1 ; -default : -return 0 ; - } -} - -/* * fixup_activate is called when: * - an active object is activated * - an unknown object is activated (might be a statically initialized object)
@@ -268,69 +231,8 @@ static int rcuhead_fixup_activate(void *addr, enum debug_obj_state state)debug_object_init (head, &rcuhead_debug_descr);debug_object_activate (head, &rcuhead_debug_descr);return 0 ; - -case ODEBUG_STATE_ACTIVE : -/* -* Ensure that queued callbacks are all executed. -* If we detect that we are nested in a RCU read-side critical -* section, we should simply fail, otherwise we would deadlock. -* In !PREEMPT configurations, there is no way to tell if we are -* in a RCU read-side critical section or not, so we never -* attempt any fixup and just print a warning. -*/ -#ifndef CONFIG_PREEMPT -WARN_ON_ONCE (1 ); -return 0 ; -#endif -if (rcu_preempt_depth () !=0 ||preempt_count () !=0 || -irqs_disabled ()) { -WARN_ON_ONCE (1 ); -return 0 ; - } -rcu_barrier (); -rcu_barrier_sched (); -rcu_barrier_bh (); -debug_object_activate (head, &rcuhead_debug_descr); -return 1 ;default : -return 0 ; - } -} - -/* -* fixup_free is called when: -* - an active object is freed -*/ -static int rcuhead_fixup_free (void *addr,enum debug_obj_state state) -{ -struct rcu_head *head = addr; - -switch (state) { -case ODEBUG_STATE_ACTIVE : -/* -* Ensure that queued callbacks are all executed. -* If we detect that we are nested in a RCU read-side critical -* section, we should simply fail, otherwise we would deadlock. -* In !PREEMPT configurations, there is no way to tell if we are -* in a RCU read-side critical section or not, so we never -* attempt any fixup and just print a warning. -*/ -#ifndef CONFIG_PREEMPT -WARN_ON_ONCE (1 ); -return 0 ; -#endif -if (rcu_preempt_depth () !=0 ||preempt_count () !=0 || -irqs_disabled ()) { -WARN_ON_ONCE (1 ); -return 0 ; - } -rcu_barrier (); -rcu_barrier_sched (); -rcu_barrier_bh (); -debug_object_free (head, &rcuhead_debug_descr);return 1 ; -default : -return 0 ; } }
@@ -369,9 +271,7 @@ EXPORT_SYMBOL_GPL(destroy_rcu_head_on_stack);struct debug_obj_descr rcuhead_debug_descr = { .name ="rcu_head" , - .fixup_init = rcuhead_fixup_init, .fixup_activate = rcuhead_fixup_activate, - .fixup_free = rcuhead_fixup_free, };EXPORT_SYMBOL_GPL (rcuhead_debug_descr);#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
Signature · 1 line
1.8.1.5
PATCH 3/9 debugobjects: Make debug_object_activate() return status
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
In order to better respond to things like duplicate invocations of call_rcu(), RCU needs to see the status of a call to debug_object_activate(). This would allow RCU to leak the callback in order to avoid adding freelist-reuse mischief to the duplicate invoations. This commit therefore makes debug_object_activate() return status, zero for success and -EINVAL for failure.
--- include/linux/debugobjects.h | 6 +++--- lib/debugobjects.c | 20 ++++++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-)
include/linux/debugobjects.h +3 -3
@@ -63,7 +63,7 @@ struct debug_obj_descr {extern void debug_object_init (void *addr,struct debug_obj_descr *descr);extern void debug_object_init_on_stack (void *addr,struct debug_obj_descr *descr); -extern void debug_object_activate (void *addr,struct debug_obj_descr *descr); +extern int debug_object_activate (void *addr,struct debug_obj_descr *descr);extern void debug_object_deactivate (void *addr,struct debug_obj_descr *descr);extern void debug_object_destroy (void *addr,struct debug_obj_descr *descr);extern void debug_object_free (void *addr,struct debug_obj_descr *descr);
@@ -85,8 +85,8 @@ static inline void debug_object_init (void *addr, struct debug_obj_descr *descr) { }static inline void debug_object_init_on_stack (void *addr,struct debug_obj_descr *descr) { } -static inline void -debug_object_activate (void *addr,struct debug_obj_descr *descr) { } +static inline int +debug_object_activate (void *addr,struct debug_obj_descr *descr) {return 0 ; }static inline void debug_object_deactivate (void *addr,struct debug_obj_descr *descr) { } static inline void
lib/debugobjects.c +14 -6
@@ -381,19 +381,21 @@ void debug_object_init_on_stack(void *addr, struct debug_obj_descr *descr)* debug_object_activate - debug checks when an object is activated * @addr: address of the object * @descr: pointer to an object specific debug description structure +* Returns 0 for success, -EINVAL for check failed. */ -void debug_object_activate (void *addr,struct debug_obj_descr *descr) +int debug_object_activate (void *addr,struct debug_obj_descr *descr) {enum debug_obj_state state;struct debug_bucket *db;struct debug_obj *obj;unsigned long flags; +int ret;struct debug_obj o = { .object = addr, .state =ODEBUG_STATE_NOTAVAILABLE , .descr = descr };if (!debug_objects_enabled) -return ; +return 0 ; db =get_bucket ((unsigned long ) addr);
@@ -405,23 +407,26 @@ void debug_object_activate(void *addr, struct debug_obj_descr *descr)case ODEBUG_STATE_INIT :case ODEBUG_STATE_INACTIVE : obj->state =ODEBUG_STATE_ACTIVE ; + ret =0 ;break ;case ODEBUG_STATE_ACTIVE :debug_print_object (obj,"activate" ); state = obj->state;raw_spin_unlock_irqrestore (&db->lock, flags); -debug_object_fixup (descr->fixup_activate, addr, state); -return ; + ret =debug_object_fixup (descr->fixup_activate, addr, state); +return ret ? -EINVAL :0 ;case ODEBUG_STATE_DESTROYED :debug_print_object (obj,"activate" ); + ret = -EINVAL ;break ;default : + ret =0 ;break ; }raw_spin_unlock_irqrestore (&db->lock, flags); -return ; +return ret; }raw_spin_unlock_irqrestore (&db->lock, flags);
@@ -431,8 +436,11 @@ void debug_object_activate(void *addr, struct debug_obj_descr *descr)* true or not. */ if (debug_object_fixup (descr->fixup_activate, addr, -ODEBUG_STATE_NOTAVAILABLE )) +ODEBUG_STATE_NOTAVAILABLE )) {debug_print_object (&o,"activate" ); +return -EINVAL ; + } +return 0 ; }/**
Signature · 1 line
1.8.1.5
PATCH 4/9 rcu: Make call_rcu() leak callbacks for debug-object errors
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
If someone does a duplicate call_rcu(), the worst thing the second call_rcu() could do would be to actually queue the callback the second time because doing so corrupts whatever list the callback was already queued on. This commit therefore makes __call_rcu() check the new return value from debug-objects and leak the callback upon error. This commit also substitutes rcu_leak_callback() for whatever callback function was previously in place in order to avoid freeing the callback out from under any readers that might still be referencing it.
These changes increase the probability that the debug-objects error messages will actually make it somewhere visible.
--- kernel/rcu.h | 10 +++++++--- kernel/rcutree.c | 14 +++++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-)
kernel/rcu.h +7 -3
@@ -67,12 +67,15 @@extern struct debug_obj_descr rcuhead_debug_descr; -static inline void debug_rcu_head_queue (struct rcu_head *head) +static inline int debug_rcu_head_queue (struct rcu_head *head) { -debug_object_activate (head, &rcuhead_debug_descr); +int r1; + + r1 =debug_object_activate (head, &rcuhead_debug_descr);debug_object_active_state (head, &rcuhead_debug_descr,STATE_RCU_HEAD_READY ,STATE_RCU_HEAD_QUEUED ); +return r1; }static inline void debug_rcu_head_unqueue (struct rcu_head *head)
@@ -83,8 +86,9 @@ static inline void debug_rcu_head_unqueue(struct rcu_head *head) debug_object_deactivate(head, &rcuhead_debug_descr); }#else /* !CONFIG_DEBUG_OBJECTS_RCU_HEAD */ -static inline void debug_rcu_head_queue (struct rcu_head *head) +static inline int debug_rcu_head_queue (struct rcu_head *head) { +return 0 ; }static inline void debug_rcu_head_unqueue (struct rcu_head *head)
kernel/rcutree.c +13 -1
@@ -2305,6 +2305,13 @@ static void __call_rcu_core(struct rcu_state *rsp, struct rcu_data *rdp, }/* +* RCU callback function to leak a callback. +*/ +static void rcu_leak_callback (struct rcu_head *rhp) +{ +} + +/* * Helper function for call_rcu() and friends. The cpu argument will * normally be -1, indicating "currently running CPU". It may specify * a CPU only if that CPU is a no-CBs CPU. Currently, only _rcu_barrier()
@@ -2318,7 +2325,12 @@ __call_rcu(struct rcu_head *head, void (*func)(struct rcu_head *rcu),struct rcu_data *rdp;WARN_ON_ONCE ((unsigned long )head &0x3 );/* Misaligned rcu_head! */ -debug_rcu_head_queue (head); +if (debug_rcu_head_queue (head)) { +/* Probable double call_rcu(), so leak the callback. */ +ACCESS_ONCE (head->func) = rcu_leak_callback; +WARN_ONCE (1 ,"__call_rcu(): Leaked duplicate callback\n" ); +return ; + } head->func = func; head->next =NULL ;
Signature · 1 line
1.8.1.5
PATCH 5/9 rcu: Avoid redundant grace-period kthread wakeups
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
When setting up an in-the-future "advanced" grace period, the code needs to wake up the relevant grace-period kthread, which it currently does unconditionally. However, this results in needless wakeups in the case where the advanced grace period is being set up by the grace-period kthread itself, which is a non-uncommon situation. This commit therefore checks to see if the running thread is the grace-period kthread, and avoids doing the irq_work_queue()-mediated wakeup in that case.
--- kernel/rcutree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
kernel/rcutree.c +5 -3
@@ -1576,10 +1576,12 @@ rcu_start_gp_advanced(struct rcu_state *rsp, struct rcu_node *rnp,/* * We can't do wakeups while holding the rnp->lock, as that -* could cause possible deadlocks with the rq->lock. Deter -* the wakeup to interrupt context. +* could cause possible deadlocks with the rq->lock. Defer +* the wakeup to interrupt context. And don't bother waking +* up the running kthread. */ -irq_work_queue (&rsp->wakeup_work); +if (current != rsp->gp_kthread) +irq_work_queue (&rsp->wakeup_work); }/*
Signature · 1 line
1.8.1.5
PATCH 6/9 rculist: list_first_or_null_rcu() should use list_entry_rcu()
In reply to Paul E. McKenneyFrom: Tejun Heo <[email protected]>
list_first_or_null() should test whether the list is empty and return pointer to the first entry if not in a RCU safe manner. It's broken in several ways.
* It compares __kernel @__ptr with __rcu @__next triggering the following sparse warning.
net/core/dev.c:4331:17: error: incompatible types in comparison expression (different address spaces)
* It doesn't perform rcu_dereference*() and computes the entry address using container_of() directly from the __rcu pointer which is inconsitent with other rculist interface. As a result, all three in-kernel users - net/core/dev.c, macvlan, cgroup - are buggy. They dereference the pointer w/o going through read barrier.
* While ->next dereference passes through list_next_rcu(), the compiler is still free to fetch ->next more than once and thus nullify the "__ptr != __next" condition check.
Fix it by making list_first_or_null_rcu() dereference ->next directly using ACCESS_ONCE() and then use list_entry_rcu() on it like other rculist accessors.
v2: Paul pointed out that the compiler may fetch the pointer more than
once nullifying the condition check. ACCESS_ONCE() added on ->next dereference.
v3: Restored () around macro param which was accidentally removed.
Spotted by Paul.
--- include/linux/rculist.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
include/linux/rculist.h +3 -2
@@ -267,8 +267,9 @@ static inline void list_splice_init_rcu(struct list_head *list,*/ #define list_first_or_null_rcu (ptr, type, member) \ ({struct list_head *__ptr = (ptr); \ - struct list_head __rcu *__next = list_next_rcu(__ptr); \ - likely(__ptr != __next) ? container_of(__next, type, member) : NULL; \ + struct list_head *__next = ACCESS_ONCE(__ptr->next); \ + likely(__ptr != __next) ? \ + list_entry_rcu(__next, type, member) : NULL; \ })/**
Signature · 1 line
1.8.1.5
PATCH 7/9 rcu: Select IRQ_WORK from TREE_PREEMPT_RCU
In reply to Paul E. McKenneyFrom: James Hogan <[email protected]>
TREE_RCU and TREE_PREEMPT_RCU both cause kernel/rcutree.c to be built, but only TREE_RCU selects IRQ_WORK, which can result in an undefined reference to irq_work_queue for some (random) configs:
kernel/built-in.o In function `rcu_start_gp_advanced':kernel/rcutree.c:1564: undefined reference to `irq_work_queue'
Select IRQ_WORK from TREE_PREEMPT_RCU too to fix this.
--- init/Kconfig | 1 + 1 file changed, 1 insertion(+)
init/Kconfig +1 -0
@@ -470,6 +470,7 @@ config TREE_RCU config TREE_PREEMPT_RCU bool "Preemptible tree-based hierarchical RCU" depends on PREEMPT + select IRQ_WORK help This option selects the RCU implementation that is designed for very large SMP systems with hundreds or
Signature · 1 line
1.8.1.5
PATCH 8/9 rcu: Simplify _rcu_barrier() processing
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
This commit drops an unneeded ACCESS_ONCE() and simplifies an "our work is done" check in _rcu_barrier(). This applies feedback from Linus
(https://lkml.org/lkml/2013/7/26/777) that he gave to similar code in an unrelated patch.
--- kernel/rcutree.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
kernel/rcutree.c +13 -2
@@ -2817,9 +2817,20 @@ static void _rcu_barrier(struct rcu_state *rsp)* transition. The "if" expression below therefore rounds the old * value up to the next even number and adds two before comparing. */ - snap_done =ACCESS_ONCE (rsp->n_barrier_done); + snap_done = rsp->n_barrier_done;_rcu_barrier_trace (rsp,"Check" ,-1 , snap_done); -if (ULONG_CMP_GE (snap_done, ((snap +1 ) & ~0x1 ) +2 )) { + +/* +* If the value in snap is odd, we needed to wait for the current +* rcu_barrier() to complete, then wait for the next one, in other +* words, we need the value of snap_done to be three larger than +* the value of snap. On the other hand, if the value in snap is +* even, we only had to wait for the next rcu_barrier() to complete, +* in other words, we need the value of snap_done to be only two +* greater than the value of snap. The "(snap + 3) & 0x1" computes +* this for us (thank you, Linus!). +*/ +if (ULONG_CMP_GE (snap_done, (snap +3 ) & ~0x1 )) {_rcu_barrier_trace (rsp,"EarlyExit" ,-1 , snap_done);smp_mb ();/* caller's subsequent code after above check. */ mutex_unlock (&rsp->barrier_mutex);
Signature · 1 line
1.8.1.5
Re: [PATCH tip/core/rcu 8/9] rcu: Simplify _rcu_barrier() processing
Quoted 24 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
This commit drops an unneeded
ACCESS_ONCE()and simplifies an "our work is done" check in_rcu_barrier(). This applies feedback from Linus
(https://lkml.org/lkml/2013/7/26/777) that he gave to similar code in an unrelated patch.Signed-off-by Paul E. McKenney <[email protected]>Reviewed-by Josh Triplett <[email protected]>--- kernel/rcutree.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)kernel/rcutree.c +1
-1@@ -2817,9 +2817,20 @@ static void _rcu_barrier(struct rcu_state *rsp)* transition. The "if" expression below therefore rounds the old * value up to the next even number and adds two before comparing. */ - snap_done =ACCESS_ONCE (rsp->n_barrier_done); + snap_done = rsp->n_barrier_done;_rcu_barrier_trace (rsp,"Check" ,-1 , snap_done);
kernel/rcutree.c static void _rcu_barrier(struct rcu_state *rsp) · line 2830
Quoted 10 lines
kernel/rcutree.c +9
-1-if (ULONG_CMP_GE (snap_done, ((snap +1 ) & ~0x1 ) +2 )) { + +/* +* If the value in snap is odd, we needed to wait for the current +* rcu_barrier() to complete, then wait for the next one, in other +* words, we need the value of snap_done to be three larger than +* the value of snap. On the other hand, if the value in snap is +* even, we only had to wait for the next rcu_barrier() to complete, +* in other words, we need the value of snap_done to be only two +* greater than the value of snap. The "(snap + 3) & 0x1" computes
"(snap + 3) & 0x1"
==> "(snap + 3) & ~0x1"
Quoted 6 lines
kernel/rcutree.c +3
-0+* this for us (thank you, Linus!). +*/ +if (ULONG_CMP_GE (snap_done, (snap +3 ) & ~0x1 )) {_rcu_barrier_trace (rsp,"EarlyExit" ,-1 , snap_done);smp_mb ();/* caller's subsequent code after above check. */ mutex_unlock (&rsp->barrier_mutex);
Re: [PATCH tip/core/rcu 8/9] rcu: Simplify _rcu_barrier() processing
Quoted 30 lines · Lai Jiangshan
Quoted 29 lines · Paul E. McKenney
From: "Paul E. McKenney" <[email protected]>
This commit drops an unneeded
ACCESS_ONCE()and simplifies an "our work is done" check in_rcu_barrier(). This applies feedback from Linus
(https://lkml.org/lkml/2013/7/26/777) that he gave to similar code in an unrelated patch.Signed-off-by Paul E. McKenney <[email protected]>Reviewed-by Josh Triplett <[email protected]>--- kernel/rcutree.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)kernel/rcutree.c +5
-2@@ -2817,9 +2817,20 @@ static void _rcu_barrier(struct rcu_state *rsp)* transition. The "if" expression below therefore rounds the old * value up to the next even number and adds two before comparing. */ - snap_done =ACCESS_ONCE (rsp->n_barrier_done); + snap_done = rsp->n_barrier_done;_rcu_barrier_trace (rsp,"Check" ,-1 , snap_done); -if (ULONG_CMP_GE (snap_done, ((snap +1 ) & ~0x1 ) +2 )) { + +/* +* If the value in snap is odd, we needed to wait for the current +* rcu_barrier() to complete, then wait for the next one, in other
Quoted 8 lines
kernel/rcutree.c static void _rcu_barrier(struct rcu_state *rsp) · line 2830
Quoted 5 lines
kernel/rcutree.c +5
-0+* words, we need the value of snap_done to be three larger than +* the value of snap. On the other hand, if the value in snap is +* even, we only had to wait for the next rcu_barrier() to complete, +* in other words, we need the value of snap_done to be only two +* greater than the value of snap. The "(snap + 3) & 0x1" computes "(snap + 3) &
0x1"
==> "(snap + 3) & ~0x1"
Good catch, fixed!
Thanx, Paul
Quoted 7 lines
Quoted 6 lines
kernel/rcutree.c +3
-0+* this for us (thank you, Linus!). +*/ +if (ULONG_CMP_GE (snap_done, (snap +3 ) & ~0x1 )) {_rcu_barrier_trace (rsp,"EarlyExit" ,-1 , snap_done);smp_mb ();/* caller's subsequent code after above check. */ mutex_unlock (&rsp->barrier_mutex);
PATCH 9/9 jiffies: Avoid undefined behavior from signed overflow
In reply to Paul E. McKenneyFrom: "Paul E. McKenney" <[email protected]>
According to the C standard 3.4.3p3, overflow of a signed integer results in undefined behavior. This commit therefore changes the definitions of time_after(), time_after_eq(), time_after64(), and time_after_eq64() to avoid this undefined behavior. The trick is that the subtraction is done using unsigned arithmetic, which according to 6.2.5p9 cannot overflow because it is defined as modulo arithmetic. This has the added (though admittedly quite small) benefit of shortening four lines of code by four characters each.
Note that the C standard considers the cast from unsigned to signed to be implementation-defined, see 6.3.1.3p3. However, on a two's-complement system, an implementation that defines anything other than a reinterpretation of the bits is free to come to me, and I will be happy to act as a witness for its being committed to an insane asylum. (Although I have nothing against saturating arithmetic or signals in some cases, these things really should not be the default when compiling an operating-system kernel.)
[ paulmck: Included time_after64() and time_after_eq64(), as suggested
by Eric Dumazet, also fixed commit message.]
--- include/linux/jiffies.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
include/linux/jiffies.h +4 -4
@@ -101,13 +101,13 @@ static inline u64 get_jiffies_64(void)#define time_after (a,b) \ (typecheck(unsigned long, a) && \ typecheck(unsigned long, b) && \ - ((long)(b) - (long)(a) < 0)) + ((long)((b) - (a)) < 0))#define time_before (a,b) time_after(b,a)#define time_after_eq (a,b) \ (typecheck(unsigned long, a) && \ typecheck(unsigned long, b) && \ - ((long)(a) - (long)(b) >= 0)) + ((long)((a) - (b)) >= 0))#define time_before_eq (a,b) time_after_eq(b,a)/*
@@ -130,13 +130,13 @@ static inline u64 get_jiffies_64(void)#define time_after64 (a,b) \ (typecheck(__u64, a) && \ typecheck(__u64, b) && \ - ((__s64)(b) - (__s64)(a) < 0)) + ((__s64)((b) - (a)) < 0))#define time_before64 (a,b) time_after64(b,a)#define time_after_eq64 (a,b) \ (typecheck(__u64, a) && \ typecheck(__u64, b) && \ - ((__s64)(a) - (__s64)(b) >= 0)) + ((__s64)((a) - (b)) >= 0))#define time_before_eq64 (a,b) time_after_eq64(b,a)#define time_in_range64 (a, b, c) \
Signature · 1 line
1.8.1.5
Re: [PATCH tip/core/rcu 1/9] rcu: Expedite grace periods during suspend/resume
In reply to Paul E. McKenneyQuoted 43 lines · Paul E. McKenney
From: Borislav Petkov <[email protected]>
CONFIG_RCU_FAST_NO_HZcan increase grace-period durations by up to a factor of four, which can result in long suspend and resume times. Thus, this commit temporarily switches to expedited grace periods when suspending the box and return to normal settings when resuming. Similar logic is applied to hibernation.Because expedited grace periods are of dubious benefit on very large systems, so this commit restricts their automated use during suspend and resume to systems of 256 or fewer CPUs. (Some day a number of Linux-kernel facilities, including RCU's expedited grace periods, will be more scalable, but I need to see bug reports first.)
[ paulmck: This also papers over an audio/irq bug, but hopefully that will
be fixed soon. ]Signed-off-by Borislav Petkov <[email protected]>Signed-off-by Bjørn Mork <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>Reviewed-by Josh Triplett <[email protected]>--- kernel/rcutree.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)kernel/rcutree.c +3
-0@@ -54,6 +54,7 @@#include <linux/stop_machine.h> #include <linux/random.h> #include <linux/ftrace_event.h> +#include <linux/suspend.h> #include "rcutree.h" #include <trace/events/rcu.h> @@ -3032,6 +3033,25 @@ static int rcu_cpu_notify(struct notifier_block *self,return NOTIFY_OK ; } +static int rcu_pm_notify (struct notifier_block *self, +unsigned long action,void *hcpu)
kernel/rcutree.c static int rcu_cpu_notify(struct notifier_block *self, · line 3047
Quoted 10 lines
kernel/rcutree.c +10
-0+{ +switch (action) { +case PM_HIBERNATION_PREPARE : +case PM_SUSPEND_PREPARE : +if (nr_cpu_ids <=256 )/* Expediting bad for large systems. */ + rcu_expedited =1 ; +break ; +case PM_POST_HIBERNATION : +case PM_POST_SUSPEND : + rcu_expedited =0 ;
Users can set it via sysfs, this notify will changes it. I think we can introduce an rcu_expedited_syfs_saved; thus we can change this line to:
- rcu_expedited = 0;
+ rcu_expedited = rcu_expedited_syfs_saved;
rcu_init() {
...+ rcu_expedited_syfs_saved = rcu_expedited;
}
static ssize_t rcu_expedited_store(struct kobject *kobj,
struct kobj_attribute *attr, const char *buf, size_t count)
{
if (kstrtoint(buf, 0, &rcu_expedited))
return -EINVAL;+ rcu_expedited_syfs_saved = rcu_expedited; return count; }
Quoted 18 lines
kernel/rcutree.c +8
-0+break ; +default : +break ; + } +return NOTIFY_OK ; +} +/* * Spawn the kthread that handles this RCU flavor's grace periods. */ @@ -3273,6 +3293,7 @@ void __init rcu_init(void)* or the scheduler are operational. */ cpu_notifier (rcu_cpu_notify,0 ); +pm_notifier (rcu_pm_notify,0 );for_each_online_cpu (cpu)rcu_cpu_notify (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); }
Re: [PATCH tip/core/rcu 1/9] rcu: Expedite grace periods during suspend/resume
Quoted 58 lines · Lai Jiangshan
Quoted 43 lines · Paul E. McKenney
From: Borislav Petkov <[email protected]>
CONFIG_RCU_FAST_NO_HZcan increase grace-period durations by up to a factor of four, which can result in long suspend and resume times. Thus, this commit temporarily switches to expedited grace periods when suspending the box and return to normal settings when resuming. Similar logic is applied to hibernation.Because expedited grace periods are of dubious benefit on very large systems, so this commit restricts their automated use during suspend and resume to systems of 256 or fewer CPUs. (Some day a number of Linux-kernel facilities, including RCU's expedited grace periods, will be more scalable, but I need to see bug reports first.)
[ paulmck: This also papers over an audio/irq bug, but hopefully that will
be fixed soon. ]Signed-off-by Borislav Petkov <[email protected]>Signed-off-by Bjørn Mork <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>Reviewed-by Josh Triplett <[email protected]>--- kernel/rcutree.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)kernel/rcutree.c +3
-0@@ -54,6 +54,7 @@#include <linux/stop_machine.h> #include <linux/random.h> #include <linux/ftrace_event.h> +#include <linux/suspend.h> #include "rcutree.h" #include <trace/events/rcu.h> @@ -3032,6 +3033,25 @@ static int rcu_cpu_notify(struct notifier_block *self,return NOTIFY_OK ; } +static int rcu_pm_notify (struct notifier_block *self, +unsigned long action,void *hcpu)kernel/rcutree.c static int rcu_cpu_notify(struct notifier_block *self, · line 3047
Quoted 10 lines
kernel/rcutree.c +10
-0+{ +switch (action) { +case PM_HIBERNATION_PREPARE : +case PM_SUSPEND_PREPARE : +if (nr_cpu_ids <=256 )/* Expediting bad for large systems. */ + rcu_expedited =1 ; +break ; +case PM_POST_HIBERNATION : +case PM_POST_SUSPEND : + rcu_expedited =0 ;Users can set it via sysfs, this notify will changes it. I think we can introduce an
rcu_expedited_syfs_saved; thus we can change this line to:
Review
Quoted 2 lines
(quoted diff) +1
-1- rcu_expedited = 0; + rcu_expedited = rcu_expedited_syfs_saved;
We could do this, but there are still races where user tasks update sysfs while the operation is in progress. There are other races as well, particularly if multiple user tasks are concurrently attempting to do this sysfs update. The final solution likely involves a bunch of stuff, possibly including a driver to gain release-on-exit semantics.
Until someone actually tries using this, we won't really know what we actually need. And it is always possible that no one will actually use it. So we need to hold off until we see some real-world use cases.
Thanx, Paul
Quoted 36 lines
rcu_init() { ...(quoted diff) +1
-0+ rcu_expedited_syfs_saved = rcu_expedited;}
static
ssize_trcu_expedited_store(struct kobject *kobj,
structkobj_attribute*attr, const char *buf,size_tcount){ if (kstrtoint(buf, 0, &rcu_expedited)) return -EINVAL;(quoted diff) +1
-0+ rcu_expedited_syfs_saved = rcu_expedited;return count; }Quoted 18 lines
kernel/rcutree.c +8
-0+break ; +default : +break ; + } +return NOTIFY_OK ; +} +/* * Spawn the kthread that handles this RCU flavor's grace periods. */ @@ -3273,6 +3293,7 @@ void __init rcu_init(void)* or the scheduler are operational. */ cpu_notifier (rcu_cpu_notify,0 ); +pm_notifier (rcu_pm_notify,0 );for_each_online_cpu (cpu)rcu_cpu_notify (NULL ,CPU_UP_PREPARE , (void *)(long )cpu); }