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

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)





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

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


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

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

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

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


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