Re: [kvm-unit-tests PATCH 1/8] s390x: lib: Extend bitops
From: Janosch Frank <frankja@linux.ibm.com>
Date: 2021-08-18 08:39:49
Also in:
linux-s390
On 8/18/21 10:20 AM, Thomas Huth wrote:
On 13/08/2021 13.31, Janosch Frank wrote:quoted
On 8/13/21 10:32 AM, Claudio Imbrenda wrote:quoted
On Fri, 13 Aug 2021 07:36:08 +0000 Janosch Frank [off-list ref] wrote:quoted
Bit setting and clearing is never bad to have. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- 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 unsignedlong *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);quoted
+} + +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.I think I agree with Claudio - unless we really need them, we should not clog the sources with arbitrary inline assembly functions.
Alright I can trim it down
quoted
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?I'm not an expert with atomic builtins either, but what's the point of this at all? Loading a value and OR-ing something into the value in one go? What's that good for?
Well it's a block-concurrent interlocked-update load, or and store. I.e. it loads the data from the ptr and copies it into [old] then ors the mask and stores it back to the ptr address. The instruction name "load and or" does not represent the full actions of the instruction.
Thomas