On 8/13/21 10:32 AM, Claudio Imbrenda wrote: > On Fri, 13 Aug 2021 07:36:08 +0000 > Janosch Frank <frankja@xxxxxxxxxxxxx> wrote: > >> Bit setting and clearing is never bad to have. >> >> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx> >> --- >> lib/s390x/asm/bitops.h | 102 >> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 >> insertions(+) >> >> diff --git a/lib/s390x/asm/bitops.h b/lib/s390x/asm/bitops.h >> index 792881ec..f5612855 100644 >> --- a/lib/s390x/asm/bitops.h >> +++ b/lib/s390x/asm/bitops.h >> @@ -17,6 +17,78 @@ >> >> #define BITS_PER_LONG 64 >> >> +static inline unsigned long *bitops_word(unsigned long nr, >> + const volatile unsigned >> long *ptr) +{ >> + unsigned long addr; >> + >> + addr = (unsigned long)ptr + ((nr ^ (nr & (BITS_PER_LONG - >> 1))) >> 3); >> + return (unsigned long *)addr; > > why not just > > return ptr + (nr / BITS_PER_LONG); > >> +} >> + >> +static inline unsigned long bitops_mask(unsigned long nr) >> +{ >> + return 1UL << (nr & (BITS_PER_LONG - 1)); >> +} >> + >> +static inline uint64_t laog(volatile unsigned long *ptr, uint64_t >> mask) +{ >> + uint64_t old; >> + >> + /* load and or 64bit concurrent and interlocked */ >> + asm volatile( >> + " laog %[old],%[mask],%[ptr]\n" >> + : [old] "=d" (old), [ptr] "+Q" (*ptr) >> + : [mask] "d" (mask) >> + : "memory", "cc" ); >> + return old; >> +} > > do we really need the artillery (asm) here? > is there a reason why we can't do this in C? Those are the interlocked/atomic instructions and even though we don't exactly need them right now I wanted to add them for completeness. We might be able to achieve the same via compiler functionality but this is not my expertise. Maybe Thomas or David have a few pointers for me? > >> +static inline uint64_t lang(volatile unsigned long *ptr, uint64_t >> mask) +{ >> + uint64_t old; >> + >> + /* load and and 64bit concurrent and interlocked */ >> + asm volatile( >> + " lang %[old],%[mask],%[ptr]\n" >> + : [old] "=d" (old), [ptr] "+Q" (*ptr) >> + : [mask] "d" (mask) >> + : "memory", "cc" ); >> + return old; >> +} > > (same here as above) > >> + >> +static inline void set_bit(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + uint64_t mask = bitops_mask(nr); >> + uint64_t *addr = bitops_word(nr, ptr); >> + >> + laog(addr, mask); >> +} >> + >> +static inline void set_bit_inv(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + return set_bit(nr ^ (BITS_PER_LONG - 1), ptr); >> +} >> + >> +static inline void clear_bit(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + uint64_t mask = bitops_mask(nr); >> + uint64_t *addr = bitops_word(nr, ptr); >> + >> + lang(addr, ~mask); >> +} >> + >> +static inline void clear_bit_inv(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + return clear_bit(nr ^ (BITS_PER_LONG - 1), ptr); >> +} >> + >> +/* non-atomic bit manipulation functions */ >> + >> static inline bool test_bit(unsigned long nr, >> const volatile unsigned long *ptr) >> { >> @@ -33,4 +105,34 @@ static inline bool test_bit_inv(unsigned long nr, >> return test_bit(nr ^ (BITS_PER_LONG - 1), ptr); >> } >> >> +static inline void __set_bit(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + uint64_t mask = bitops_mask(nr); >> + uint64_t *addr = bitops_word(nr, ptr); >> + >> + *addr |= mask; >> +} >> + >> +static inline void __set_bit_inv(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + return __set_bit(nr ^ (BITS_PER_LONG - 1), ptr); >> +} >> + >> +static inline void __clear_bit(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + uint64_t mask = bitops_mask(nr); >> + uint64_t *addr = bitops_word(nr, ptr); >> + >> + *addr &= ~mask; >> +} >> + >> +static inline void __clear_bit_inv(unsigned long nr, >> + const volatile unsigned long *ptr) >> +{ >> + return __clear_bit(nr ^ (BITS_PER_LONG - 1), ptr); >> +} >> + >> #endif >