[PATCH 1/8] i386: bitops: Update/correct comments

[Satyam Sharma]

From: Satyam Sharma <ssatyam@xxxxxxxxxxxxxx>

[1/8] i386: bitops: Update/correct comments

Just trying to standardize the look of comments for various functions of the
bitops API, removed some trailing whitespace here and there, give different
kernel-doc description to the atomic functions and their corresponding
unlocked variants, remove/explicitly mention what is inapplicable/applicable
to i386, add kernel-doc comments to functions that lacked them already, and
other janitorial work. Only comments touched in this patch.

Signed-off-by: Satyam Sharma <ssatyam@xxxxxxxxxxxxxx>
Cc: David Howells <dhowells@xxxxxxxxxx>
Cc: Nick Piggin <nickpiggin@xxxxxxxxxxxx>

---

 include/asm-i386/bitops.h |  123 +++++++++++++++++++++++++++++++-------------
 1 files changed, 86 insertions(+), 37 deletions(-)

diff --git a/include/asm-i386/bitops.h b/include/asm-i386/bitops.h
index a20fe98..ba8e4bb 100644
--- a/include/asm-i386/bitops.h
+++ b/include/asm-i386/bitops.h
@@ -10,10 +10,20 @@
 
 /*
  * 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.
+ * is guaranteed to be atomic. All bit test operations return an int type:
+ * 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).
+ * However, the operand itself is always of unsigned long type, so care
+ * must be taken to ensure that the int type return of bit test operations
+ * for the != 0 case does not get truncate the '1' bits. This is not an
+ * issue on i386 where sizeof(int) == sizeof(unsigned long), but we avoid
+ * this pitfall anyway by returning with all 1's or LSB set in this case.
+ *
+ * bit 0 is the LSB of addr; bit 31 is the MSB.
+ *
+ * But note that all functions that take a bit-number argument allow it
+ * to be arbitrarily large; these operations are not restricted to acting
+ * on single-dword quantities.
  */
 
 #define ADDR (*(volatile long *) addr)
@@ -23,15 +33,10 @@
  * @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.
+ * set_bit() is atomic and cannot be reordered. On the x86, this includes
+ * an implicit memory barrier.
  *
- * Note: there are no guarantees that this function will not be reordered
- * on non x86 architectures, so if you are writing 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.
+ * See __set_bit() if you do not require the atomic guarantees.
  */
 static inline void set_bit(int nr, volatile unsigned long * addr)
 {
@@ -49,6 +54,9 @@ static inline void set_bit(int nr, volatile unsigned long * addr)
  * Unlike set_bit(), this function is non-atomic and may be reordered.
  * If it's called on the same region of memory simultaneously, the effect
  * may be that only one operation succeeds.
+ *
+ * This is a fast and unlocked operation, and only suitable for callers
+ * that already implement higher-level locking to protect access.
  */
 static inline void __set_bit(int nr, volatile unsigned long * addr)
 {
@@ -59,14 +67,14 @@ static inline void __set_bit(int nr, volatile unsigned long * addr)
 }
 
 /**
- * clear_bit - Clears a bit in memory
+ * clear_bit - Atomically clear a bit in memory
  * @nr: Bit to clear
  * @addr: Address to start counting from
  *
- * 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.
+ * clear_bit() is atomic and cannot be reordered. On the x86, this includes
+ * an implicit memory barrier.
+ *
+ * See __clear_bit() if you do not require the atomic guarantees.
  */
 static inline void clear_bit(int nr, volatile unsigned long * addr)
 {
@@ -76,6 +84,18 @@ static inline void clear_bit(int nr, volatile unsigned long * addr)
 		:"Ir" (nr));
 }
 
+/**
+ * __clear_bit - Clear a bit in memory
+ * @nr: Bit to clear
+ * @addr: Address to start counting from
+ *
+ * Unlike clear_bit(), this function is non-atomic and may be reordered.
+ * It it's called on the same region of memory simultaneously, the effect
+ * may be that only one operation succeeds.
+ *
+ * This is a fast and unlocked operation, and only suitable for callers
+ * that already implement higher-level locking to protect access.
+ */
 static inline void __clear_bit(int nr, volatile unsigned long * addr)
 {
 	__asm__ __volatile__(
@@ -83,6 +103,11 @@ static inline void __clear_bit(int nr, volatile unsigned long * addr)
 		:"+m" (ADDR)
 		:"Ir" (nr));
 }
+
+/*
+ * Bit operations are already serializing on x86.
+ * These must still be defined here for API completeness.
+ */
 #define smp_mb__before_clear_bit()	barrier()
 #define smp_mb__after_clear_bit()	barrier()
 
@@ -94,6 +119,9 @@ static inline void __clear_bit(int nr, volatile unsigned long * addr)
  * Unlike change_bit(), this function is non-atomic and may be reordered.
  * If it's called on the same region of memory simultaneously, the effect
  * may be that only one operation succeeds.
