|
xen-devel
Re: [Xen-devel][PATCH] add memory barriers to IPI related code
After discussions with Kevin and other folks at Intel, and some
experimentation
here, I am convinced that this patch is not needed. The reason for my hangs
was that I had the initial cs for x2apic, but not the second one, which
had the
crucial mb() in the send_IPI_mask_x2apic. The mb()s I added to fix the
problem were more than the minimum required. Indeed it appears that taking
an interrupt implies a mb() on ia32. Thanks to everyone who helped me
with this,
especially Kevin Tian.
-Dave
Keir Fraser wrote:
I'd like some advice from Intel on the need for all these barriers. I'm
pretty sure that interrupts are supposed to imply memory barriers on the x86
architecture. It's not beyond the realm of possibility that you're running
on a buggy SDP.
You certainly should not need barriers on the sending side, since (apart
from x2apic, which already has an explicit mb()) the send is performed by a
memory write, and stores are guaranteed to occur in-order on x86
architecture. So e.g., call_data must be visible before the IPI occurs in
smp_call_function().
And on the receiving side, it's hard to see how a read would appear to occur
before the interrupt which transfers control to the IPI handler. Unless
there's really weird cache synchronisation magic going on.
-- Keir
On 23/10/08 19:23, "Dave Winchell" <dwinchell@xxxxxxxxxxxxxxx> wrote:
Hi Keir,
We have been seeing some hangs in call function and
tlb shootdown. In our case the machine was an Intel Tylersburg.
One failure mode is this early return in smp_call_function_interrupt:
if ( !cpu_isset(smp_processor_id(), call_data->selected) )
return;
Without a mb(), the target processor takes this path as it doesn't yet
see the update
to call_data->selected from the initiating processor. Then the initiator
waits for ever
to get an acknowledgment from this processor.
On the initiating side, I added the mb()'s to the send_IPI functions,
x2apic already
having one.
On the target side, I added the mb()'s in the individual target functions.
Regards,
Dave
diff -r 4129f0f2f2ba xen/arch/x86/smp.c
--- a/xen/arch/x86/smp.c Fri Oct 17 14:15:37 2008 +0100
+++ b/xen/arch/x86/smp.c Mon Oct 20 10:53:24 2008 -0400
@@ -93,6 +93,8 @@ void send_IPI_mask_flat(cpumask_t cpumas
/* An IPI with no target generates a send accept error from P5/P6 APICs.
*/
WARN_ON(mask == 0);
+ mb();
+
local_irq_save(flags);
/*
@@ -123,6 +125,8 @@ void send_IPI_mask_phys(cpumask_t mask,
{
unsigned long cfg, flags;
unsigned int query_cpu;
+
+ mb();
local_irq_save(flags);
@@ -160,6 +164,7 @@ static unsigned int flush_flags;
fastcall void smp_invalidate_interrupt(void)
{
+ mb();
ack_APIC_irq();
perfc_incr(ipis);
irq_enter();
@@ -337,17 +342,22 @@ void smp_send_stop(void)
fastcall void smp_event_check_interrupt(struct cpu_user_regs *regs)
{
+ mb();
ack_APIC_irq();
perfc_incr(ipis);
}
fastcall void smp_call_function_interrupt(struct cpu_user_regs *regs)
{
- void (*func)(void *info) = call_data->func;
- void *info = call_data->info;
-
+ void (*func)(void *info);
+ void *info;
+
+ mb();
ack_APIC_irq();
perfc_incr(ipis);
+
+ func = call_data->func;
+ info = call_data->info;
if ( !cpu_isset(smp_processor_id(), call_data->selected) )
return;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
Home