Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dd56af42 authored by Paul E. McKenney's avatar Paul E. McKenney
Browse files

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: default avatarPaul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Tested-by: default avatarLan Tianyu <tianyu.lan@intel.com>
parent ec4518aa

include/linux/cpu.h

+2 −0
Original line number Diff line number Diff line
@@ -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)

include/linux/lockdep.h

+1 −0
Original line number Diff line number Diff line
@@ -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

kernel/cpu.c

+15 −1
Original line number Diff line number Diff line
@@ -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)

kernel/rcu/tree.c

+12 −7
Original line number Diff line number Diff line
@@ -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(). */

	}

kernel/rcu/tree_plugin.h

+5 −6
Original line number Diff line number Diff line
@@ -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