linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
@ 2014-08-28 19:47 Paul E. McKenney
  2014-08-29  6:54 ` Lan Tianyu
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Paul E. McKenney @ 2014-08-28 19:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, tglx,
	peterz, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

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:
https://lkml.org/lkml/2014/8/5/754

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>
Cc: Lan Tianyu <tianyu.lan@intel.com>

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 95978ad7fcdd..b2d9a43012b2 100644
--- 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 008388f920d7..4f86465cc317 100644
--- 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 81e2a388a0f6..356450f09c1f 100644
--- 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 d7a3b13bc94c..04558f0c9d64 100644
--- 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()));
 
 	/*
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index fb833811c2f6..821dcf9a3b94 100644
--- 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-08-28 19:47 [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods Paul E. McKenney
@ 2014-08-29  6:54 ` Lan Tianyu
  2014-08-29 13:11   ` Paul E. McKenney
  2014-09-01 11:20 ` Peter Zijlstra
  2014-09-17  7:11 ` Lan Tianyu
  2 siblings, 1 reply; 17+ messages in thread
From: Lan Tianyu @ 2014-08-29  6:54 UTC (permalink / raw)
  To: paulmck, linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, tglx,
	peterz, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, Rafael J. Wysocki

On 2014年08月29日 03:47, Paul E. McKenney wrote:
> 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:
> https://lkml.org/lkml/2014/8/5/754
> 
> 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>
> Cc: Lan Tianyu <tianyu.lan@intel.com>
> 

Hi Paul:
	I tested this patch and it fixes my issue. Thanks.

Tested-by: Lan Tianyu <tianyu.lan@intel.com>

> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 95978ad7fcdd..b2d9a43012b2 100644
> --- 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 008388f920d7..4f86465cc317 100644
> --- 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 81e2a388a0f6..356450f09c1f 100644
> --- 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 d7a3b13bc94c..04558f0c9d64 100644
> --- 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()));
>  
>  	/*
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index fb833811c2f6..821dcf9a3b94 100644
> --- 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
> 


-- 
Best regards
Tianyu Lan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-08-29  6:54 ` Lan Tianyu
@ 2014-08-29 13:11   ` Paul E. McKenney
  0 siblings, 0 replies; 17+ messages in thread
From: Paul E. McKenney @ 2014-08-29 13:11 UTC (permalink / raw)
  To: Lan Tianyu
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, peterz, rostedt, dhowells, edumazet, dvhart,
	fweisbec, oleg, bobby.prani, Rafael J. Wysocki

On Fri, Aug 29, 2014 at 02:54:54PM +0800, Lan Tianyu wrote:
> On 2014年08月29日 03:47, Paul E. McKenney wrote:
> > 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:
> > https://lkml.org/lkml/2014/8/5/754
> > 
> > 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>
> > Cc: Lan Tianyu <tianyu.lan@intel.com>
> > 
> 
> Hi Paul:
> 	I tested this patch and it fixes my issue. Thanks.
> 
> Tested-by: Lan Tianyu <tianyu.lan@intel.com>

Glad it worked for you, and thank you for your testing efforts!

							Thanx, Paul

> > diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> > index 95978ad7fcdd..b2d9a43012b2 100644
> > --- 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 008388f920d7..4f86465cc317 100644
> > --- 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 81e2a388a0f6..356450f09c1f 100644
> > --- 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 d7a3b13bc94c..04558f0c9d64 100644
> > --- 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()));
> >  
> >  	/*
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index fb833811c2f6..821dcf9a3b94 100644
> > --- 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
> > 
> 
> 
> -- 
> Best regards
> Tianyu Lan
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-08-28 19:47 [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods Paul E. McKenney
  2014-08-29  6:54 ` Lan Tianyu
@ 2014-09-01 11:20 ` Peter Zijlstra
  2014-09-01 16:05   ` Paul E. McKenney
  2014-09-17  7:11 ` Lan Tianyu
  2 siblings, 1 reply; 17+ messages in thread
From: Peter Zijlstra @ 2014-09-01 11:20 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Thu, Aug 28, 2014 at 12:47:45PM -0700, Paul E. McKenney wrote:
> 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:
> https://lkml.org/lkml/2014/8/5/754

Please recap the actual problem; the link might die and the actual mail
linked to isn't very useful in any case.

> 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.

URGH.. I really hate that. The hotplug interface is already too
horrible, we should not add such hacks to it.

How about ripping that rcu_expedited stuff out instead? That's all
conditional anyhow, so might as well not do it.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-01 11:20 ` Peter Zijlstra
@ 2014-09-01 16:05   ` Paul E. McKenney
  2014-09-01 16:17     ` Peter Zijlstra
  0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-01 16:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Mon, Sep 01, 2014 at 01:20:59PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 28, 2014 at 12:47:45PM -0700, Paul E. McKenney wrote:
> > 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:
> > https://lkml.org/lkml/2014/8/5/754
> 
> Please recap the actual problem; the link might die and the actual mail
> linked to isn't very useful in any case.

Will do.

> > 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.
> 
> URGH.. I really hate that. The hotplug interface is already too
> horrible, we should not add such hacks to it.

We do have try_ interfaces to a number of other subsystems, so I don't
believe that it qualifies as such a hack.

> How about ripping that rcu_expedited stuff out instead? That's all
> conditional anyhow, so might as well not do it.

In what way is the expedited stuff conditional?

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-01 16:05   ` Paul E. McKenney
@ 2014-09-01 16:17     ` Peter Zijlstra
  2014-09-02 16:36       ` Paul E. McKenney
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Zijlstra @ 2014-09-01 16:17 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Mon, Sep 01, 2014 at 09:05:50AM -0700, Paul E. McKenney wrote:
> > URGH.. I really hate that. The hotplug interface is already too
> > horrible, we should not add such hacks to it.
> 
> We do have try_ interfaces to a number of other subsystems, so I don't
> believe that it qualifies as such a hack.

We do indeed, but I'm not sure about adding this to the hotplug stuff.

Also; not really understanding the problem doesn't help. 

> > How about ripping that rcu_expedited stuff out instead? That's all
> > conditional anyhow, so might as well not do it.
> 
> In what way is the expedited stuff conditional?

synchronize_sched() conditionally calls synchronize_sched_expedited()
and its condition: rcu_expedited, gets set/cleared on pm notifiers and
nr_cpu_ids.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-01 16:17     ` Peter Zijlstra
@ 2014-09-02 16:36       ` Paul E. McKenney
  2014-09-03 11:31         ` Peter Zijlstra
  0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-02 16:36 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Mon, Sep 01, 2014 at 06:17:35PM +0200, Peter Zijlstra wrote:
> On Mon, Sep 01, 2014 at 09:05:50AM -0700, Paul E. McKenney wrote:
> > > URGH.. I really hate that. The hotplug interface is already too
> > > horrible, we should not add such hacks to it.
> > 
> > We do have try_ interfaces to a number of other subsystems, so I don't
> > believe that it qualifies as such a hack.
> 
> We do indeed, but I'm not sure about adding this to the hotplug stuff.

Looks pretty straightforward to me.

> Also; not really understanding the problem doesn't help. 

The current implementation of synchronize_sched_expedited()
calls get_online_cpus().  Some of the ACPI code needs to hold the
acpi_ioremap_lock mutex across synchronize_sched_expedited(), and
also needs to acquire this same mutex from a CPU hotplug notifier.
This results in deadlock between the cpu_hotplug.lock mutex and the
acpi_ioremap_lock mutex.

Normal RCU grace periods avoid this by synchronizing on a lock acquired by
the RCU CPU-hotplug notifiers, but this does not work for the expedited
grace periods because the outgoing CPU can be running random tasks for
quite some time after RCU's notifier executes.  So the fix is just to
drop back to a normal grace period when there is a CPU-hotplug operation
in progress.

> > > How about ripping that rcu_expedited stuff out instead? That's all
> > > conditional anyhow, so might as well not do it.
> > 
> > In what way is the expedited stuff conditional?
> 
> synchronize_sched() conditionally calls synchronize_sched_expedited()
> and its condition: rcu_expedited, gets set/cleared on pm notifiers and
> nr_cpu_ids.

There are also direct calls to both synchronize_sched_expedited() and
synchronize_rcu_expedited().

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-02 16:36       ` Paul E. McKenney
@ 2014-09-03 11:31         ` Peter Zijlstra
  2014-09-03 15:03           ` Paul E. McKenney
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Zijlstra @ 2014-09-03 11:31 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Tue, Sep 02, 2014 at 09:36:56AM -0700, Paul E. McKenney wrote:
> On Mon, Sep 01, 2014 at 06:17:35PM +0200, Peter Zijlstra wrote:
> > On Mon, Sep 01, 2014 at 09:05:50AM -0700, Paul E. McKenney wrote:
> > > > URGH.. I really hate that. The hotplug interface is already too
> > > > horrible, we should not add such hacks to it.
> > > 
> > > We do have try_ interfaces to a number of other subsystems, so I don't
> > > believe that it qualifies as such a hack.
> > 
> > We do indeed, but I'm not sure about adding this to the hotplug stuff.
> 
> Looks pretty straightforward to me.
> 
> > Also; not really understanding the problem doesn't help. 
> 
> The current implementation of synchronize_sched_expedited()
> calls get_online_cpus().  Some of the ACPI code needs to hold the
> acpi_ioremap_lock mutex across synchronize_sched_expedited(), and
> also needs to acquire this same mutex from a CPU hotplug notifier.
> This results in deadlock between the cpu_hotplug.lock mutex and the
> acpi_ioremap_lock mutex.


  acpi_ioremap_lock			cpu_hotplug_begin()
    synchronize_sched()			  acpi_ioremap_lock
      get_online_cpus()

So yes, AB-BA.

> Normal RCU grace periods avoid this by synchronizing on a lock acquired by
> the RCU CPU-hotplug notifiers, but this does not work for the expedited
> grace periods because the outgoing CPU can be running random tasks for
> quite some time after RCU's notifier executes.  So the fix is just to
> drop back to a normal grace period when there is a CPU-hotplug operation
> in progress.

So why are we 'normally' doing an expedited call here anyhow? 

> > > > How about ripping that rcu_expedited stuff out instead? That's all
> > > > conditional anyhow, so might as well not do it.
> > > 
> > > In what way is the expedited stuff conditional?
> > 
> > synchronize_sched() conditionally calls synchronize_sched_expedited()
> > and its condition: rcu_expedited, gets set/cleared on pm notifiers and
> > nr_cpu_ids.
> 
> There are also direct calls to both synchronize_sched_expedited() and
> synchronize_rcu_expedited().

But those are not within hotplug bits. Also weren't we removing them? I
thought we didn't appreciate spraying IPIs like they do?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-03 11:31         ` Peter Zijlstra
@ 2014-09-03 15:03           ` Paul E. McKenney
  2014-09-03 15:28             ` Peter Zijlstra
  0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-03 15:03 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Wed, Sep 03, 2014 at 01:31:12PM +0200, Peter Zijlstra wrote:
