lkml.org 
[lkml]   [2022]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH v3] wait_on_bit: add an acquire memory barrier
On Fri, Aug 26, 2022 at 12:23 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> include/asm-generic/bitops/non-instrumented-non-atomic.h:15:33:
> error: implicit declaration of function 'arch_test_bit_acquire'; did
> you mean '_test_bit_acquire'? [-Werror=implicit-function-declaration]
>

Ahh. m68k isn't using any of the generic bitops headers.

*Most* architectures have that

#include <asm-generic/bitops/non-atomic.h>

and get it that way, but while it's common, it's most definitely not universal:

[torvalds@ryzen linux]$ git grep -L bitops/non-atomic.h
arch/*/include/asm/bitops.h
arch/alpha/include/asm/bitops.h
arch/hexagon/include/asm/bitops.h
arch/ia64/include/asm/bitops.h
arch/m68k/include/asm/bitops.h
arch/s390/include/asm/bitops.h
arch/sparc/include/asm/bitops.h
arch/x86/include/asm/bitops.h

and of that list only x86 has the new arch_test_bit_acquire().

So I assume it's not just m68k, but also alpha, hexagon, ia64, s390
and sparc that have this issue (unless they maybe have some other path
that includes the gerneric ones, I didn't check).

This was actually why my original suggested patch used the
'generic-non-atomic.h' header for it, because that is actually
included regardless of any architecture headers directly from
<linux/bitops.h>.

And it never triggered for me that Mikulas' updated patch then had
this arch_test_bit_acquire() issue.

Something like the attached patch *MAY* fix it, but I really haven't
thought about it a lot, and it's pretty ugly. Maybe it would be better
to just add the

#define arch_test_bit_acquire generic_test_bit_acquire

to the affected <asm/bitops.h> files instead, and then let those
architectures decide on their own that maybe they want to use their
own test_bit() function because it is _already_ an acquire one.

Mikulas?

Geert - any opinions on that "maybe the arch should just do that
#define itself"? I don't think it actually matters for m68k, you end
up with pretty much the same thing anyway, because
"smp_load_acquire()" is just a load anyway..

Linus
arch/x86/include/asm/bitops.h | 1 +
include/linux/bitops.h | 4 ++++
2 files changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 0fe9de58af31..b82006138c60 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -246,6 +246,7 @@ arch_test_bit_acquire(unsigned long nr, const volatile unsigned long *addr)
return __builtin_constant_p(nr) ? constant_test_bit_acquire(nr, addr) :
variable_test_bit(nr, addr);
}
+#define arch_test_bit_acquire arch_test_bit_acquire

/**
* __ffs - find first set bit in word
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 3b89c64bcfd8..a046b9c45fdb 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -67,6 +67,10 @@ extern unsigned long __sw_hweight64(__u64 w);
*/
#include <asm/bitops.h>

+#ifndef arch_test_bit_acquire
+#define arch_test_bit_acquire generic_test_bit_acquire
+#endif
+
/* Check that the bitops prototypes are sane */
#define __check_bitop_pr(name) \
static_assert(__same_type(arch_##name, generic_##name) && \
\
 
 \ /
  Last update: 2022-08-26 22:11    [W:1.527 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site