Re: [PATCH 01/20] x86/ticketlock: clean up types and accessors

2010-11-15 Thread Américo Wang 
On Wed, Nov 03, 2010 at 10:59:42AM -0400, Jeremy Fitzhardinge wrote:
...
 static __always_inline void __ticket_spin_lock(arch_spinlock_t *lock)
 {
-  int inc = 0x0001;
-  int tmp;
+  unsigned inc = 1  TICKET_SHIFT;
+  unsigned tmp;

Please don't use old implicit-int.

 
-  return (((tmp  TICKET_SHIFT) - tmp)  ((1  TICKET_SHIFT) - 1))  1;
+  return ((tmp.tail - tmp.head)  TICKET_MASK)  1;


There is a type promotion here.


 }
 
 #ifndef CONFIG_PARAVIRT_SPINLOCKS
diff --git a/arch/x86/include/asm/spinlock_types.h 
b/arch/x86/include/asm/spinlock_types.h
index dcb48b2..4582640 100644
--- a/arch/x86/include/asm/spinlock_types.h
+++ b/arch/x86/include/asm/spinlock_types.h
@@ -5,11 +5,27 @@
 # error please don't include this file directly
 #endif
 
+#include linux/types.h
+
+#if (CONFIG_NR_CPUS  256)
+typedef u8  __ticket_t;
+#else
+typedef u16 __ticket_t;
+#endif
+
+#define TICKET_SHIFT  (sizeof(__ticket_t) * 8)
+#define TICKET_MASK   ((1  TICKET_SHIFT) - 1)


So here you may need to cast the result to __ticket_t.


Thanks.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH 01/20] x86/ticketlock: clean up types and accessors

2010-11-15 Thread Jeremy Fitzhardinge 
On 11/13/2010 01:57 AM, Américo Wang wrote:
 On Wed, Nov 03, 2010 at 10:59:42AM -0400, Jeremy Fitzhardinge wrote:
 ...
 static __always_inline void __ticket_spin_lock(arch_spinlock_t *lock)
 {
 -int inc = 0x0001;
 -int tmp;
 +unsigned inc = 1  TICKET_SHIFT;
 +unsigned tmp;
 Please don't use old implicit-int.

I don't mind much either way, but I don't think I've seen anyone worry
about this before.

 -return (((tmp  TICKET_SHIFT) - tmp)  ((1  TICKET_SHIFT) - 1))  1;
 +return ((tmp.tail - tmp.head)  TICKET_MASK)  1;

 There is a type promotion here.

...

 }

 #ifndef CONFIG_PARAVIRT_SPINLOCKS
 diff --git a/arch/x86/include/asm/spinlock_types.h 
 b/arch/x86/include/asm/spinlock_types.h
 index dcb48b2..4582640 100644
 --- a/arch/x86/include/asm/spinlock_types.h
 +++ b/arch/x86/include/asm/spinlock_types.h
 @@ -5,11 +5,27 @@
 # error please don't include this file directly
 #endif

 +#include linux/types.h
 +
 +#if (CONFIG_NR_CPUS  256)
 +typedef u8  __ticket_t;
 +#else
 +typedef u16 __ticket_t;
 +#endif
 +
 +#define TICKET_SHIFT(sizeof(__ticket_t) * 8)
 +#define TICKET_MASK ((1  TICKET_SHIFT) - 1)

 So here you may need to cast the result to __ticket_t.

OK.

J
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization