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@xxxxxxxxxx>
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Fixes: 8238b4579866 ("wait_on_bit: add an acquire memory barrier")
> > > Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> > >
> > > ---
> > > 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