> On Tue, Sep 02, 2014 at 09:36:56AM -0700, Paul E. McKenney wrote:
> > On Mon, Sep 01, 2014 at 06:17:35PM +0200, Peter Zijlstra wrote:
> > > On Mon, Sep 01, 2014 at 09:05:50AM -0700, Paul E. McKenney wrote:
> > > > > URGH.. I really hate that. The hotplug interface is already too
> > > > > horrible, we should not add such hacks to it.
> > > > 
> > > > We do have try_ interfaces to a number of other subsystems, so I don't
> > > > believe that it qualifies as such a hack.
> > > 
> > > We do indeed, but I'm not sure about adding this to the hotplug stuff.
> > 
> > Looks pretty straightforward to me.
> > 
> > > Also; not really understanding the problem doesn't help. 
> > 
> > The current implementation of synchronize_sched_expedited()
> > calls get_online_cpus().  Some of the ACPI code needs to hold the
> > acpi_ioremap_lock mutex across synchronize_sched_expedited(), and
> > also needs to acquire this same mutex from a CPU hotplug notifier.
> > This results in deadlock between the cpu_hotplug.lock mutex and the
> > acpi_ioremap_lock mutex.
> 
> 
>   acpi_ioremap_lock			cpu_hotplug_begin()
>     synchronize_sched()			  acpi_ioremap_lock
>       get_online_cpus()
> 
> So yes, AB-BA.
> 
> > Normal RCU grace periods avoid this by synchronizing on a lock acquired by
> > the RCU CPU-hotplug notifiers, but this does not work for the expedited
> > grace periods because the outgoing CPU can be running random tasks for
> > quite some time after RCU's notifier executes.  So the fix is just to
> > drop back to a normal grace period when there is a CPU-hotplug operation
> > in progress.
> 
> So why are we 'normally' doing an expedited call here anyhow? 

Presumably because they set either the boot parameter or
the sysfs variable that causes synchronize_sched() to so
synchronize_sched_expedited().

> > > > > How about ripping that rcu_expedited stuff out instead? That's all
> > > > > conditional anyhow, so might as well not do it.
> > > > 
> > > > In what way is the expedited stuff conditional?
> > > 
> > > synchronize_sched() conditionally calls synchronize_sched_expedited()
> > > and its condition: rcu_expedited, gets set/cleared on pm notifiers and
> > > nr_cpu_ids.
> > 
> > There are also direct calls to both synchronize_sched_expedited() and
> > synchronize_rcu_expedited().
> 
> But those are not within hotplug bits. Also weren't we removing them? I
> thought we didn't appreciate spraying IPIs like they do?

I hadn't heard anything about removing them, but making the
expedited primitives a bit less IPI-happy is on my list.

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-03 15:03           ` Paul E. McKenney
@ 2014-09-03 15:28             ` Peter Zijlstra
  2014-09-03 16:38               ` Paul E. McKenney
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Zijlstra @ 2014-09-03 15:28 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Wed, Sep 03, 2014 at 08:03:06AM -0700, Paul E. McKenney wrote:
> > > Normal RCU grace periods avoid this by synchronizing on a lock acquired by
> > > the RCU CPU-hotplug notifiers, but this does not work for the expedited
> > > grace periods because the outgoing CPU can be running random tasks for
> > > quite some time after RCU's notifier executes.  So the fix is just to
> > > drop back to a normal grace period when there is a CPU-hotplug operation
> > > in progress.
> > 
> > So why are we 'normally' doing an expedited call here anyhow? 
> 
> Presumably because they set either the boot parameter or
> the sysfs variable that causes synchronize_sched() to so
> synchronize_sched_expedited().

That's not a why but a how. Why does that option exist, why are we doing
this?

I cannot actually find a sysfs variable that controls this though; only
the rcu_pm_notifier. It seems to favour doing an expedited call when
suspending on 'small' machines.

> > But those are not within hotplug bits. Also weren't we removing them? I
> > thought we didn't appreciate spraying IPIs like they do?
> 
> I hadn't heard anything about removing them, but making the
> expedited primitives a bit less IPI-happy is on my list.

I had some recollections of removing a fair number of expedited calls,
but its was a long while ago so what do I know ;-)

Making them less IPI happy would be good indeed.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-03 15:28             ` Peter Zijlstra
@ 2014-09-03 16:38               ` Paul E. McKenney
  0 siblings, 0 replies; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-03 16:38 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, ,
	tianyu.lan

On Wed, Sep 03, 2014 at 05:28:50PM +0200, Peter Zijlstra wrote:
> On Wed, Sep 03, 2014 at 08:03:06AM -0700, Paul E. McKenney wrote:
> > > > Normal RCU grace periods avoid this by synchronizing on a lock acquired by
> > > > the RCU CPU-hotplug notifiers, but this does not work for the expedited
> > > > grace periods because the outgoing CPU can be running random tasks for
> > > > quite some time after RCU's notifier executes.  So the fix is just to
> > > > drop back to a normal grace period when there is a CPU-hotplug operation
> > > > in progress.
> > > 
> > > So why are we 'normally' doing an expedited call here anyhow? 
> > 
> > Presumably because they set either the boot parameter or
> > the sysfs variable that causes synchronize_sched() to so
> > synchronize_sched_expedited().
> 
> That's not a why but a how. Why does that option exist, why are we doing
> this?

As you say below, to reduce RCU grace-period latency on small systems.
And one level of abstraction's why is another level's how.  ;-)

> I cannot actually find a sysfs variable that controls this though; only
> the rcu_pm_notifier. It seems to favour doing an expedited call when
> suspending on 'small' machines.

See rcu_expedited_store() in kernel/ksysfs.c.  Or the rcu_expedited
module_param() in kernel/rcu/update.c.

> > > But those are not within hotplug bits. Also weren't we removing them? I
> > > thought we didn't appreciate spraying IPIs like they do?
> > 
> > I hadn't heard anything about removing them, but making the
> > expedited primitives a bit less IPI-happy is on my list.
> 
> I had some recollections of removing a fair number of expedited calls,
> but its was a long while ago so what do I know ;-)

If a given use case can tolerate the latency of normal calls, then the
normal calls certainly are preferable, no two ways about it.

> Making them less IPI happy would be good indeed.

Priority of this task duly increased.  ;-)

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-08-28 19:47 [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods Paul E. McKenney
  2014-08-29  6:54 ` Lan Tianyu
  2014-09-01 11:20 ` Peter Zijlstra
@ 2014-09-17  7:11 ` Lan Tianyu
  2014-09-17 13:10   ` Paul E. McKenney
  2 siblings, 1 reply; 17+ messages in thread
From: Lan Tianyu @ 2014-09-17  7:11 UTC (permalink / raw)
  To: paulmck, linux-kernel
  Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, tglx,
	peterz, rostedt, dhowells, edumazet, dvhart, fweisbec, oleg,
	bobby.prani, Rafael J. Wysocki

On 2014年08月29日 03:47, Paul E. McKenney wrote:
> 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:
> https://lkml.org/lkml/2014/8/5/754
> 
> 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>
> Cc: Lan Tianyu <tianyu.lan@intel.com>

Hi Paul:
	What's the status of the patch? Will you push it? Thanks.

-- 
Best regards
Tianyu Lan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-17  7:11 ` Lan Tianyu
@ 2014-09-17 13:10   ` Paul E. McKenney
  2014-09-18  7:15     ` Lan Tianyu
  0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-17 13:10 UTC (permalink / raw)
  To: Lan Tianyu
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, peterz, rostedt, dhowells, edumazet, dvhart,
	fweisbec, oleg, bobby.prani, Rafael J. Wysocki

On Wed, Sep 17, 2014 at 03:11:42PM +0800, Lan Tianyu wrote:
> On 2014年08月29日 03:47, Paul E. McKenney wrote:
> > 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:
> > https://lkml.org/lkml/2014/8/5/754
> > 
> > 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>
> > Cc: Lan Tianyu <tianyu.lan@intel.com>
> 
> Hi Paul:
> 	What's the status of the patch? Will you push it? Thanks.

