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>
This commit is contained in:
Paul E. McKenney 2014-08-25 20:25:06 -07:00
parent ec4518aad8
commit dd56af42bd
5 changed files with 35 additions and 14 deletions

View File

@ -213,6 +213,7 @@ extern struct bus_type cpu_subsys;
extern void cpu_hotplug_begin(void); extern void cpu_hotplug_begin(void);
extern void cpu_hotplug_done(void); extern void cpu_hotplug_done(void);
extern void get_online_cpus(void); extern void get_online_cpus(void);
extern bool try_get_online_cpus(void);
extern void put_online_cpus(void); extern void put_online_cpus(void);
extern void cpu_hotplug_disable(void); extern void cpu_hotplug_disable(void);
extern void cpu_hotplug_enable(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_begin(void) {}
static inline void cpu_hotplug_done(void) {} static inline void cpu_hotplug_done(void) {}
#define get_online_cpus() do { } while (0) #define get_online_cpus() do { } while (0)
#define try_get_online_cpus() true
#define put_online_cpus() do { } while (0) #define put_online_cpus() do { } while (0)
#define cpu_hotplug_disable() do { } while (0) #define cpu_hotplug_disable() do { } while (0)
#define cpu_hotplug_enable() do { } while (0) #define cpu_hotplug_enable() do { } while (0)

View File

@ -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(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_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_) #define lock_map_release(l) lock_release(l, 1, _THIS_IP_)
#ifdef CONFIG_PROVE_LOCKING #ifdef CONFIG_PROVE_LOCKING

View File

@ -79,6 +79,8 @@ static struct {
/* Lockdep annotations for get/put_online_cpus() and cpu_hotplug_begin/end() */ /* 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_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_acquire() lock_map_acquire(&cpu_hotplug.dep_map)
#define cpuhp_lock_release() lock_map_release(&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); mutex_lock(&cpu_hotplug.lock);
cpu_hotplug.refcount++; cpu_hotplug.refcount++;
mutex_unlock(&cpu_hotplug.lock); mutex_unlock(&cpu_hotplug.lock);
} }
EXPORT_SYMBOL_GPL(get_online_cpus); 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) void put_online_cpus(void)
{ {
if (cpu_hotplug.active_writer == current) if (cpu_hotplug.active_writer == current)

View File

@ -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 * restructure your code to batch your updates, and then use a single
* synchronize_sched() instead. * 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 * This implementation can be thought of as an application of ticket
* locking to RCU, with sync_sched_expedited_started and * locking to RCU, with sync_sched_expedited_started and
* sync_sched_expedited_done taking on the roles of the halves * 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); snap = atomic_long_inc_return(&rsp->expedited_start);
firstsnap = snap; 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())); 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 * and they started after our first try, so their grace
* period works for us. * 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); snap = atomic_long_read(&rsp->expedited_start);
smp_mb(); /* ensure read is before try_stop_cpus(). */ smp_mb(); /* ensure read is before try_stop_cpus(). */
} }

View File

@ -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, * In fact, if you are using synchronize_rcu_expedited() in a loop,
* please restructure your code to batch your updates, and then Use a * please restructure your code to batch your updates, and then Use a
* single synchronize_rcu() instead. * 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) void synchronize_rcu_expedited(void)
{ {
@ -819,7 +814,11 @@ void synchronize_rcu_expedited(void)
* being boosted. This simplifies the process of moving tasks * being boosted. This simplifies the process of moving tasks
* from leaf to root rcu_node structures. * 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 * Acquire lock, falling back to synchronize_rcu() if too many