[PATCH] UNTESTED backport rcu_needs_cpu and call it from stop_hz_timer UNTESTED

[PATCH] UNTESTED backport rcu_needs_cpu and call it from stop_hz_timer UNTESTED

[Harry Butterworth]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

THIS PATCH FOR REVIEW ONLY IT'S UNTESTED!!!!!!  Ill test on Monday if
no-one has done it by then.

There is a problem with the current implementation of stop_hz_timer in
arch/i386/kernel/time-xen.c where the hz timer can be stopped on a CPU
which has RCU callbacks pending.

This patch backports a new RCU API created to fix this problem for the
s390 implementation of stop_hz_timer and also updates the time-xen.c
implementation of stop_hz_timer to call the new API.

Signed-off-by: Harry Butterworth <butterwo@uk.ibm.com>


[-- Attachment #2: rcu_needs_cpu.patch --]
[-- Type: text/x-patch, Size: 2544 bytes --]

diff -r 8c64169a05d3 -r 8ef455f268b3 linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c
--- a/linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c	Thu Jun  8 08:52:04 2006
+++ b/linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c	Fri Jun  9 19:47:18 2006
@@ -978,12 +978,19 @@
 	unsigned int cpu = smp_processor_id();
 	unsigned long j;
 
-	/* We must do this /before/ checking rcu_pending(). */
 	cpu_set(cpu, nohz_cpu_mask);
+
+	/* See matching smp_mb in rcu_start_batch in rcupdate.c.  These mbs  */
+	/* ensure that if __rcu_pending (nested in rcu_needs_cpu) fetches a  */
+	/* value of rcp->cur that matches rdp->quiescbatch and allows us to  */
+	/* stop the hz timer then the cpumasks created for subsequent values */
+	/* of cur in rcu_start_batch are guaranteed to pick up the updated   */
+	/* nohz_cpu_mask and so will not depend on this cpu.                 */
+
 	smp_mb();
 
 	/* Leave ourselves in 'tick mode' if rcu or softirq pending. */
-	if (rcu_pending(cpu) || local_softirq_pending()) {
+	if (rcu_needs_cpu(cpu) || local_softirq_pending()) {
 		cpu_clear(cpu, nohz_cpu_mask);
 		j = jiffies + 1;
 	} else {
diff -r 8c64169a05d3 -r 8ef455f268b3 patches/linux-2.6.16.13/rcu_needs_cpu.patch
--- /dev/null	Thu Jun  8 08:52:04 2006
+++ b/patches/linux-2.6.16.13/rcu_needs_cpu.patch	Fri Jun  9 19:47:18 2006
@@ -0,0 +1,33 @@
+--- ../pristine-linux-2.6.16.13/kernel/rcupdate.c	2006-05-02 22:38:44.000000000 +0100
++++ ./kernel/rcupdate.c	2006-06-09 20:27:45.000000000 +0100
+@@ -485,6 +485,20 @@ int rcu_pending(int cpu)
+ 		__rcu_pending(&rcu_bh_ctrlblk, &per_cpu(rcu_bh_data, cpu));
+ }
+ 
++/*
++ * Check to see if any future RCU-related work will need to be done
++ * by the current CPU, even if none need be done immediately, returning
++ * 1 if so.  This function is part of the RCU implementation; it is -not-
++ * an exported member of the RCU API.
++ */
++int rcu_needs_cpu(int cpu)
++{
++	struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
++	struct rcu_data *rdp_bh = &per_cpu(rcu_bh_data, cpu);
++
++	return (!!rdp->curlist || !!rdp_bh->curlist || rcu_pending(cpu));
++}
++
+ void rcu_check_callbacks(int cpu, int user)
+ {
+ 	if (user || 
+--- ../pristine-linux-2.6.16.13/include/linux/rcupdate.h	2006-05-02 22:38:44.000000000 +0100
++++ ./include/linux/rcupdate.h	2006-06-09 20:28:57.000000000 +0100
+@@ -134,6 +134,7 @@ static inline void rcu_bh_qsctr_inc(int 
+ }
+ 
+ extern int rcu_pending(int cpu);
++extern int rcu_needs_cpu(int cpu);
+ 
+ /**
+  * rcu_read_lock - mark the beginning of an RCU read-side critical section.

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] UNTESTED backport rcu_needs_cpu and call it from stop_hz_timer UNTESTED

[David F Barrera]

Harry Butterworth wrote:
> THIS PATCH FOR REVIEW ONLY IT'S UNTESTED!!!!!!  Ill test on Monday if
> no-one has done it by then.
>
> There is a problem with the current implementation of stop_hz_timer in
> arch/i386/kernel/time-xen.c where the hz timer can be stopped on a CPU
> which has RCU callbacks pending.
>
> This patch backports a new RCU API created to fix this problem for the
> s390 implementation of stop_hz_timer and also updates the time-xen.c
> implementation of stop_hz_timer to call the new API.
>
> Signed-off-by: Harry Butterworth <butterwo@uk.ibm.com>
>
>   
> ------------------------------------------------------------------------
>
> diff -r 8c64169a05d3 -r 8ef455f268b3 linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c
> --- a/linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c	Thu Jun  8 08:52:04 2006
> +++ b/linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c	Fri Jun  9 19:47:18 2006
> @@ -978,12 +978,19 @@
>  	unsigned int cpu = smp_processor_id();
>  	unsigned long j;
>  
> -	/* We must do this /before/ checking rcu_pending(). */
>  	cpu_set(cpu, nohz_cpu_mask);
> +
> +	/* See matching smp_mb in rcu_start_batch in rcupdate.c.  These mbs  */
> +	/* ensure that if __rcu_pending (nested in rcu_needs_cpu) fetches a  */
> +	/* value of rcp->cur that matches rdp->quiescbatch and allows us to  */
> +	/* stop the hz timer then the cpumasks created for subsequent values */
> +	/* of cur in rcu_start_batch are guaranteed to pick up the updated   */
> +	/* nohz_cpu_mask and so will not depend on this cpu.                 */
> +
>  	smp_mb();
>  
>  	/* Leave ourselves in 'tick mode' if rcu or softirq pending. */
> -	if (rcu_pending(cpu) || local_softirq_pending()) {
> +	if (rcu_needs_cpu(cpu) || local_softirq_pending()) {
>  		cpu_clear(cpu, nohz_cpu_mask);
>  		j = jiffies + 1;
>  	} else {
> diff -r 8c64169a05d3 -r 8ef455f268b3 patches/linux-2.6.16.13/rcu_needs_cpu.patch
> --- /dev/null	Thu Jun  8 08:52:04 2006
> +++ b/patches/linux-2.6.16.13/rcu_needs_cpu.patch	Fri Jun  9 19:47:18 2006
> @@ -0,0 +1,33 @@
> +--- ../pristine-linux-2.6.16.13/kernel/rcupdate.c	2006-05-02 22:38:44.000000000 +0100
> ++++ ./kernel/rcupdate.c	2006-06-09 20:27:45.000000000 +0100
> +@@ -485,6 +485,20 @@ int rcu_pending(int cpu)
> + 		__rcu_pending(&rcu_bh_ctrlblk, &per_cpu(rcu_bh_data, cpu));
> + }
> + 
> ++/*
> ++ * Check to see if any future RCU-related work will need to be done
> ++ * by the current CPU, even if none need be done immediately, returning
> ++ * 1 if so.  This function is part of the RCU implementation; it is -not-
> ++ * an exported member of the RCU API.
> ++ */
> ++int rcu_needs_cpu(int cpu)
> ++{
> ++	struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
> ++	struct rcu_data *rdp_bh = &per_cpu(rcu_bh_data, cpu);
> ++
> ++	return (!!rdp->curlist || !!rdp_bh->curlist || rcu_pending(cpu));
> ++}
> ++
> + void rcu_check_callbacks(int cpu, int user)
> + {
> + 	if (user || 
> +--- ../pristine-linux-2.6.16.13/include/linux/rcupdate.h	2006-05-02 22:38:44.000000000 +0100
> ++++ ./include/linux/rcupdate.h	2006-06-09 20:28:57.000000000 +0100
> +@@ -134,6 +134,7 @@ static inline void rcu_bh_qsctr_inc(int 
> + }
> + 
> + extern int rcu_pending(int cpu);
> ++extern int rcu_needs_cpu(int cpu);
> + 
> + /**
> +  * rcu_read_lock - mark the beginning of an RCU read-side critical section.
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>   
Harry,

I am doing some quick testing of your patch. So far, the results are 
consistent with the mainline testing results.

-- 

Regards,

David F Barrera
Linux Technology Center
Systems and Technology Group, IBM

"The wisest men follow their own direction. "
	
                          Euripides

[PATCH] backport rcu_needs_cpu and call it from stop_hz_timer

[Harry Butterworth]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

I have now tested this against xen-testing 9735.  Without the patch, and
with a domU kernel configured with CONFIG_DEBUG_SOFTLOCKUP disabled
(this debug code masks the problem), and smp and cpu hotplug enabled, a
script which does lots of cpu hotplug/removal will hang (cpu
hotplug/removal stresses the RCU synchronize_sched function).  With the
patch, the script doing cpu hotplug/removal runs for as long as I have
let it so far (thousands of hotplugs).

Please apply.

Signed-off-by: Harry Butterworth <butterwo@uk.ibm.com>

On Fri, 2006-06-09 at 21:21 +0100, Harry Butterworth wrote:
> There is a problem with the current implementation of stop_hz_timer in
> arch/i386/kernel/time-xen.c where the hz timer can be stopped on a CPU
> which has RCU callbacks pending.
> 
> This patch backports a new RCU API created to fix this problem for the
> s390 implementation of stop_hz_timer and also updates the time-xen.c
> implementation of stop_hz_timer to call the new API.
> 
> Signed-off-by: Harry Butterworth <butterwo@uk.ibm.com>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[-- Attachment #2: rcu_needs_cpu.patch --]
[-- Type: text/x-patch, Size: 2544 bytes --]

diff -r 8c64169a05d3 -r 8ef455f268b3 linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c
--- a/linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c	Thu Jun  8 08:52:04 2006
+++ b/linux-2.6-xen-sparse/arch/i386/kernel/time-xen.c	Fri Jun  9 19:47:18 2006
@@ -978,12 +978,19 @@
 	unsigned int cpu = smp_processor_id();
 	unsigned long j;
 
-	/* We must do this /before/ checking rcu_pending(). */
 	cpu_set(cpu, nohz_cpu_mask);
+
+	/* See matching smp_mb in rcu_start_batch in rcupdate.c.  These mbs  */
+	/* ensure that if __rcu_pending (nested in rcu_needs_cpu) fetches a  */
+	/* value of rcp->cur that matches rdp->quiescbatch and allows us to  */
+	/* stop the hz timer then the cpumasks created for subsequent values */
+	/* of cur in rcu_start_batch are guaranteed to pick up the updated   */
+	/* nohz_cpu_mask and so will not depend on this cpu.                 */
+
 	smp_mb();
 
 	/* Leave ourselves in 'tick mode' if rcu or softirq pending. */
-	if (rcu_pending(cpu) || local_softirq_pending()) {
+	if (rcu_needs_cpu(cpu) || local_softirq_pending()) {
 		cpu_clear(cpu, nohz_cpu_mask);
 		j = jiffies + 1;
 	} else {
diff -r 8c64169a05d3 -r 8ef455f268b3 patches/linux-2.6.16.13/rcu_needs_cpu.patch
--- /dev/null	Thu Jun  8 08:52:04 2006
+++ b/patches/linux-2.6.16.13/rcu_needs_cpu.patch	Fri Jun  9 19:47:18 2006
@@ -0,0 +1,33 @@
+--- ../pristine-linux-2.6.16.13/kernel/rcupdate.c	2006-05-02 22:38:44.000000000 +0100
++++ ./kernel/rcupdate.c	2006-06-09 20:27:45.000000000 +0100
+@@ -485,6 +485,20 @@ int rcu_pending(int cpu)
+ 		__rcu_pending(&rcu_bh_ctrlblk, &per_cpu(rcu_bh_data, cpu));
+ }
+ 
++/*
++ * Check to see if any future RCU-related work will need to be done
++ * by the current CPU, even if none need be done immediately, returning
++ * 1 if so.  This function is part of the RCU implementation; it is -not-
++ * an exported member of the RCU API.
++ */
++int rcu_needs_cpu(int cpu)
++{
++	struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
++	struct rcu_data *rdp_bh = &per_cpu(rcu_bh_data, cpu);
++
++	return (!!rdp->curlist || !!rdp_bh->curlist || rcu_pending(cpu));
++}
++
+ void rcu_check_callbacks(int cpu, int user)
+ {
+ 	if (user || 
+--- ../pristine-linux-2.6.16.13/include/linux/rcupdate.h	2006-05-02 22:38:44.000000000 +0100
++++ ./include/linux/rcupdate.h	2006-06-09 20:28:57.000000000 +0100
+@@ -134,6 +134,7 @@ static inline void rcu_bh_qsctr_inc(int 
+ }
+ 
+ extern int rcu_pending(int cpu);
++extern int rcu_needs_cpu(int cpu);
+ 
+ /**
+  * rcu_read_lock - mark the beginning of an RCU read-side critical section.

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] backport rcu_needs_cpu and call it from stop_hz_timer

[Keir Fraser]

On 12 Jun 2006, at 12:57, Harry Butterworth wrote:

> I have now tested this against xen-testing 9735.  Without the patch, 
> and
> with a domU kernel configured with CONFIG_DEBUG_SOFTLOCKUP disabled
> (this debug code masks the problem), and smp and cpu hotplug enabled, a
> script which does lots of cpu hotplug/removal will hang (cpu
> hotplug/removal stresses the RCU synchronize_sched function).  With the
> patch, the script doing cpu hotplug/removal runs for as long as I have
> let it so far (thousands of hotplugs).

Okay, the patch looks good to me. I'll apply to our trees. Thanks.

  -- Keir