By default, it would go into 3.19.  Do you need it earlier?

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-17 13:10   ` Paul E. McKenney
@ 2014-09-18  7:15     ` Lan Tianyu
  2014-09-18 12:38       ` Paul E. McKenney
  0 siblings, 1 reply; 17+ messages in thread
From: Lan Tianyu @ 2014-09-18  7:15 UTC (permalink / raw)
  To: paulmck
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, peterz, rostedt, dhowells, edumazet, dvhart,
	fweisbec, oleg, bobby.prani, Rafael J. Wysocki

On 2014年09月17日 21:10, Paul E. McKenney wrote:
> On Wed, Sep 17, 2014 at 03:11:42PM +0800, Lan Tianyu wrote:
>> On 2014年08月29日 03:47, Paul E. McKenney wrote:
>>> 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:
>>> https://lkml.org/lkml/2014/8/5/754
>>>
>>> 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>
>>> Cc: Lan Tianyu <tianyu.lan@intel.com>
>>
>> Hi Paul:
>> 	What's the status of the patch? Will you push it? Thanks.
> 
> By default, it would go into 3.19.  Do you need it earlier?

IMO, this is a dead lock bug which is hard to reproduce and the patch
should go into v3.17 and stable tree?

> 
> 							Thanx, Paul
> 


-- 
Best regards
Tianyu Lan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-18  7:15     ` Lan Tianyu
@ 2014-09-18 12:38       ` Paul E. McKenney
  2014-09-18 22:55         ` Rafael J. Wysocki
  0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-18 12:38 UTC (permalink / raw)
  To: Lan Tianyu
  Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
	josh, tglx, peterz, rostedt, dhowells, edumazet, dvhart,
	fweisbec, oleg, bobby.prani, Rafael J. Wysocki

On Thu, Sep 18, 2014 at 03:15:36PM +0800, Lan Tianyu wrote:
> On 2014年09月17日 21:10, Paul E. McKenney wrote:
> > On Wed, Sep 17, 2014 at 03:11:42PM +0800, Lan Tianyu wrote:
> >> On 2014年08月29日 03:47, Paul E. McKenney wrote:
> >>> 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:
> >>> https://lkml.org/lkml/2014/8/5/754
> >>>
> >>> 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>
> >>> Cc: Lan Tianyu <tianyu.lan@intel.com>
> >>
> >> Hi Paul:
> >> 	What's the status of the patch? Will you push it? Thanks.
> > 
> > By default, it would go into 3.19.  Do you need it earlier?
> 
> IMO, this is a dead lock bug which is hard to reproduce and the patch
> should go into v3.17 and stable tree?

The problem with pushing for v3.17 is that I would have to rebase
that commit to the bottom of my current stack and redo all my testing.
If there were any problems, I could not only miss v3.17, but also miss
the v3.18 merge window.

So, given that the next merge window happens pretty soon, how about
v3.18 and the stable tree?

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-18 12:38       ` Paul E. McKenney
@ 2014-09-18 22:55         ` Rafael J. Wysocki
  2014-09-18 22:57           ` Paul E. McKenney
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael J. Wysocki @ 2014-09-18 22:55 UTC (permalink / raw)
  To: paulmck
  Cc: Lan Tianyu, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, dvhart, fweisbec, oleg, bobby.prani

On Thursday, September 18, 2014 05:38:45 AM Paul E. McKenney wrote:
> On Thu, Sep 18, 2014 at 03:15:36PM +0800, Lan Tianyu wrote:
> > On 2014年09月17日 21:10, Paul E. McKenney wrote:
> > > On Wed, Sep 17, 2014 at 03:11:42PM +0800, Lan Tianyu wrote:
> > >> On 2014年08月29日 03:47, Paul E. McKenney wrote:
> > >>> 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:
> > >>> https://lkml.org/lkml/2014/8/5/754
> > >>>
> > >>> 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>
> > >>> Cc: Lan Tianyu <tianyu.lan@intel.com>
> > >>
> > >> Hi Paul:
> > >> 	What's the status of the patch? Will you push it? Thanks.
> > > 
> > > By default, it would go into 3.19.  Do you need it earlier?
> > 
> > IMO, this is a dead lock bug which is hard to reproduce and the patch
> > should go into v3.17 and stable tree?
> 
> The problem with pushing for v3.17 is that I would have to rebase
> that commit to the bottom of my current stack and redo all my testing.
> If there were any problems, I could not only miss v3.17, but also miss
> the v3.18 merge window.
> 
> So, given that the next merge window happens pretty soon, how about
> v3.18 and the stable tree?

That sounds good to me.

Rafael


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods
  2014-09-18 22:55         ` Rafael J. Wysocki
