forked from mirrors/linux
		
	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:
		
							parent
							
								
									ec4518aad8
								
							
						
					
					
						commit
						dd56af42bd
					
				
					 5 changed files with 35 additions and 14 deletions
				
			
		| 
						 | 
					@ -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)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
							
								
								
									
										16
									
								
								kernel/cpu.c
									
									
									
									
									
								
							
							
						
						
									
										16
									
								
								kernel/cpu.c
									
									
									
									
									
								
							| 
						 | 
					@ -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)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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(). */
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue