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