rcu: Eliminate deadlock between CPU hotplug and expedited grace periods

Currently, the expedited grace-period primitives do get_online_cpus().
This greatly simplifies their implementation, but means that calls
to them holding locks that are acquired by CPU-hotplug notifiers (to
say nothing of calls to these primitives from CPU-hotplug notifiers)
can deadlock.  But this is starting to become inconvenient, as can be
seen here: https://lkml.org/lkml/2014/8/5/754.  The problem in this
case is that some developers need to acquire a mutex from a CPU-hotplug
notifier, but also need to hold it across a synchronize_rcu_expedited().
As noted above, this currently results in deadlock.

This commit avoids the deadlock and retains the simplicity by creating
a try_get_online_cpus(), which returns false if the get_online_cpus()
reference count could not immediately be incremented.  If a call to
try_get_online_cpus() returns true, the expedited primitives operate as
before.  If a call returns false, the expedited primitives fall back to
normal grace-period operations.  This falling back of course results in
increased grace-period latency, but only during times when CPU hotplug
operations are actually in flight.  The effect should therefore be
negligible during normal operation.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Tested-by: Lan Tianyu <tianyu.lan@intel.com>

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 95978ad7fcddae86ed517956b08aaa0d0615e221..b2d9a43012b20a18d7382bba29806554138383d7 100644 (file)
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -213,6 +213,7 @@ extern struct bus_type cpu_subsys;
 extern void cpu_hotplug_begin(void);
 extern void cpu_hotplug_done(void);
 extern void get_online_cpus(void);
+extern bool try_get_online_cpus(void);
 extern void put_online_cpus(void);
 extern void cpu_hotplug_disable(void);
 extern void cpu_hotplug_enable(void);
@@ -230,6 +231,7 @@ int cpu_down(unsigned int cpu);
 static inline void cpu_hotplug_begin(void) {}
 static inline void cpu_hotplug_done(void) {}
 #define get_online_cpus()      do { } while (0)
+#define try_get_online_cpus()  true
 #define put_online_cpus()      do { } while (0)
 #define cpu_hotplug_disable()  do { } while (0)
 #define cpu_hotplug_enable()   do { } while (0)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 008388f920d7e93e32ba388dd4b497f33d7bd49d..4f86465cc31744a77d14128a1ab924bef2f3164b 100644 (file)
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -505,6 +505,7 @@ static inline void print_irqtrace_events(struct task_struct *curr)
 
 #define lock_map_acquire(l)                    lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
 #define lock_map_acquire_read(l)               lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
+#define lock_map_acquire_tryread(l)            lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_)
 #define lock_map_release(l)                    lock_release(l, 1, _THIS_IP_)
 
 #ifdef CONFIG_PROVE_LOCKING

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 81e2a388a0f687eeb472bf33a4e24dd763ef34cf..356450f09c1f89d1ee35820a59a59e3a68cebe63 100644 (file)
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -79,6 +79,8 @@ static struct {
 
 /* Lockdep annotations for get/put_online_cpus() and cpu_hotplug_begin/end() */
 #define cpuhp_lock_acquire_read() lock_map_acquire_read(&cpu_hotplug.dep_map)
+#define cpuhp_lock_acquire_tryread() \
+                                 lock_map_acquire_tryread(&cpu_hotplug.dep_map)
 #define cpuhp_lock_acquire()      lock_map_acquire(&cpu_hotplug.dep_map)
 #define cpuhp_lock_release()      lock_map_release(&cpu_hotplug.dep_map)
 
@@ -91,10 +93,22 @@ void get_online_cpus(void)
        mutex_lock(&cpu_hotplug.lock);
        cpu_hotplug.refcount++;
        mutex_unlock(&cpu_hotplug.lock);
-
 }
 EXPORT_SYMBOL_GPL(get_online_cpus);
 
+bool try_get_online_cpus(void)
+{
+       if (cpu_hotplug.active_writer == current)
+               return true;
+       if (!mutex_trylock(&cpu_hotplug.lock))
+               return false;
+       cpuhp_lock_acquire_tryread();
+       cpu_hotplug.refcount++;
+       mutex_unlock(&cpu_hotplug.lock);
+       return true;
+}
+EXPORT_SYMBOL_GPL(try_get_online_cpus);
+
 void put_online_cpus(void)
 {
        if (cpu_hotplug.active_writer == current)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index d7a3b13bc94c782570fc7eb08649ae6334a3dd60..133e47223095d76fd1203f949a955009d1075688 100644 (file)
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2940,11 +2940,6 @@ static int synchronize_sched_expedited_cpu_stop(void *data)
  * restructure your code to batch your updates, and then use a single
  * synchronize_sched() instead.
  *
- * Note that it is illegal to call this function while holding any lock
- * that is acquired by a CPU-hotplug notifier.  And yes, it is also illegal
- * to call this function from a CPU-hotplug notifier.  Failing to observe
- * these restriction will result in deadlock.
- *
  * This implementation can be thought of as an application of ticket
  * locking to RCU, with sync_sched_expedited_started and
  * sync_sched_expedited_done taking on the roles of the halves
@@ -2994,7 +2989,12 @@ void synchronize_sched_expedited(void)
         */
        snap = atomic_long_inc_return(&rsp->expedited_start);
        firstsnap = snap;
-       get_online_cpus();
+       if (!try_get_online_cpus()) {
+               /* CPU hotplug operation in flight, fall back to normal GP. */
+               wait_rcu_gp(call_rcu_sched);
+               atomic_long_inc(&rsp->expedited_normal);
+               return;
+       }
        WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
 
        /*
@@ -3041,7 +3041,12 @@ void synchronize_sched_expedited(void)
                 * and they started after our first try, so their grace
                 * period works for us.
                 */
-               get_online_cpus();
+               if (!try_get_online_cpus()) {
+                       /* CPU hotplug operation in flight, use normal GP. */
+                       wait_rcu_gp(call_rcu_sched);
+                       atomic_long_inc(&rsp->expedited_normal);
+                       return;
+               }
                snap = atomic_long_read(&rsp->expedited_start);
                smp_mb(); /* ensure read is before try_stop_cpus(). */
        }

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index e2c5910546f6412598581dd3de4c531873e75c22..387dd45993449f7a54310fd936693bbc16611f5b 100644 (file)
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -793,11 +793,6 @@ sync_rcu_preempt_exp_init(struct rcu_state *rsp, struct rcu_node *rnp)
  * In fact, if you are using synchronize_rcu_expedited() in a loop,
  * please restructure your code to batch your updates, and then Use a
  * single synchronize_rcu() instead.
- *
- * Note that it is illegal to call this function while holding any lock
- * that is acquired by a CPU-hotplug notifier.  And yes, it is also illegal
- * to call this function from a CPU-hotplug notifier.  Failing to observe
- * these restriction will result in deadlock.
  */
 void synchronize_rcu_expedited(void)
 {
@@ -819,7 +814,11 @@ void synchronize_rcu_expedited(void)
         * being boosted.  This simplifies the process of moving tasks
         * from leaf to root rcu_node structures.
         */
-       get_online_cpus();
+       if (!try_get_online_cpus()) {
+               /* CPU-hotplug operation in flight, fall back to normal GP. */
+               wait_rcu_gp(call_rcu);
+               return;
+       }
 
        /*
         * Acquire lock, falling back to synchronize_rcu() if too many