| Message ID | 20220610113427.908751-3-alexandr.lobakin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bitops: let optimize out non-atomic bitops on compile-time constants | expand |
On Fri, Jun 10, 2022 at 01:34:23PM +0200, Alexander Lobakin wrote: > Move generic non-atomic bitops from the asm-generic header which > gets included only when there are no architecture-specific > alternatives, to a separate independent file to make them always > available. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> But see below. > Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> > --- > .../asm-generic/bitops/generic-non-atomic.h | 124 ++++++++++++++++++ > include/asm-generic/bitops/non-atomic.h | 109 ++------------- > 2 files changed, 132 insertions(+), 101 deletions(-) > create mode 100644 include/asm-generic/bitops/generic-non-atomic.h > > diff --git a/include/asm-generic/bitops/generic-non-atomic.h b/include/asm-generic/bitops/generic-non-atomic.h > new file mode 100644 > index 000000000000..808bc4469886 > --- /dev/null > +++ b/include/asm-generic/bitops/generic-non-atomic.h > @@ -0,0 +1,124 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +#ifndef __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H > +#define __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H > + > +#include <linux/bits.h> > + > +#ifndef _LINUX_BITOPS_H > +#error only <linux/bitops.h> can be included directly > +#endif > + > +/* > + * Generic definitions for bit operations, should not be used in regular code > + * directly. > + */ > + > +/** > + * generic___set_bit - Set a bit in memory > + * @nr: the bit to set > + * @addr: the address to start counting from > + * > + * 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. > + */ > +static __always_inline void > +generic___set_bit(unsigned int nr, volatile unsigned long *addr) > +{ > + unsigned long mask = BIT_MASK(nr); > + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > + > + *p |= mask; > +} > + > +static __always_inline void > +generic___clear_bit(unsigned int nr, volatile unsigned long *addr) > +{ > + unsigned long mask = BIT_MASK(nr); > + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > + > + *p &= ~mask; > +} > + > +/** > + * generic___change_bit - Toggle a bit in memory > + * @nr: the bit to change > + * @addr: the address to start counting from > + * > + * 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. > + */ > +static __always_inline > +void generic___change_bit(unsigned int nr, volatile unsigned long *addr) > +{ > + unsigned long mask = BIT_MASK(nr); > + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > + > + *p ^= mask; > +} > + > +/** > + * generic___test_and_set_bit - Set a bit and return its old value > + * @nr: Bit to set > + * @addr: Address to count from > + * > + * This operation is non-atomic and can 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. > + */ > +static __always_inline int > +generic___test_and_set_bit(unsigned int nr, volatile unsigned long *addr) > +{ > + unsigned long mask = BIT_MASK(nr); > + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > + unsigned long old = *p; > + > + *p = old | mask; > + return (old & mask) != 0; > +} > + > +/** > + * generic___test_and_clear_bit - Clear a bit and return its old value > + * @nr: Bit to clear > + * @addr: Address to count from > + * > + * This operation is non-atomic and can 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. > + */ > +static __always_inline int > +generic___test_and_clear_bit(unsigned int nr, volatile unsigned long *addr) > +{ > + unsigned long mask = BIT_MASK(nr); > + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > + unsigned long old = *p; > + > + *p = old & ~mask; > + return (old & mask) != 0; > +} > + > +/* WARNING: non atomic and it can be reordered! */ > +static __always_inline int > +generic___test_and_change_bit(unsigned int nr, volatile unsigned long *addr) > +{ > + unsigned long mask = BIT_MASK(nr); > + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > + unsigned long old = *p; > + > + *p = old ^ mask; > + return (old & mask) != 0; > +} > + > +/** > + * generic_test_bit - Determine whether a bit is set > + * @nr: bit number to test > + * @addr: Address to start counting from > + */ Shouldn't we add in this or in separate patch a big NOTE to explain that this is actually atomic and must be kept as a such? > +static __always_inline int > +generic_test_bit(unsigned int nr, const volatile unsigned long *addr) > +{ > + return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); > +} > + > +#endif /* __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H */ > diff --git a/include/asm-generic/bitops/non-atomic.h b/include/asm-generic/bitops/non-atomic.h > index 078cc68be2f1..a05bc090a6a3 100644 > --- a/include/asm-generic/bitops/non-atomic.h > +++ b/include/asm-generic/bitops/non-atomic.h > @@ -2,121 +2,28 @@ > #ifndef _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ > #define _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ > > +#include <asm-generic/bitops/generic-non-atomic.h> > #include <asm/types.h> > > -/** > - * arch___set_bit - Set a bit in memory > - * @nr: the bit to set > - * @addr: the address to start counting from > - * > - * 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. > - */ > -static __always_inline void > -arch___set_bit(unsigned int nr, volatile unsigned long *addr) > -{ > - unsigned long mask = BIT_MASK(nr); > - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > - > - *p |= mask; > -} > +#define arch___set_bit generic___set_bit > #define __set_bit arch___set_bit > > -static __always_inline void > -arch___clear_bit(unsigned int nr, volatile unsigned long *addr) > -{ > - unsigned long mask = BIT_MASK(nr); > - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > - > - *p &= ~mask; > -} > +#define arch___clear_bit generic___clear_bit > #define __clear_bit arch___clear_bit > > -/** > - * arch___change_bit - Toggle a bit in memory > - * @nr: the bit to change > - * @addr: the address to start counting from > - * > - * 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. > - */ > -static __always_inline > -void arch___change_bit(unsigned int nr, volatile unsigned long *addr) > -{ > - unsigned long mask = BIT_MASK(nr); > - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > - > - *p ^= mask; > -} > +#define arch___change_bit generic___change_bit > #define __change_bit arch___change_bit > > -/** > - * arch___test_and_set_bit - Set a bit and return its old value > - * @nr: Bit to set > - * @addr: Address to count from > - * > - * This operation is non-atomic and can 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. > - */ > -static __always_inline int > -arch___test_and_set_bit(unsigned int nr, volatile unsigned long *addr) > -{ > - unsigned long mask = BIT_MASK(nr); > - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > - unsigned long old = *p; > - > - *p = old | mask; > - return (old & mask) != 0; > -} > +#define arch___test_and_set_bit generic___test_and_set_bit > #define __test_and_set_bit arch___test_and_set_bit > > -/** > - * arch___test_and_clear_bit - Clear a bit and return its old value > - * @nr: Bit to clear > - * @addr: Address to count from > - * > - * This operation is non-atomic and can 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. > - */ > -static __always_inline int > -arch___test_and_clear_bit(unsigned int nr, volatile unsigned long *addr) > -{ > - unsigned long mask = BIT_MASK(nr); > - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > - unsigned long old = *p; > - > - *p = old & ~mask; > - return (old & mask) != 0; > -} > +#define arch___test_and_clear_bit generic___test_and_clear_bit > #define __test_and_clear_bit arch___test_and_clear_bit > > -/* WARNING: non atomic and it can be reordered! */ > -static __always_inline int > -arch___test_and_change_bit(unsigned int nr, volatile unsigned long *addr) > -{ > - unsigned long mask = BIT_MASK(nr); > - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); > - unsigned long old = *p; > - > - *p = old ^ mask; > - return (old & mask) != 0; > -} > +#define arch___test_and_change_bit generic___test_and_change_bit > #define __test_and_change_bit arch___test_and_change_bit > > -/** > - * arch_test_bit - Determine whether a bit is set > - * @nr: bit number to test > - * @addr: Address to start counting from > - */ > -static __always_inline int > -arch_test_bit(unsigned int nr, const volatile unsigned long *addr) > -{ > - return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); > -} > +#define arch_test_bit generic_test_bit > #define test_bit arch_test_bit > > #endif /* _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ */ > -- > 2.36.1 >
> > +/** > > + * generic_test_bit - Determine whether a bit is set > > + * @nr: bit number to test > > + * @addr: Address to start counting from > > + */ > > Shouldn't we add in this or in separate patch a big NOTE to explain that this > is actually atomic and must be kept as a such? "atomic" isn't really the right word. The volatile access makes sure that the compiler does the test at the point that the source code asked, and doesn't move it before/after other operations. But there is no such thing as an atomic test_bit() operation: if (test_bit(5, addr)) { /* some other CPU nukes bit 5 */ /* I know it was set when I looked, but now, could be anything */ ... } -Tony
On Fri, 10 Jun 2022 at 18:02, Luck, Tony <tony.luck@intel.com> wrote: > > > > +/** > > > + * generic_test_bit - Determine whether a bit is set > > > + * @nr: bit number to test > > > + * @addr: Address to start counting from > > > + */ > > > > Shouldn't we add in this or in separate patch a big NOTE to explain that this > > is actually atomic and must be kept as a such? > > "atomic" isn't really the right word. The volatile access makes sure that the > compiler does the test at the point that the source code asked, and doesn't > move it before/after other operations. It's listed in Documentation/atomic_bitops.txt. It is as "atomic" as READ_ONCE() or atomic_read() is. Though you are right that the "atomicity" of reading one bit is almost a given, because we can't really read half a bit. The main thing is that the compiler keeps it "atomic" and e.g. doesn't fuse the load with another or elide it completely, and then transforms the code in concurrency-unfriendly ways. Like READ_ONCE() and friends, test_bit(), unlike non-atomic bitops, may also be used to dependency-order some subsequent marked (viz. atomic) operations. > But there is no such thing as an atomic test_bit() operation: > > if (test_bit(5, addr)) { > /* some other CPU nukes bit 5 */ > > /* I know it was set when I looked, but now, could be anything */ The operation itself is atomic, because reading half a bit is impossible. Whether or not that bit is modified concurrently is a different problem. Thanks, -- Marco
From: Marco Elver <elver@google.com> Date: Fri, 10 Jun 2022 18:32:36 +0200 > On Fri, 10 Jun 2022 at 18:02, Luck, Tony <tony.luck@intel.com> wrote: > > > > > > +/** > > > > + * generic_test_bit - Determine whether a bit is set > > > > + * @nr: bit number to test > > > > + * @addr: Address to start counting from > > > > + */ > > > > > > Shouldn't we add in this or in separate patch a big NOTE to explain that this > > > is actually atomic and must be kept as a such? > > > > "atomic" isn't really the right word. The volatile access makes sure that the > > compiler does the test at the point that the source code asked, and doesn't > > move it before/after other operations. > > It's listed in Documentation/atomic_bitops.txt. Oh, so my memory was actually correct that I saw it in the docs somewhere. WDYT, should I mention this here in the code (block comment) as well that it's atomic and must not lose `volatile` as Andy suggested or it's sufficient to have it in the docs (+ it's not underscored)? > > It is as "atomic" as READ_ONCE() or atomic_read() is. Though you are > right that the "atomicity" of reading one bit is almost a given, > because we can't really read half a bit. > The main thing is that the compiler keeps it "atomic" and e.g. doesn't > fuse the load with another or elide it completely, and then transforms > the code in concurrency-unfriendly ways. > > Like READ_ONCE() and friends, test_bit(), unlike non-atomic bitops, > may also be used to dependency-order some subsequent marked (viz. > atomic) operations. > > > But there is no such thing as an atomic test_bit() operation: > > > > if (test_bit(5, addr)) { > > /* some other CPU nukes bit 5 */ > > > > /* I know it was set when I looked, but now, could be anything */ > > The operation itself is atomic, because reading half a bit is > impossible. Whether or not that bit is modified concurrently is a > different problem. > > Thanks, > -- Marco Thanks, Olek
On Mon, 13 Jun 2022 at 16:21, Alexander Lobakin <alexandr.lobakin@intel.com> wrote: > > From: Marco Elver <elver@google.com> > Date: Fri, 10 Jun 2022 18:32:36 +0200 > > > On Fri, 10 Jun 2022 at 18:02, Luck, Tony <tony.luck@intel.com> wrote: > > > > > > > > +/** > > > > > + * generic_test_bit - Determine whether a bit is set > > > > > + * @nr: bit number to test > > > > > + * @addr: Address to start counting from > > > > > + */ > > > > > > > > Shouldn't we add in this or in separate patch a big NOTE to explain that this > > > > is actually atomic and must be kept as a such? > > > > > > "atomic" isn't really the right word. The volatile access makes sure that the > > > compiler does the test at the point that the source code asked, and doesn't > > > move it before/after other operations. > > > > It's listed in Documentation/atomic_bitops.txt. > > Oh, so my memory was actually correct that I saw it in the docs > somewhere. > WDYT, should I mention this here in the code (block comment) as well > that it's atomic and must not lose `volatile` as Andy suggested or > it's sufficient to have it in the docs (+ it's not underscored)? Perhaps a quick comment in the code (not kerneldoc above) will be sufficient, with reference to Documentation/atomic_bitops.txt.
>> It's listed in Documentation/atomic_bitops.txt. > > Oh, so my memory was actually correct that I saw it in the docs > somewhere. > WDYT, should I mention this here in the code (block comment) as well > that it's atomic and must not lose `volatile` as Andy suggested or > it's sufficient to have it in the docs (+ it's not underscored)? I think a comment that the "volatile" is required to prevent re-ordering is enough. But maybe others are sufficiently clear on the meaning? I once wasted time looking for the non-atomic __test_bit() version (to use in some code that was already protected by a spin lock, so didn't need the overhead of an "atomic" version) before realizing there wasn't a non-atomic one. -Tony
From: Luck, Tony > Sent: 13 June 2022 17:27 > > >> It's listed in Documentation/atomic_bitops.txt. > > > > Oh, so my memory was actually correct that I saw it in the docs > > somewhere. > > WDYT, should I mention this here in the code (block comment) as well > > that it's atomic and must not lose `volatile` as Andy suggested or > > it's sufficient to have it in the docs (+ it's not underscored)? > > I think a comment that the "volatile" is required to prevent re-ordering > is enough. > > But maybe others are sufficiently clear on the meaning? I once wasted > time looking for the non-atomic __test_bit() version (to use in some code > that was already protected by a spin lock, so didn't need the overhead > of an "atomic" version) before realizing there wasn't a non-atomic one. Does it make any sense for 'test bit' to be atomic? I'm not even sure is needs any ordering constraints either. The result is always stale - the value can be changed by another cpu at any time. The set/clear atomic bit-ops require a RMW bus cycle - which has to be locked (or similar) to avoid corruption. The atomic 'test and set' (etc) are RMW and return a valid state. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Jun 13, 2022 at 04:33:17PM +0200, Marco Elver wrote: > On Mon, 13 Jun 2022 at 16:21, Alexander Lobakin > <alexandr.lobakin@intel.com> wrote: > > > > From: Marco Elver <elver@google.com> > > Date: Fri, 10 Jun 2022 18:32:36 +0200 > > > > > On Fri, 10 Jun 2022 at 18:02, Luck, Tony <tony.luck@intel.com> wrote: > > > > > > > > > > +/** > > > > > > + * generic_test_bit - Determine whether a bit is set > > > > > > + * @nr: bit number to test > > > > > > + * @addr: Address to start counting from > > > > > > + */ > > > > > > > > > > Shouldn't we add in this or in separate patch a big NOTE to explain that this > > > > > is actually atomic and must be kept as a such? > > > > > > > > "atomic" isn't really the right word. The volatile access makes sure that the > > > > compiler does the test at the point that the source code asked, and doesn't > > > > move it before/after other operations. > > > > > > It's listed in Documentation/atomic_bitops.txt. > > > > Oh, so my memory was actually correct that I saw it in the docs > > somewhere. > > WDYT, should I mention this here in the code (block comment) as well > > that it's atomic and must not lose `volatile` as Andy suggested or > > it's sufficient to have it in the docs (+ it's not underscored)? > > Perhaps a quick comment in the code (not kerneldoc above) will be > sufficient, with reference to Documentation/atomic_bitops.txt. If it may help, we can do: /* * Bit testing is a naturally atomic operation because bit is * a minimal quantum of information. */ #define __test_bit test_bit
On Wed, 15 Jun 2022 at 04:47, Yury Norov <yury.norov@gmail.com> wrote: > > On Mon, Jun 13, 2022 at 04:33:17PM +0200, Marco Elver wrote: > > On Mon, 13 Jun 2022 at 16:21, Alexander Lobakin > > <alexandr.lobakin@intel.com> wrote: > > > > > > From: Marco Elver <elver@google.com> > > > Date: Fri, 10 Jun 2022 18:32:36 +0200 > > > > > > > On Fri, 10 Jun 2022 at 18:02, Luck, Tony <tony.luck@intel.com> wrote: > > > > > > > > > > > > +/** > > > > > > > + * generic_test_bit - Determine whether a bit is set > > > > > > > + * @nr: bit number to test > > > > > > > + * @addr: Address to start counting from > > > > > > > + */ > > > > > > > > > > > > Shouldn't we add in this or in separate patch a big NOTE to explain that this > > > > > > is actually atomic and must be kept as a such? > > > > > > > > > > "atomic" isn't really the right word. The volatile access makes sure that the > > > > > compiler does the test at the point that the source code asked, and doesn't > > > > > move it before/after other operations. > > > > > > > > It's listed in Documentation/atomic_bitops.txt. > > > > > > Oh, so my memory was actually correct that I saw it in the docs > > > somewhere. > > > WDYT, should I mention this here in the code (block comment) as well > > > that it's atomic and must not lose `volatile` as Andy suggested or > > > it's sufficient to have it in the docs (+ it's not underscored)? > > > > Perhaps a quick comment in the code (not kerneldoc above) will be > > sufficient, with reference to Documentation/atomic_bitops.txt. > > If it may help, we can do: > > /* > * Bit testing is a naturally atomic operation because bit is > * a minimal quantum of information. > */ > #define __test_bit test_bit That's redundant and we'll end up with a random mix of both. What'd be more interesting is having a __test_bit without the volatile that allows compilers to optimize things more. But I think that also becomes mostly redundant with the optimizations that this series seeks out to do. The distinction is ever so subtle, and clever compilers *will* break concurrent code in ways that are rather hard to imagine: https://lwn.net/Articles/793253/
diff --git a/include/asm-generic/bitops/generic-non-atomic.h b/include/asm-generic/bitops/generic-non-atomic.h new file mode 100644 index 000000000000..808bc4469886 --- /dev/null +++ b/include/asm-generic/bitops/generic-non-atomic.h @@ -0,0 +1,124 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H +#define __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H + +#include <linux/bits.h> + +#ifndef _LINUX_BITOPS_H +#error only <linux/bitops.h> can be included directly +#endif + +/* + * Generic definitions for bit operations, should not be used in regular code + * directly. + */ + +/** + * generic___set_bit - Set a bit in memory + * @nr: the bit to set + * @addr: the address to start counting from + * + * 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. + */ +static __always_inline void +generic___set_bit(unsigned int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + + *p |= mask; +} + +static __always_inline void +generic___clear_bit(unsigned int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + + *p &= ~mask; +} + +/** + * generic___change_bit - Toggle a bit in memory + * @nr: the bit to change + * @addr: the address to start counting from + * + * 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. + */ +static __always_inline +void generic___change_bit(unsigned int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + + *p ^= mask; +} + +/** + * generic___test_and_set_bit - Set a bit and return its old value + * @nr: Bit to set + * @addr: Address to count from + * + * This operation is non-atomic and can 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. + */ +static __always_inline int +generic___test_and_set_bit(unsigned int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + unsigned long old = *p; + + *p = old | mask; + return (old & mask) != 0; +} + +/** + * generic___test_and_clear_bit - Clear a bit and return its old value + * @nr: Bit to clear + * @addr: Address to count from + * + * This operation is non-atomic and can 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. + */ +static __always_inline int +generic___test_and_clear_bit(unsigned int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + unsigned long old = *p; + + *p = old & ~mask; + return (old & mask) != 0; +} + +/* WARNING: non atomic and it can be reordered! */ +static __always_inline int +generic___test_and_change_bit(unsigned int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + unsigned long old = *p; + + *p = old ^ mask; + return (old & mask) != 0; +} + +/** + * generic_test_bit - Determine whether a bit is set + * @nr: bit number to test + * @addr: Address to start counting from + */ +static __always_inline int +generic_test_bit(unsigned int nr, const volatile unsigned long *addr) +{ + return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); +} + +#endif /* __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H */ diff --git a/include/asm-generic/bitops/non-atomic.h b/include/asm-generic/bitops/non-atomic.h index 078cc68be2f1..a05bc090a6a3 100644 --- a/include/asm-generic/bitops/non-atomic.h +++ b/include/asm-generic/bitops/non-atomic.h @@ -2,121 +2,28 @@ #ifndef _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ #define _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ +#include <asm-generic/bitops/generic-non-atomic.h> #include <asm/types.h> -/** - * arch___set_bit - Set a bit in memory - * @nr: the bit to set - * @addr: the address to start counting from - * - * 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. - */ -static __always_inline void -arch___set_bit(unsigned int nr, volatile unsigned long *addr) -{ - unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - - *p |= mask; -} +#define arch___set_bit generic___set_bit #define __set_bit arch___set_bit -static __always_inline void -arch___clear_bit(unsigned int nr, volatile unsigned long *addr) -{ - unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - - *p &= ~mask; -} +#define arch___clear_bit generic___clear_bit #define __clear_bit arch___clear_bit -/** - * arch___change_bit - Toggle a bit in memory - * @nr: the bit to change - * @addr: the address to start counting from - * - * 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. - */ -static __always_inline -void arch___change_bit(unsigned int nr, volatile unsigned long *addr) -{ - unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - - *p ^= mask; -} +#define arch___change_bit generic___change_bit #define __change_bit arch___change_bit -/** - * arch___test_and_set_bit - Set a bit and return its old value - * @nr: Bit to set - * @addr: Address to count from - * - * This operation is non-atomic and can 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. - */ -static __always_inline int -arch___test_and_set_bit(unsigned int nr, volatile unsigned long *addr) -{ - unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - unsigned long old = *p; - - *p = old | mask; - return (old & mask) != 0; -} +#define arch___test_and_set_bit generic___test_and_set_bit #define __test_and_set_bit arch___test_and_set_bit -/** - * arch___test_and_clear_bit - Clear a bit and return its old value - * @nr: Bit to clear - * @addr: Address to count from - * - * This operation is non-atomic and can 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. - */ -static __always_inline int -arch___test_and_clear_bit(unsigned int nr, volatile unsigned long *addr) -{ - unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - unsigned long old = *p; - - *p = old & ~mask; - return (old & mask) != 0; -} +#define arch___test_and_clear_bit generic___test_and_clear_bit #define __test_and_clear_bit arch___test_and_clear_bit -/* WARNING: non atomic and it can be reordered! */ -static __always_inline int -arch___test_and_change_bit(unsigned int nr, volatile unsigned long *addr) -{ - unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); - unsigned long old = *p; - - *p = old ^ mask; - return (old & mask) != 0; -} +#define arch___test_and_change_bit generic___test_and_change_bit #define __test_and_change_bit arch___test_and_change_bit -/** - * arch_test_bit - Determine whether a bit is set - * @nr: bit number to test - * @addr: Address to start counting from - */ -static __always_inline int -arch_test_bit(unsigned int nr, const volatile unsigned long *addr) -{ - return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); -} +#define arch_test_bit generic_test_bit #define test_bit arch_test_bit #endif /* _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ */
Move generic non-atomic bitops from the asm-generic header which gets included only when there are no architecture-specific alternatives, to a separate independent file to make them always available. Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> --- .../asm-generic/bitops/generic-non-atomic.h | 124 ++++++++++++++++++ include/asm-generic/bitops/non-atomic.h | 109 ++------------- 2 files changed, 132 insertions(+), 101 deletions(-) create mode 100644 include/asm-generic/bitops/generic-non-atomic.h