[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Jeremy Fit » Fri, 04 Aug 2006 09:30:07


dd "always lock'd" implementations of set_bit, clear_bit and
change_bit and the corresponding test_and_ functions. Also add
"always lock'd" implementation of cmpxchg. These give guaranteed
strong synchronisation and are required for non-SMP kernels running on
an SMP hypervisor.

Signed-off-by: Ian Pratt < XXXX@XXXXX.COM >
Signed-off-by: Christian Limpach < XXXX@XXXXX.COM >
Signed-off-by: Chris Wright < XXXX@XXXXX.COM >
Signed-off-by: Jeremy Fitzhardinge < XXXX@XXXXX.COM >
Cc: Christoph Lameter < XXXX@XXXXX.COM >

---
include/asm-i386/sync_bitops.h | 156 ++++++++++++++++++++++++++++++++++++++++
include/asm-i386/system.h | 36 +++++++++
2 files changed, 192 insertions(+)


===================================================================
--- a/include/asm-i386/system.h
+++ b/include/asm-i386/system.h
@@ -261,6 +261,9 @@ static inline unsigned long __xchg(unsig
#define cmpxchg(ptr,o,n)\
((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)(o),\
(unsigned long)(n),sizeof(*(ptr))))
+#define sync_cmpxchg(ptr,o,n)\
+ ((__typeof__(*(ptr)))__sync_cmpxchg((ptr),(unsigned long)(o),\
+ (unsigned long)(n),sizeof(*(ptr))))
#endif

static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
@@ -282,6 +285,39 @@ static inline unsigned long __cmpxchg(vo
return prev;
case 4:
__asm__ __volatile__(LOCK_PREFIX "cmpxchgl %1,%2"
+ : "=a"(prev)
+ : "r"(new), "m"(*__xg(ptr)), "0"(old)
+ : "memory");
+ return prev;
+ }
+ return old;
+}
+
+/*
+ * Always use locked operations when touching memory shared with a
+ * hypervisor, since the system may be SMP even if the guest kernel
+ * isn't.
+ */
+static inline unsigned long __sync_cmpxchg(volatile void *ptr,
+ unsigned long old,
+ unsigned long new, int size)
+{
+ unsigned long prev;
+ switch (size) {
+ case 1:
+ __asm__ __volatile__("lock; cmpxchgb %b1,%2"
+ : "=a"(prev)
+ : "q"(new), "m"(*__xg(ptr)), "0"(old)
+ : "memory");
+ return prev;
+ case 2:
+ __asm__ __volatile__("lock; cmpxchgw %w1,%2"
+ : "=a"(prev)
+ : "r"(new), "m"(*__xg(ptr)), "0"(old)
+ : "memory");
+ return prev;
+ case 4:
+ __asm__ __volatile__("lock; cmpxchgl %1,%2"
: "=a"(prev)
: "r"(new), "m"(*__xg(ptr)), "0"(old)
: "memory");
===================================================================
--- /dev/null
+++ b/include/asm-i386/sync_bitops.h
@@ -0,0 +1,156 @@
+#ifndef _I386_SYNC_BITOPS_H
+#define _I386_SYNC_BITOPS_H
+
+/*
+ * Copyright 1992, Linus Torvalds.
+ */
+
+/*
+ * These have to be done with inline assembly: that way the bit-setting
+ * is guaranteed to be atomic. All bit operations return 0 if the bit
+ * was cleared before the operation and != 0 if it was not.
+ *
+ * bit 0 is the LSB of addr; bit 32 is the LSB of (addr+1).
+ */
+
+#define ADDR (*(volatile long *) addr)
+
+/**
+ * sync_set_bit - Atomically set a bit in memory
+ * @nr: the bit to set
+ * @addr: the address to start counting from
+ *
+ * This function is atomic and may not be reordered. See __set_bit()
+ * if you do not require the atomic guarantees.
+ *
+ * Note: there are no guarantees that this function will not be reordered
+ * on non x86 architectures, so if you are writting portable code,
+ * make sure not to rely on its reordering guarantees.
+ *
+ * Note
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 11:50:07


I don't think it's that big an issue because most architectures either
use always locked bitops already or don't need them because they don't do
SMP.

So it will be fine with just a asm-generic header that defines them
to the normal bitops. Not much burden.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/

 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 13:00:12


I think it's a bad idea. We don't want lots of architecture ifdefs
in some Xen specific file

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 13:50:05


It's cleaner to just do it in the generic code.

I think for most architectures it is only a one liner anyways if
done properly.

-Andi

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 13:50:07


i386/x86-64

They could do a single line #include for asm-generic that defines them
to the normal bitops.



The Xen driver will be "regular" kernel code.


Well there might be reasons someone else uses this in the future too.
It's also not exactly Linux style - normally we try to add generic
facilities.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 14:30:11


Yes, but in general when a driver that runs on multiple architectures
(including IA64 btw) needs something architecture specific we usually
add it to asm, not add ifdefs.


e.g. for other hypervisors or possibly for special hardware access
(e.g. I could imagine it being used for some kind of cluster interconnect)
I remember Alan was using a similar hack in his EDAC drivers because
it was the only way to clear ECC errors.


Well we have our share of weird hacks for IA64 too in generic code.

Just adding a single line #include for a wrapper asm-generic surely isn't
a undue burden for the other architectures, and it will save some
mess in the Xen drivers.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 14:40:09


Because the Xen drivers will run on a couple of architectures, including
IA64 and PPC.

If IA64 or PPC didn't implement at least wrappers for the sync ops
then they would all need special ifdefs to handle this.


It's a huge performance difference.


Ok it could be put into a separate file (although with a neutral name)

But you would still need to add that to IA64, PPC etc. too, so it
would only avoid adding a single to the other architectures.
-Andi


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Fri, 04 Aug 2006 15:10:05


If IA64 and PPC64 wouldn't have xen-bitops.h (which you seem to argue
for) then they would need ifdefs.


You mean into asm-generic/bitops.h? Then it would need ifdefs
to handle the i386/x86-64 case.

-Andi

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Chris Wrig » Sat, 05 Aug 2006 02:20:07


While Xen is the primary user, it is a misnomer to call it xen-bitops.
These are simply always locked, hence the sync-bitops name. Also,
there's a use of sync_cmpxchg, and cmpxchg is not in bitops.h.

As for the mechanism, it is manual. Arch specific header includes
asm-generic one.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[patch 2/8] Implement always-locked bit ops, for memory shared with an SMP hypervisor.

Post by Andi Klee » Sat, 05 Aug 2006 09:50:07


AFAIK not. If you want generic you need a proxy include file.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/