[PATCH] linux-2.6.18/evtchn: clear secondary CPUs' cpu_evtchn_mask[] after restore

[PATCH] linux-2.6.18/evtchn: clear secondary CPUs' cpu_evtchn_mask[] after restore

[Jan Beulich]

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

To bind all event channels to CPU#0, it is not sufficient to set all of
its cpu_evtchn_mask[] bits; all other CPUs also need to get their bits
cleared. Otherwise, evtchn_do_upcall() will start handling interrupts
on CPUs they're not intended to run on, which can be particularly bad
for per-CPU ones.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- a/drivers/xen/core/evtchn.c
+++ b/drivers/xen/core/evtchn.c
@@ -161,7 +161,8 @@ static void init_evtchn_cpu_bindings(voi
 		set_native_irq_info(i, cpumask_of_cpu(0));
 
 	memset(cpu_evtchn, 0, sizeof(cpu_evtchn));
-	memset(cpu_evtchn_mask[0], ~0, sizeof(cpu_evtchn_mask[0]));
+	for_each_possible_cpu(i)
+		memset(cpu_evtchn_mask[i], -!i, sizeof(cpu_evtchn_mask[i]));
 }
 
 static inline unsigned int cpu_from_evtchn(unsigned int evtchn)




[-- Attachment #2: xen-evtchn-cpu-clear.patch --]
[-- Type: text/plain, Size: 813 bytes --]

To bind all event channels to CPU#0, it is not sufficient to set all of
its cpu_evtchn_mask[] bits; all other CPUs also need to get their bits
cleared. Otherwise, evtchn_do_upcall() will start handling interrupts
on CPUs they're not intended to run on, which can be particularly bad
for per-CPU ones.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- a/drivers/xen/core/evtchn.c
+++ b/drivers/xen/core/evtchn.c
@@ -161,7 +161,8 @@ static void init_evtchn_cpu_bindings(voi
 		set_native_irq_info(i, cpumask_of_cpu(0));
 
 	memset(cpu_evtchn, 0, sizeof(cpu_evtchn));
-	memset(cpu_evtchn_mask[0], ~0, sizeof(cpu_evtchn_mask[0]));
+	for_each_possible_cpu(i)
+		memset(cpu_evtchn_mask[i], -!i, sizeof(cpu_evtchn_mask[i]));
 }
 
 static inline unsigned int cpu_from_evtchn(unsigned int evtchn)

[-- 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] linux-2.6.18/evtchn: clear secondary CPUs' cpu_evtchn_mask[] after restore

[Jan Beulich]

>>> On 15.11.10 at 17:56, "Jan Beulich" <JBeulich@novell.com> wrote:
> To bind all event channels to CPU#0, it is not sufficient to set all of
> its cpu_evtchn_mask[] bits; all other CPUs also need to get their bits
> cleared. Otherwise, evtchn_do_upcall() will start handling interrupts
> on CPUs they're not intended to run on, which can be particularly bad
> for per-CPU ones.

I believe this also needs to be fixed in the pv-ops sources.

Furthermore, while debugging this, I looked at pv-ops'
implementation of bind_evtchn_to_cpu(), and I suspect that the
use of __clear_bit() and __set_bit() isn't correct there - looking
over the call sites there doesn't appear to be a common serializing
spinlock. If so the underscore-less variants ought to be used.

Jan

Re: [PATCH] linux-2.6.18/evtchn: clear secondary CPUs' cpu_evtchn_mask[] after restore

[Jeremy Fitzhardinge]

On 11/15/2010 08:56 AM, Jan Beulich wrote:
> To bind all event channels to CPU#0, it is not sufficient to set all of
> its cpu_evtchn_mask[] bits; all other CPUs also need to get their bits
> cleared. Otherwise, evtchn_do_upcall() will start handling interrupts
> on CPUs they're not intended to run on, which can be particularly bad
> for per-CPU ones.

Yes, that would be awkward.

> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> --- a/drivers/xen/core/evtchn.c
> +++ b/drivers/xen/core/evtchn.c
> @@ -161,7 +161,8 @@ static void init_evtchn_cpu_bindings(voi
>  		set_native_irq_info(i, cpumask_of_cpu(0));
>  
>  	memset(cpu_evtchn, 0, sizeof(cpu_evtchn));
> -	memset(cpu_evtchn_mask[0], ~0, sizeof(cpu_evtchn_mask[0]));
> +	for_each_possible_cpu(i)
> +		memset(cpu_evtchn_mask[i], -!i, sizeof(cpu_evtchn_mask[i]));

-!i?  Really?  Golf-clap and all, but I think something a bit more
explicit would be better ;)

    J

Re: [PATCH] linux-2.6.18/evtchn: clear secondary CPUs' cpu_evtchn_mask[] after restore

[Keir Fraser]

On 16/11/2010 01:28, "Jeremy Fitzhardinge" <jeremy@goop.org> wrote:

> On 11/15/2010 08:56 AM, Jan Beulich wrote:

>> memset(cpu_evtchn, 0, sizeof(cpu_evtchn));
>> - memset(cpu_evtchn_mask[0], ~0, sizeof(cpu_evtchn_mask[0]));
>> + for_each_possible_cpu(i)
>> +  memset(cpu_evtchn_mask[i], -!i, sizeof(cpu_evtchn_mask[i]));
> 
> -!i?  Really?  Golf-clap and all, but I think something a bit more
> explicit would be better ;)

I'm going to clarify that bit when I apply to 2.6.18. :-)

 -- Keir

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