@ 2014-09-18 22:57           ` Paul E. McKenney
  0 siblings, 0 replies; 17+ messages in thread
From: Paul E. McKenney @ 2014-09-18 22:57 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lan Tianyu, linux-kernel, mingo, laijs, dipankar, akpm,
	mathieu.desnoyers, josh, tglx, peterz, rostedt, dhowells,
	edumazet, dvhart, fweisbec, oleg, bobby.prani

On Fri, Sep 19, 2014 at 12:55:11AM +0200, Rafael J. Wysocki wrote:
> On Thursday, September 18, 2014 05:38:45 AM Paul E. McKenney wrote:
> > On Thu, Sep 18, 2014 at 03:15:36PM +0800, Lan Tianyu wrote:
> > > On 2014年09月17日 21:10, Paul E. McKenney wrote:
> > > > On Wed, Sep 17, 2014 at 03:11:42PM +0800, Lan Tianyu wrote:
> > > >> On 2014年08月29日 03:47, Paul E. McKenney wrote:
> > > >>> 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:
> > > >>> https://lkml.org/lkml/2014/8/5/754
> > > >>>
> > > >>> 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>
> > > >>> Cc: Lan Tianyu <tianyu.lan@intel.com>
> > > >>
> > > >> Hi Paul:
> > > >> 	What's the status of the patch? Will you push it? Thanks.
> > > > 
> > > > By default, it would go into 3.19.  Do you need it earlier?
> > > 
> > > IMO, this is a dead lock bug which is hard to reproduce and the patch
> > > should go into v3.17 and stable tree?
> > 
> > The problem with pushing for v3.17 is that I would have to rebase
> > that commit to the bottom of my current stack and redo all my testing.
> > If there were any problems, I could not only miss v3.17, but also miss
> > the v3.18 merge window.
> > 
> > So, given that the next merge window happens pretty soon, how about
> > v3.18 and the stable tree?
> 
> That sounds good to me.

Very good, I have added it to my v3.18 queue.

							Thanx, Paul


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-09-18 22:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-28 19:47 [PATCH RFC tip/core/rcu] Eliminate deadlock between CPU hotplug and expedited grace periods Paul E. McKenney
2014-08-29  6:54 ` Lan Tianyu
2014-08-29 13:11   ` Paul E. McKenney
2014-09-01 11:20 ` Peter Zijlstra
2014-09-01 16:05   ` Paul E. McKenney
2014-09-01 16:17     ` Peter Zijlstra
2014-09-02 16:36       ` Paul E. McKenney
2014-09-03 11:31         ` Peter Zijlstra
2014-09-03 15:03           ` Paul E. McKenney
2014-09-03 15:28             ` Peter Zijlstra
2014-09-03 16:38               ` Paul E. McKenney
2014-09-17  7:11 ` Lan Tianyu
2014-09-17 13:10   ` Paul E. McKenney
2014-09-18  7:15     ` Lan Tianyu
2014-09-18 12:38       ` Paul E. McKenney
2014-09-18 22:55         ` Rafael J. Wysocki
2014-09-18 22:57           ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).