Yang, Xiaowei
2009-Sep-07 07:40 UTC
[Xen-devel] [PATCH] Fix memory order issue inside pv spinlock
barrier() can''t prevent reads after it not being reordered with older
writes to
different locates before it. Because of it, I can''t bring up > 4 HVM
guests on
one SMP machine. Use mb() instead.
Signed-off-by: Yang Xiaowei <xiaowei.yang@intel.com>
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 5601506..9dee5f8 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -324,7 +325,7 @@ static void xen_spin_unlock(struct raw_spinlock *lock)
xl->lock = 0; /* release lock */
/* make sure unlock happens before kick */
- barrier();
+ mb();
if (unlikely(xl->spinners))
xen_spin_unlock_slow(xl);
Thanks,
Xiaowei
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2009-Sep-08 21:59 UTC
[Xen-devel] Re: [PATCH] Fix memory order issue inside pv spinlock
On 09/07/09 00:40, Yang, Xiaowei wrote:> barrier() can''t prevent reads after it not being reordered with older > writes to different locates before it. Because of it, I can''t bring up > > 4 HVM guests on one SMP machine. Use mb() instead.Which read is happening too early? Is it "xl->spinners"? How does it fail? Thanks, J> > Signed-off-by: Yang Xiaowei <xiaowei.yang@intel.com> > > diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c > index 5601506..9dee5f8 100644 > --- a/arch/x86/xen/spinlock.c > +++ b/arch/x86/xen/spinlock.c > @@ -324,7 +325,7 @@ static void xen_spin_unlock(struct raw_spinlock > *lock) > xl->lock = 0; /* release lock */ > > /* make sure unlock happens before kick */ > - barrier(); > + mb(); > > if (unlikely(xl->spinners)) > xen_spin_unlock_slow(xl); > > Thanks, > Xiaowei >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yang, Xiaowei
2009-Sep-09 16:30 UTC
[Xen-devel] Re: [PATCH] Fix memory order issue inside pv spinlock
Jeremy Fitzhardinge wrote:> On 09/07/09 00:40, Yang, Xiaowei wrote: >> barrier() can''t prevent reads after it not being reordered with older >> writes to different locates before it. Because of it, I can''t bring up >>> 4 HVM guests on one SMP machine. Use mb() instead. > > Which read is happening too early? Is it "xl->spinners"? How does it fail?Yes. If read of xl->spinners happens earlier than write 0 to xl->lock, notifications to wake up other spinners can be omitted incorrectly, resulting in others polling indefinitely (because of poll evtchn not pending) with the lock is uncontended. Thanks, xiaowei> > Thanks, > J > >> Signed-off-by: Yang Xiaowei <xiaowei.yang@intel.com> >> >> diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c >> index 5601506..9dee5f8 100644 >> --- a/arch/x86/xen/spinlock.c >> +++ b/arch/x86/xen/spinlock.c >> @@ -324,7 +325,7 @@ static void xen_spin_unlock(struct raw_spinlock >> *lock) >> xl->lock = 0; /* release lock */ >> >> /* make sure unlock happens before kick */ >> - barrier(); >> + mb(); >> >> if (unlikely(xl->spinners)) >> xen_spin_unlock_slow(xl); >> >> Thanks, >> Xiaowei >> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2009-Sep-09 19:02 UTC
[Xen-devel] Re: [PATCH] Fix memory order issue inside pv spinlock
On 09/09/09 09:30, Yang, Xiaowei wrote:> Jeremy Fitzhardinge wrote: >> On 09/07/09 00:40, Yang, Xiaowei wrote: >>> barrier() can''t prevent reads after it not being reordered with older >>> writes to different locates before it. Because of it, I can''t bring up >>>> 4 HVM guests on one SMP machine. Use mb() instead. >> >> Which read is happening too early? Is it "xl->spinners"? How does >> it fail? > > Yes. If read of xl->spinners happens earlier than write 0 to xl->lock, > notifications to wake up other spinners can be omitted incorrectly, > resulting in others polling indefinitely (because of poll evtchn not > pending) with the lock is uncontended.OK. And the CPU only guarantees that, without explicit barriers, write-read ordering is only maintained between accesses to the same memory location, not separate locations? J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yang, Xiaowei
2009-Sep-10 01:41 UTC
[Xen-devel] Re: [PATCH] Fix memory order issue inside pv spinlock
Jeremy Fitzhardinge wrote:> On 09/09/09 09:30, Yang, Xiaowei wrote: >> Jeremy Fitzhardinge wrote: >>> On 09/07/09 00:40, Yang, Xiaowei wrote: >>>> barrier() can''t prevent reads after it not being reordered with older >>>> writes to different locates before it. Because of it, I can''t bring up >>>>> 4 HVM guests on one SMP machine. Use mb() instead. >>> Which read is happening too early? Is it "xl->spinners"? How does >>> it fail? >> Yes. If read of xl->spinners happens earlier than write 0 to xl->lock, >> notifications to wake up other spinners can be omitted incorrectly, >> resulting in others polling indefinitely (because of poll evtchn not >> pending) with the lock is uncontended. > > OK. And the CPU only guarantees that, without explicit barriers, > write-read ordering is only maintained between accesses to the same > memory location, not separate locations? >Exactly! For more details please refer to Chapter 8.2 of Intel SDM 3A. Thanks, Xiaowei _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2009-Sep-10 01:50 UTC
[Xen-devel] Re: [PATCH] Fix memory order issue inside pv spinlock
On 09/09/09 18:41, Yang, Xiaowei wrote:> Jeremy Fitzhardinge wrote: > >> On 09/09/09 09:30, Yang, Xiaowei wrote: >>> Jeremy Fitzhardinge wrote: >>>> On 09/07/09 00:40, Yang, Xiaowei wrote: >>>>> barrier() can''t prevent reads after it not being reordered with older >>>>> writes to different locates before it. Because of it, I can''t >>>>> bring up >>>>>> 4 HVM guests on one SMP machine. Use mb() instead. >>>> Which read is happening too early? Is it "xl->spinners"? How does >>>> it fail? >>> Yes. If read of xl->spinners happens earlier than write 0 to xl->lock, >>> notifications to wake up other spinners can be omitted incorrectly, >>> resulting in others polling indefinitely (because of poll evtchn not >>> pending) with the lock is uncontended. >> >> OK. And the CPU only guarantees that, without explicit barriers, >> write-read ordering is only maintained between accesses to the same >> memory location, not separate locations? >> > > Exactly! For more details please refer to Chapter 8.2 of Intel SDM 3A.Thanks. I just submitted both fixes to upstream. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel