[RFC] change non-atomic bitops method
From: Uwe Kleine-König <hidden>
Date: 2015-02-02 19:32:01
Also in:
linux-arch, lkml
On Mon, Feb 02, 2015 at 11:55:03AM +0800, Wang, Yalin wrote:
quoted hunk ↗ jump to hunk
This patch change non-atomic bitops, add a if() condition to test it, before set/clear the bit. so that we don't need dirty the cache line, if this bit have been set or clear. On SMP system, dirty cache line will need invalidate other processors cache line, this will have some impact on SMP systems. Signed-off-by: Yalin Wang <redacted> --- include/asm-generic/bitops/non-atomic.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)diff --git a/include/asm-generic/bitops/non-atomic.h b/include/asm-generic/bitops/non-atomic.h index 697cc2b..e4ef18a 100644 --- a/include/asm-generic/bitops/non-atomic.h +++ b/include/asm-generic/bitops/non-atomic.h@@ -17,7 +17,9 @@ static inline void __set_bit(int nr, volatile unsigned long *addr) unsigned long mask = BIT_MASK(nr); unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - *p |= mask; + if ((*p & mask) == 0) + *p |= mask;
Care to fix the double space here while touching the code? I think the more natural check here is: if ((~*p & mask) != 0) *p |= mask; Might be a matter of taste, but this check is equivalent to *p != (*p | mask) which is what you really want to test for. (Your check only has this property for values of mask that have a single bit set, which is ok here of course.)
quoted hunk ↗ jump to hunk
+ } static inline void __clear_bit(int nr, volatile unsigned long *addr)@@ -25,7 +27,8 @@ static inline void __clear_bit(int nr, volatile unsigned long *addr) unsigned long mask = BIT_MASK(nr); unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - *p &= ~mask; + if ((*p & mask) != 0) + *p &= ~mask;
This is already fine.
quoted hunk ↗ jump to hunk
} /**@@ -60,7 +63,8 @@ static inline int __test_and_set_bit(int nr, volatile unsigned long *addr) unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); unsigned long old = *p; - *p = old | mask; + if ((old & mask) == 0) + *p = old | mask;
Here it would be: if ((~old & mask) != 0)
return (old & mask) != 0; }
Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |