[RFC 0/2] Import and use non-atomic bit-ops

The following bitops are implemented pretty similarly for many arches and now when we faced a need in them on ARC I guess there's no point in copy-pasting them yet another time but instead it might be better re-use generic version from the Linux kernel.
Since we had non of those bitops for ARC inclusion of imported header works perfectly fine. As for other arches I do see they use a bit different implementation but those might be just older versions etc.
Sobefore breaking stuff for other arches I'd like to get some feedback from maintainers. Or we may just import proposed header and switch to its usage arch-by-arch whenever people feel kile cleaning-up their bitops.
Alexey Brodkin (2): include: Import non-atomic.h from Linux ARC: Add support of bitops via generic implementation
arch/arc/include/asm/bitops.h | 1 + include/asm-generic/bitops/non-atomic.h | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 include/asm-generic/bitops/non-atomic.h

This header contains implementations of some common bit ops: * __set_bit()/__clear_bit()/__change_bit()/test_bit() * __test_and_set_bit()/__test_and_clear_bit()/__test_and_change_bit()
No point in copy-pasting that again & again.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com --- include/asm-generic/bitops/non-atomic.h | 109 ++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 include/asm-generic/bitops/non-atomic.h
diff --git a/include/asm-generic/bitops/non-atomic.h b/include/asm-generic/bitops/non-atomic.h new file mode 100644 index 0000000000..9b3be37fff --- /dev/null +++ b/include/asm-generic/bitops/non-atomic.h @@ -0,0 +1,109 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ +#define _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ + +#include <asm/types.h> + +/** + * __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 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; +} + +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; +} + +/** + * __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 inline void __change_bit(int nr, volatile unsigned long *addr) +{ + unsigned long mask = BIT_MASK(nr); + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + + *p ^= mask; +} + +/** + * __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 inline int __test_and_set_bit(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; +} + +/** + * __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 inline int __test_and_clear_bit(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 inline int __test_and_change_bit(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; +} + +/** + * test_bit - Determine whether a bit is set + * @nr: bit number to test + * @addr: Address to start counting from + */ +static inline int test_bit(int nr, const volatile unsigned long *addr) +{ + return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG - 1))); +} + +#endif /* _ASM_GENERIC_BITOPS_NON_ATOMIC_H_ */

This allows building more things. In particular with CONFIG_PKCS7_MESSAGE_PARSER=y we saw this: ------------------------>8---------------------- lib/crypto/pkcs7_parser.c: In function 'pkcs7_sig_note_authenticated_attr': lib/crypto/pkcs7_parser.c:489:7: warning: implicit declaration of function '__test_and_set_bit' [-Wimplicit-function-declaration] 489 | if (__test_and_set_bit(sinfo_has_content_type, &sinfo->aa_set)) | ^~~~~~~~~~~~~~~~~~ CC lib/crc32.o CC lib/ctype.o DTB test/overlay/test-fdt-overlay-stacked.dtb.S CC lib/div64.o lib/crypto/pkcs7_parser.c: In function 'pkcs7_sig_note_set_of_authattrs': lib/crypto/pkcs7_parser.c:572:7: warning: implicit declaration of function 'test_bit' [-Wimplicit-function-declaration] 572 | if (!test_bit(sinfo_has_content_type, &sinfo->aa_set) || ... LD u-boot arc-elf32-ld.bfd: lib/built-in.o: in function 'pkcs7_sig_note_authenticated_attr': .../lib/crypto/pkcs7_parser.c:489: undefined reference to '__test_and_set_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:489: undefined reference to '__test_and_set_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:501: undefined reference to '__test_and_set_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:501: undefined reference to '__test_and_set_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:510: undefined reference to '__test_and_set_bit' arc-elf32-ld.bfd: lib/built-in.o:.../lib/crypto/pkcs7_parser.c:510: more undefined references to '__test_and_set_bit' follow arc-elf32-ld.bfd: lib/built-in.o: in function 'pkcs7_sig_note_set_of_authattrs': .../lib/crypto/pkcs7_parser.c:572: undefined reference to `test_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:572: undefined reference to `test_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:573: undefined reference to `test_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:573: undefined reference to `test_bit' arc-elf32-ld.bfd: .../lib/crypto/pkcs7_parser.c:579: undefined reference to `test_bit' arc-elf32-ld.bfd: lib/built-in.o:.../lib/crypto/pkcs7_parser.c:579: more undefined references to 'test_bit' follow ------------------------>8----------------------
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com --- arch/arc/include/asm/bitops.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arc/include/asm/bitops.h b/arch/arc/include/asm/bitops.h index c6dd28ecef..daa07e80a8 100644 --- a/arch/arc/include/asm/bitops.h +++ b/arch/arc/include/asm/bitops.h @@ -19,5 +19,6 @@ #include <asm-generic/bitops/__fls.h> #include <asm-generic/bitops/fls64.h> #include <asm-generic/bitops/__ffs.h> +#include <asm-generic/bitops/non-atomic.h>
#endif /* __ASM_ARC_BITOPS_H */

On Mon, Jan 20, 2020 at 03:43:27PM +0300, Alexey Brodkin wrote:
The following bitops are implemented pretty similarly for many arches and now when we faced a need in them on ARC I guess there's no point in copy-pasting them yet another time but instead it might be better re-use generic version from the Linux kernel.
Since we had non of those bitops for ARC inclusion of imported header works perfectly fine. As for other arches I do see they use a bit different implementation but those might be just older versions etc.
Sobefore breaking stuff for other arches I'd like to get some feedback from maintainers. Or we may just import proposed header and switch to its usage arch-by-arch whenever people feel kile cleaning-up their bitops.
Alexey Brodkin (2): include: Import non-atomic.h from Linux ARC: Add support of bitops via generic implementation
arch/arc/include/asm/bitops.h | 1 + include/asm-generic/bitops/non-atomic.h | 109 ++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 include/asm-generic/bitops/non-atomic.h
I would like to see this as a series to re-sync include/asm-generic/bitops/ with a more recent Linux Kernel release (it's from v4.2.3 per git log) and then add non-atomic.h and update nios2 to use it instead of its own copy. Thanks!
participants (2)
-
Alexey Brodkin
-
Tom Rini