+ *
+ * This is a fast and unlocked operation, and only suitable for callers
+ * that already implement higher-level locking to protect access.
  */
 static inline void __change_bit(int nr, volatile unsigned long * addr)
 {
@@ -104,14 +132,14 @@ static inline void __change_bit(int nr, volatile unsigned long * addr)
 }
 
 /**
- * change_bit - Toggle a bit in memory
+ * change_bit - Atomically toggle a bit in memory
  * @nr: Bit to change
  * @addr: Address to start counting from
  *
- * 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.
+ * change_bit() is atomic and cannot be reordered. On the x86, this includes
+ * an implicit memory barrier.
+ *
+ * See __change_bit() if you do not require the atomic guarantees.
  */
 static inline void change_bit(int nr, volatile unsigned long * addr)
 {
@@ -122,13 +150,14 @@ static inline void change_bit(int nr, volatile unsigned long * addr)
 }
 
 /**
- * test_and_set_bit - Set a bit and return its old value
+ * test_and_set_bit - Atomically set a bit and return its old value
  * @nr: Bit to set
  * @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.
+ * test_and_set_bit() is atomic and cannot be reordered. On the x86, this
+ * includes an implicit memory barrier.
+ *
+ * See __test_and_set_bit() if you do not require the atomic guarantees.
  */
 static inline int test_and_set_bit(int nr, volatile unsigned long * addr)
 {
@@ -146,9 +175,12 @@ static inline int test_and_set_bit(int nr, volatile unsigned long * addr)
  * @nr: Bit to set
  * @addr: Address to count from
  *
- * This operation is non-atomic and can be reordered.  
+ * Unlike test_and_set_bit(), this function is non-atomic and may be reordered.
  * If two examples of this operation race, one can appear to succeed
- * but actually fail.  You must protect multiple accesses with a lock.
+ * but actually fail.
+ *
+ * This is a fast and unlocked operation, and only suitable for callers
+ * that already implement higher-level locking to protect access.
  */
 static inline int __test_and_set_bit(int nr, volatile unsigned long * addr)
 {
@@ -162,13 +194,14 @@ static inline int __test_and_set_bit(int nr, volatile unsigned long * addr)
 }
 
 /**
- * test_and_clear_bit - Clear a bit and return its old value
+ * test_and_clear_bit - Atomically clear a bit and return its old value
  * @nr: Bit to clear
  * @addr: Address to count from
  *
- * This operation is atomic and cannot be reordered.
- * It can be reorderdered on other architectures other than x86.
- * It also implies a memory barrier.
+ * test_and_clear_bit() is atomic and cannot be reordered. On the x86, this
+ * includes an implicit memory barrier.
+ *
+ * See __test_and_clear_bit() if you do not require the atomic guarantees.
  */
 static inline int test_and_clear_bit(int nr, volatile unsigned long * addr)
 {
@@ -186,9 +219,12 @@ static inline int test_and_clear_bit(int nr, volatile unsigned long * addr)
  * @nr: Bit to clear
  * @addr: Address to count from
  *
- * This operation is non-atomic and can be reordered.  
+ * Unlike test_and_clear_bit(), this function is non-atomic and may be reordered.
  * If two examples of this operation race, one can appear to succeed
- * but actually fail.  You must protect multiple accesses with a lock.
+ * but actually fail.
+ *
+ * This is a fast and unlocked operation, and only suitable for callers
+ * that already implement higher-level locking to protect access.
  */
 static inline int __test_and_clear_bit(int nr, volatile unsigned long *addr)
 {
@@ -201,7 +237,18 @@ static inline int __test_and_clear_bit(int nr, volatile unsigned long *addr)
 	return oldbit;
 }
 
-/* WARNING: non atomic and it can be reordered! */
+/**
+ * __test_and_change_bit - Change a bit and return its old value
+ * @nr: Bit to clear
+ * @addr: Address to count from
+ *
+ * Unlike test_and_change_bit(), this function is non-atomic and may be reordered.
+ * If two examples of this operation race, one can appear to succeed
+ * but actually fail.
+ *
+ * This is a fast and unlocked operation, and only suitable for callers
+ * that already implement higher-level locking to protect access.
+ */
 static inline int __test_and_change_bit(int nr, volatile unsigned long *addr)
 {
 	int oldbit;
@@ -214,12 +261,14 @@ static inline int __test_and_change_bit(int nr, volatile unsigned long *addr)
 }
 
 /**
- * test_and_change_bit - Change a bit and return its old value
+ * test_and_change_bit - Atomically change a bit and return its old value
  * @nr: Bit to change
  * @addr: Address to count from
  *
- * This operation is atomic and cannot be reordered.  
- * It also implies a memory barrier.
+ * test_and_change_bit() is atomic and cannot be reordered. On the x86, this
+ * includes an implicit memory barrier.
+ *
+ * See __test_and_change_bit() if you do not require the atomic guarantees.
  */
 static inline int test_and_change_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/