[PATCH] x86 bitops.h commentary on instruction reordering

[Marcelo Tosatti]

Hi, 

Back when we were discussing the need for a memory barrier in sync_page(), 
it came to me (thanks Andrea!) that the bit operations can be perfectly 
reordered on architectures other than x86. 

I think the commentary on i386 bitops.h is misleading, its worth 
to note that that these operations are not guaranteed not to be 
reordered on different architectures. 

clear_bit() already does that:

 * clear_bit() is atomic and may not be reordered.  However, it does
 * not contain a memory barrier, so if it is used for locking purposes,
 * you should call smp_mb__before_clear_bit() and/or smp_mb__after_clear_bit()
 * in order to ensure changes are visible on other processors.

What you think of the following


--- a/include/asm-i386/bitops.h.orig	2004-08-05 17:35:40.639336160 -0300
+++ b/include/asm-i386/bitops.h	2004-08-05 17:35:12.486616024 -0300
@@ -30,7 +30,12 @@
  * @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.
+ * 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 that @nr may be almost arbitrarily large; this function is not
  * restricted to acting on a single-word quantity.
  */
@@ -109,7 +114,8 @@ static inline void __change_bit(int nr, 
  * @nr: Bit to change
  * @addr: Address to start counting from
  *
- * change_bit() is atomic and may not be reordered.
+ * change_bit() is atomic and may not be reordered. It may be
+ * reordered on other architectures than x86.
  * Note that @nr may be almost arbitrarily large; this function is not
  * restricted to acting on a single-word quantity.
  */
@@ -127,6 +133,7 @@ static inline void change_bit(int nr, vo
  * @addr: Address to count from
  *
  * This operation is atomic and cannot be reordered.  
+ * It may be reordered on other architectures than x86.
  * It also implies a memory barrier.
  */
 static inline int test_and_set_bit(int nr, volatile unsigned long * addr)
@@ -165,7 +172,8 @@ static inline int __test_and_set_bit(int
  * @nr: Bit to clear
  * @addr: Address to count from
  *
- * This operation is atomic and cannot be reordered.  
+ * This operation is atomic and cannot be reordered. 
+ * It can be reorderdered on other architectures other than x86.
  * It also implies a memory barrier.
  */
 static inline int test_and_clear_bit(int nr, volatile unsigned long * addr)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/