commit d6ffe6067a54972564552ea45d320fb98db1ac5e upstream.
Some architectures define their own arch_test_bit and they also need arch_test_bit_acquire, otherwise they won't compile. We also clean up the code by using the generic test_bit if that is equivalent to the arch-specific version.
Signed-off-by: Mikulas Patocka mpatocka@redhat.com Cc: stable@vger.kernel.org Fixes: 8238b4579866 ("wait_on_bit: add an acquire memory barrier") Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
--- arch/alpha/include/asm/bitops.h | 7 +++++++ arch/h8300/include/asm/bitops.h | 3 ++- arch/hexagon/include/asm/bitops.h | 15 +++++++++++++++ arch/ia64/include/asm/bitops.h | 7 +++++++ arch/m68k/include/asm/bitops.h | 6 ++++++ arch/s390/include/asm/bitops.h | 7 +++++++ arch/sh/include/asm/bitops-op32.h | 7 +++++++ 7 files changed, 51 insertions(+), 1 deletion(-)
Index: linux-stable/arch/alpha/include/asm/bitops.h =================================================================== --- linux-stable.orig/arch/alpha/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/alpha/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 @@ -289,6 +289,13 @@ test_bit(int nr, const volatile void * a return (1UL & (((const int *) addr)[nr >> 5] >> (nr & 31))) != 0UL; }
+static __always_inline bool +test_bit_acquire(unsigned long nr, const volatile unsigned long *addr) +{ + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1))); +} + /* * ffz = Find First Zero in word. Undefined if no zero exists, * so code should check against ~0UL first.. Index: linux-stable/arch/hexagon/include/asm/bitops.h =================================================================== --- linux-stable.orig/arch/hexagon/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/hexagon/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 @@ -172,7 +172,22 @@ static inline int __test_bit(int nr, con return retval; }
+static inline int __test_bit_acquire(int nr, const volatile unsigned long *addr) +{ + int retval; + + asm volatile( + "{P0 = tstbit(%1,%2); if (P0.new) %0 = #1; if (!P0.new) %0 = #0;}\n" + : "=&r" (retval) + : "r" (addr[BIT_WORD(nr)]), "r" (nr % BITS_PER_LONG) + : "p0", "memory" + ); + + return retval; +} + #define test_bit(nr, addr) __test_bit(nr, addr) +#define test_bit_acquire(nr, addr) __test_bit_acquire(nr, addr)
/* * ffz - find first zero in word. Index: linux-stable/arch/ia64/include/asm/bitops.h =================================================================== --- linux-stable.orig/arch/ia64/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/ia64/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 @@ -337,6 +337,13 @@ test_bit (int nr, const volatile void *a return 1 & (((const volatile __u32 *) addr)[nr >> 5] >> (nr & 31)); }
+static __always_inline bool +test_bit_acquire(unsigned long nr, const volatile unsigned long *addr) +{ + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1))); +} + /** * ffz - find the first zero bit in a long word * @x: The long word to find the bit in Index: linux-stable/arch/m68k/include/asm/bitops.h =================================================================== --- linux-stable.orig/arch/m68k/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/m68k/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 @@ -153,6 +153,12 @@ static inline int test_bit(int nr, const return (vaddr[nr >> 5] & (1UL << (nr & 31))) != 0; }
+static __always_inline bool +test_bit_acquire(unsigned long nr, const volatile unsigned long *addr) +{ + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1))); +}
static inline int bset_reg_test_and_set_bit(int nr, volatile unsigned long *vaddr) Index: linux-stable/arch/s390/include/asm/bitops.h =================================================================== --- linux-stable.orig/arch/s390/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/s390/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 @@ -184,6 +184,13 @@ static inline bool arch_test_bit(unsigne return *addr & mask; }
+static __always_inline bool +arch_test_bit_acquire(unsigned long nr, const volatile unsigned long *addr) +{ + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1))); +} + static inline bool arch_test_and_set_bit_lock(unsigned long nr, volatile unsigned long *ptr) { Index: linux-stable/arch/sh/include/asm/bitops-op32.h =================================================================== --- linux-stable.orig/arch/sh/include/asm/bitops-op32.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/sh/include/asm/bitops-op32.h 2022-10-27 14:22:01.000000000 +0200 @@ -138,4 +138,11 @@ static inline int test_bit(int nr, const return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); }
+static __always_inline bool +test_bit_acquire(unsigned long nr, const volatile unsigned long *addr) +{ + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); + return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1))); +} + #endif /* __ASM_SH_BITOPS_OP32_H */ Index: linux-stable/arch/h8300/include/asm/bitops.h =================================================================== --- linux-stable.orig/arch/h8300/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 +++ linux-stable/arch/h8300/include/asm/bitops.h 2022-10-27 14:22:01.000000000 +0200 @@ -87,7 +87,8 @@ static inline int test_bit(int nr, const return ret; }
-#define __test_bit(nr, addr) test_bit(nr, addr) +#define __test_bit(nr, addr) test_bit(nr, addr) +#define test_bit_acquire(nr, addr) test_bit(nr, addr)
#define H8300_GEN_TEST_BITOP(FNNAME, OP) \ static inline int FNNAME(int nr, void *addr) \
On Thu, Oct 27, 2022 at 08:41:45AM -0400, Mikulas Patocka wrote:
commit d6ffe6067a54972564552ea45d320fb98db1ac5e upstream.
Some architectures define their own arch_test_bit and they also need arch_test_bit_acquire, otherwise they won't compile. We also clean up the code by using the generic test_bit if that is equivalent to the arch-specific version.
Signed-off-by: Mikulas Patocka mpatocka@redhat.com Cc: stable@vger.kernel.org Fixes: 8238b4579866 ("wait_on_bit: add an acquire memory barrier") Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
arch/alpha/include/asm/bitops.h | 7 +++++++ arch/h8300/include/asm/bitops.h | 3 ++- arch/hexagon/include/asm/bitops.h | 15 +++++++++++++++ arch/ia64/include/asm/bitops.h | 7 +++++++ arch/m68k/include/asm/bitops.h | 6 ++++++ arch/s390/include/asm/bitops.h | 7 +++++++ arch/sh/include/asm/bitops-op32.h | 7 +++++++ 7 files changed, 51 insertions(+), 1 deletion(-)
This is _very_ different from the upstream change that you are trying to backport here.
Are you sure it is correct? You are adding real functions for these arches, while the original backport was _REMOVING_ them and having the arch code call the generic functions.
So why is this the same?
confused,
greg k-h
On Mon, 7 Nov 2022, Greg KH wrote:
On Thu, Oct 27, 2022 at 08:41:45AM -0400, Mikulas Patocka wrote:
commit d6ffe6067a54972564552ea45d320fb98db1ac5e upstream.
Some architectures define their own arch_test_bit and they also need arch_test_bit_acquire, otherwise they won't compile. We also clean up the code by using the generic test_bit if that is equivalent to the arch-specific version.
Signed-off-by: Mikulas Patocka mpatocka@redhat.com Cc: stable@vger.kernel.org Fixes: 8238b4579866 ("wait_on_bit: add an acquire memory barrier") Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
arch/alpha/include/asm/bitops.h | 7 +++++++ arch/h8300/include/asm/bitops.h | 3 ++- arch/hexagon/include/asm/bitops.h | 15 +++++++++++++++ arch/ia64/include/asm/bitops.h | 7 +++++++ arch/m68k/include/asm/bitops.h | 6 ++++++ arch/s390/include/asm/bitops.h | 7 +++++++ arch/sh/include/asm/bitops-op32.h | 7 +++++++ 7 files changed, 51 insertions(+), 1 deletion(-)
This is _very_ different from the upstream change that you are trying to backport here.
Are you sure it is correct?
I compile-tested it with all the cross-compilers downloaded from https://mirrors.edge.kernel.org/pub/tools/crosstool/ (I tried to compile the file kernel/sched/build_utility.o that includes wait_bit.c)
You are adding real functions for these arches, while the original backport was _REMOVING_ them and having the arch code call the generic functions.
The kernels 5.19 and older don't contain the function generic_test_bit - and they don't contain the file "include/asm-generic/bitops/generic-non-atomic.h" where this function is defined. The test_bit code was refactored in 6.0 - see the commits
21bb8af513d35c005c401706030f4eb469538d1d 0e862838f290147ea9c16db852d8d494b552d38d bb7379bfa680bd48b468e856475778db2ad866c1 e69eb9c460f128b71c6b995d75a05244e4b6cc3e b03fc1173c0c2bb8fad61902a862985cecdc4b1b
So, the upstream patch doesn't apply to the older kernels.
So why is this the same?
Functionally, it is equivalent if you define a function test_bit_acquire or refer to an existing generic_test_bit_acquire that has the same content.
Mikulas
confused,
greg k-h
On Mon, Nov 07, 2022 at 04:10:30PM -0500, Mikulas Patocka wrote:
On Mon, 7 Nov 2022, Greg KH wrote:
On Thu, Oct 27, 2022 at 08:41:45AM -0400, Mikulas Patocka wrote:
commit d6ffe6067a54972564552ea45d320fb98db1ac5e upstream.
Some architectures define their own arch_test_bit and they also need arch_test_bit_acquire, otherwise they won't compile. We also clean up the code by using the generic test_bit if that is equivalent to the arch-specific version.
Signed-off-by: Mikulas Patocka mpatocka@redhat.com Cc: stable@vger.kernel.org Fixes: 8238b4579866 ("wait_on_bit: add an acquire memory barrier") Signed-off-by: Linus Torvalds torvalds@linux-foundation.org
arch/alpha/include/asm/bitops.h | 7 +++++++ arch/h8300/include/asm/bitops.h | 3 ++- arch/hexagon/include/asm/bitops.h | 15 +++++++++++++++ arch/ia64/include/asm/bitops.h | 7 +++++++ arch/m68k/include/asm/bitops.h | 6 ++++++ arch/s390/include/asm/bitops.h | 7 +++++++ arch/sh/include/asm/bitops-op32.h | 7 +++++++ 7 files changed, 51 insertions(+), 1 deletion(-)
This is _very_ different from the upstream change that you are trying to backport here.
Are you sure it is correct?
I compile-tested it with all the cross-compilers downloaded from https://mirrors.edge.kernel.org/pub/tools/crosstool/ (I tried to compile the file kernel/sched/build_utility.o that includes wait_bit.c)
That's not what I am asking here.
You are adding real functions for these arches, while the original backport was _REMOVING_ them and having the arch code call the generic functions.
The kernels 5.19 and older don't contain the function generic_test_bit - and they don't contain the file "include/asm-generic/bitops/generic-non-atomic.h" where this function is defined. The test_bit code was refactored in 6.0 - see the commits
21bb8af513d35c005c401706030f4eb469538d1d 0e862838f290147ea9c16db852d8d494b552d38d bb7379bfa680bd48b468e856475778db2ad866c1 e69eb9c460f128b71c6b995d75a05244e4b6cc3e b03fc1173c0c2bb8fad61902a862985cecdc4b1b
So, the upstream patch doesn't apply to the older kernels.
I agree, so then please backport the needed commits. Do not create a commit that says it is one thing, but really looks very very different from the original. That's going to make it almost impossible to maintain over time, right?
So why is this the same?
Functionally, it is equivalent if you define a function test_bit_acquire or refer to an existing generic_test_bit_acquire that has the same content.
No, it's not the same, you are putting logic in each arch that in mainline is not there. In Linus's tree there is a reference to the generic asm function. Here you are adding arch-specific functions.
Please redo this series and resubmit them, properly threaded (hint, your latest one is not threaded at all and is impossible to use our tools on), so that we can review them again.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Mikulas Patocka