From 5a41344a3d83ef2c08e40bfce1efa5795def9b82 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Thu, 29 Nov 2012 16:46:02 +0800 Subject: srcu: Simplify __srcu_read_unlock() via this_cpu_dec() This commit replaces disabling of preemption and decrement of a per-CPU variable with this_cpu_dec(), which avoids preemption disabling on x86 and shortens the code on all platforms. Signed-off-by: Lai Jiangshan Signed-off-by: Paul E. McKenney --- kernel/srcu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'kernel/srcu.c') diff --git a/kernel/srcu.c b/kernel/srcu.c index 2b859828cdc3..c9d003bc6c49 100644 --- a/kernel/srcu.c +++ b/kernel/srcu.c @@ -321,10 +321,8 @@ EXPORT_SYMBOL_GPL(__srcu_read_lock); */ void __srcu_read_unlock(struct srcu_struct *sp, int idx) { - preempt_disable(); smp_mb(); /* C */ /* Avoid leaking the critical section. */ - ACCESS_ONCE(this_cpu_ptr(sp->per_cpu_ref)->c[idx]) -= 1; - preempt_enable(); + this_cpu_dec(sp->per_cpu_ref->c[idx]); } EXPORT_SYMBOL_GPL(__srcu_read_unlock); -- cgit v1.2.3 From 6e6f1b307e23201fb3e7aaf16322e80355d2a3d5 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Thu, 29 Nov 2012 16:46:03 +0800 Subject: srcu: Add might_sleep() annotation to synchronize_srcu() Although synchronize_srcu() can sleep, it will not sleep if the fast path succeeds, which means that illegal use of synchronize_rcu() might go unnoticed. This commit therefore adds might_sleep(), which unconditionally catches illegal use of synchronize_rcu() from atomic context. Signed-off-by: Lai Jiangshan Signed-off-by: Paul E. McKenney --- kernel/srcu.c | 1 + 1 file changed, 1 insertion(+) (limited to 'kernel/srcu.c') diff --git a/kernel/srcu.c b/kernel/srcu.c index c9d003bc6c49..3e43a214b4dc 100644 --- a/kernel/srcu.c +++ b/kernel/srcu.c @@ -421,6 +421,7 @@ static void __synchronize_srcu(struct srcu_struct *sp, int trycount) !lock_is_held(&rcu_sched_lock_map), "Illegal synchronize_srcu() in same-type SRCU (or RCU) read-side critical section"); + might_sleep(); init_completion(&rcu.completion); head->next = NULL; -- cgit v1.2.3 From ab4d2986e44c589aa1b647d7da5e21c2707babea Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Thu, 29 Nov 2012 16:46:04 +0800 Subject: srcu: Simple cleanup for cleanup_srcu_struct() Pack six lines of code into two lines. Signed-off-by: Lai Jiangshan Signed-off-by: Paul E. McKenney --- kernel/srcu.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'kernel/srcu.c') diff --git a/kernel/srcu.c b/kernel/srcu.c index 3e43a214b4dc..7cf5baba96f9 100644 --- a/kernel/srcu.c +++ b/kernel/srcu.c @@ -282,12 +282,8 @@ static int srcu_readers_active(struct srcu_struct *sp) */ void cleanup_srcu_struct(struct srcu_struct *sp) { - int sum; - - sum = srcu_readers_active(sp); - WARN_ON(sum); /* Leakage unless caller handles error. */ - if (sum != 0) - return; + if (WARN_ON(srcu_readers_active(sp))) + return; /* Leakage unless caller handles error. */ free_percpu(sp->per_cpu_ref); sp->per_cpu_ref = NULL; } -- cgit v1.2.3 From 34a64b6bb64b5cf193932e2b4394c5284732e008 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Thu, 29 Nov 2012 16:46:07 +0800 Subject: srcu: Update synchronize_srcu()'s comments The core of SRCU is changed, but synchronize_srcu()'s comments describe the old algorithm. This commit therefore updates them to match the new algorithm. Signed-off-by: Lai Jiangshan Signed-off-by: Paul E. McKenney --- kernel/srcu.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'kernel/srcu.c') diff --git a/kernel/srcu.c b/kernel/srcu.c index 7cf5baba96f9..f098f1768215 100644 --- a/kernel/srcu.c +++ b/kernel/srcu.c @@ -450,10 +450,12 @@ static void __synchronize_srcu(struct srcu_struct *sp, int trycount) * synchronize_srcu - wait for prior SRCU read-side critical-section completion * @sp: srcu_struct with which to synchronize. * - * Flip the completed counter, and wait for the old count to drain to zero. - * As with classic RCU, the updater must use some separate means of - * synchronizing concurrent updates. Can block; must be called from - * process context. + * Wait for the count to drain to zero of both indexes. To avoid the + * possible starvation of synchronize_srcu(), it waits for the count of + * the index=((->completed & 1) ^ 1) to drain to zero at first, + * and then flip the completed and wait for the count of the other index. + * + * Can block; must be called from process context. * * Note that it is illegal to call synchronize_srcu() from the corresponding * SRCU read-side critical section; doing so will result in deadlock. -- cgit v1.2.3 From 49271ca60645d64197b28c0835fed39f74b1a2d7 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Thu, 29 Nov 2012 16:46:08 +0800 Subject: srcu: Update synchronize_srcu_expedited()'s comments Because synchronize_srcu_expedited() no longer uses synchronize_rcu_sched_expedited(), synchronize_srcu_expedited() no longer indirectly acquires any CPU-hotplug-related locks. This commit therefore updates the comments accordingly. Signed-off-by: Lai Jiangshan Signed-off-by: Paul E. McKenney --- kernel/srcu.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'kernel/srcu.c') diff --git a/kernel/srcu.c b/kernel/srcu.c index f098f1768215..e34f2991ed41 100644 --- a/kernel/srcu.c +++ b/kernel/srcu.c @@ -477,12 +477,11 @@ EXPORT_SYMBOL_GPL(synchronize_srcu); * Wait for an SRCU grace period to elapse, but be more aggressive about * spinning rather than blocking when waiting. * - * Note that it is illegal to call this function while holding any lock - * that is acquired by a CPU-hotplug notifier. It is also illegal to call - * synchronize_srcu_expedited() from the corresponding SRCU read-side - * critical section; doing so will result in deadlock. However, it is - * perfectly legal to call synchronize_srcu_expedited() on one srcu_struct - * from some other srcu_struct's read-side critical section, as long as + * Note that it is also illegal to call synchronize_srcu_expedited() + * from the corresponding SRCU read-side critical section; + * doing so will result in deadlock. However, it is perfectly legal + * to call synchronize_srcu_expedited() on one srcu_struct from some + * other srcu_struct's read-side critical section, as long as * the resulting graph of srcu_structs is acyclic. */ void synchronize_srcu_expedited(struct srcu_struct *sp) -- cgit v1.2.3 From 7a6b55e7108b3476d13ee9501ec69dbe1605d774 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Thu, 29 Nov 2012 16:46:09 +0800 Subject: srcu: use ACCESS_ONCE() to access sp->completed in srcu_read_lock() The old SRCU implementation loads sp->completed within an RCU-sched section, courtesy of preempt_disable(). This was required due to the use of synchronize_sched() in the old implemenation's synchronize_srcu(). However, the new implementation does not rely on synchronize_sched(), so it in turn does not require the load of sp->completed and the ->c[] counter to be in a single preempt-disabled region of code. This commit therefore moves the sp->completed access outside of the preempt-disabled region and applies ACCESS_ONCE(). The resulting code is almost as the same as before, but it removes the now-misleading rcu_dereference_index_check() call. Signed-off-by: Lai Jiangshan Signed-off-by: Paul E. McKenney --- kernel/srcu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'kernel/srcu.c') diff --git a/kernel/srcu.c b/kernel/srcu.c index e34f2991ed41..01d5ccb8bfe3 100644 --- a/kernel/srcu.c +++ b/kernel/srcu.c @@ -298,9 +298,8 @@ int __srcu_read_lock(struct srcu_struct *sp) { int idx; + idx = ACCESS_ONCE(sp->completed) & 0x1; preempt_disable(); - idx = rcu_dereference_index_check(sp->completed, - rcu_read_lock_sched_held()) & 0x1; ACCESS_ONCE(this_cpu_ptr(sp->per_cpu_ref)->c[idx]) += 1; smp_mb(); /* B */ /* Avoid leaking the critical section. */ ACCESS_ONCE(this_cpu_ptr(sp->per_cpu_ref)->seq[idx]) += 1; -- cgit v1.2.3