Message ID | 20180709124741.21037-1-abrodkin@synopsys.com |
---|---|
State | New |
Headers | show |
Series | atomic{64}_t: Explicitly specify data storage length and alignment | expand |
Hi Alexey, On Mon, Jul 9, 2018 at 2:48 PM Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Atomic instructions require data they operate on to be aligned > according to data size. I.e. 32-bit atomic values must be 32-bit > aligned while 64-bit values must be 64-bit aligned. > > Otherwise even if CPU may handle not-aligend normal data access, > still atomic instructions fail and typically raise an exception > leaving us dead in the water. > > This came-up during lengthly discussion here: > http://lists.infradead.org/pipermail/linux-snps-arc/2018-July/004022.html > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Thanks for your patch! > --- a/include/asm-generic/atomic64.h > +++ b/include/asm-generic/atomic64.h > @@ -13,7 +13,7 @@ > #define _ASM_GENERIC_ATOMIC64_H > > typedef struct { > - long long counter; > + u64 __aligned(8) counter; I think you can drop the change to this file, as this is the generic implementation using spinlocks, for processors that don't have 64-bit atomic instructions. > } atomic64_t; Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert
On Mon, Jul 09, 2018 at 03:47:41PM +0300, Alexey Brodkin wrote: > Atomic instructions require data they operate on to be aligned > according to data size. I.e. 32-bit atomic values must be 32-bit > aligned while 64-bit values must be 64-bit aligned. > > Otherwise even if CPU may handle not-aligend normal data access, > still atomic instructions fail and typically raise an exception > leaving us dead in the water. > > This came-up during lengthly discussion here: > http://lists.infradead.org/pipermail/linux-snps-arc/2018-July/004022.html > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Boqun Feng <boqun.feng@gmail.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Darren Hart <dvhart@infradead.org> > Cc: Shuah Khan <shuah@kernel.org> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> > Cc: Josh Triplett <josh@joshtriplett.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Cc: David Laight <David.Laight@ACULAB.COM> > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > arch/arm/include/asm/atomic.h | 2 +- > include/asm-generic/atomic64.h | 2 +- > include/linux/types.h | 4 ++-- > tools/include/linux/types.h | 2 +- > tools/testing/selftests/futex/include/atomic.h | 2 +- > .../rcutorture/formal/srcu-cbmc/include/linux/types.h | 4 ++-- > 6 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h > index 66d0e215a773..2ed6d7cf1407 100644 > --- a/arch/arm/include/asm/atomic.h > +++ b/arch/arm/include/asm/atomic.h > @@ -267,7 +267,7 @@ ATOMIC_OPS(xor, ^=, eor) > > #ifndef CONFIG_GENERIC_ATOMIC64 > typedef struct { > - long long counter; > + u64 __aligned(8) counter; > } atomic64_t; Long long is 8-byte aligned per EABI ARM, and we use the generic atomic64 infrastructure for OABI, so we don't need to change anything here afaict. Will
From: Alexey Brodkin > Sent: 09 July 2018 13:48 > Atomic instructions require data they operate on to be aligned > according to data size. I.e. 32-bit atomic values must be 32-bit > aligned while 64-bit values must be 64-bit aligned. > > Otherwise even if CPU may handle not-aligend normal data access, > still atomic instructions fail and typically raise an exception > leaving us dead in the water. ... > diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h > index 8d28eb010d0d..b94b749b5952 100644 > --- a/include/asm-generic/atomic64.h > +++ b/include/asm-generic/atomic64.h > @@ -13,7 +13,7 @@ > #define _ASM_GENERIC_ATOMIC64_H > > typedef struct { > - long long counter; > + u64 __aligned(8) counter; > } atomic64_t; Apart from the fact that this changes the value from signed to unsigned should most of the architectures be using this generic definition? The only exceptions would be 32bit architectures that lack 64bit instructions (eg m68k). David
On Mon, Jul 09, 2018 at 03:47:41PM +0300, Alexey Brodkin wrote: > diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h > index 8d28eb010d0d..b94b749b5952 100644 > --- a/include/asm-generic/atomic64.h > +++ b/include/asm-generic/atomic64.h > @@ -13,7 +13,7 @@ > #define _ASM_GENERIC_ATOMIC64_H > > typedef struct { > - long long counter; > + u64 __aligned(8) counter; > } atomic64_t; The type is wrong, atomic is signed, the alignment also really doesn't matter, generic atomic64 is utter crap. > #define ATOMIC64_INIT(i) { (i) } > diff --git a/include/linux/types.h b/include/linux/types.h > index 9834e90aa010..e2f631782621 100644 > --- a/include/linux/types.h > +++ b/include/linux/types.h > @@ -174,12 +174,12 @@ typedef phys_addr_t resource_size_t; > typedef unsigned long irq_hw_number_t; > > typedef struct { > - int counter; > + u32 __aligned(4) counter; > } atomic_t; u32 is wrong, the atomic type is signed. Also, if an architecture doesn't properly align its native machine word size but requires alignment for atomics it's a broken architecture. > > #ifdef CONFIG_64BIT > typedef struct { > - long counter; > + u64 __aligned(8) counter; > } atomic64_t; > #endif > Similar for this one, on 64bit archs that support atomics the native 64bit types (long included) had better already imply this alignment.
On Mon, Jul 9, 2018 at 3:29 PM David Laight <David.Laight@aculab.com> wrote: > From: Alexey Brodkin > > Sent: 09 July 2018 13:48 > > Atomic instructions require data they operate on to be aligned > > according to data size. I.e. 32-bit atomic values must be 32-bit > > aligned while 64-bit values must be 64-bit aligned. > > > > Otherwise even if CPU may handle not-aligend normal data access, > > still atomic instructions fail and typically raise an exception > > leaving us dead in the water. > ... > > diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h > > index 8d28eb010d0d..b94b749b5952 100644 > > --- a/include/asm-generic/atomic64.h > > +++ b/include/asm-generic/atomic64.h > > @@ -13,7 +13,7 @@ > > #define _ASM_GENERIC_ATOMIC64_H > > > > typedef struct { > > - long long counter; > > + u64 __aligned(8) counter; > > } atomic64_t; > > Apart from the fact that this changes the value from signed to unsigned > should most of the architectures be using this generic definition? 64-bit architectures use the one from include/linux/types.h instead. Gr{oetje,eeting}s, Geert
On Mon, Jul 09, 2018 at 03:47:41PM +0300, Alexey Brodkin wrote: > Atomic instructions require data they operate on to be aligned > according to data size. I.e. 32-bit atomic values must be 32-bit > aligned while 64-bit values must be 64-bit aligned. > > Otherwise even if CPU may handle not-aligend normal data access, > still atomic instructions fail and typically raise an exception > leaving us dead in the water. > > This came-up during lengthly discussion here: > http://lists.infradead.org/pipermail/linux-snps-arc/2018-July/004022.html > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Boqun Feng <boqun.feng@gmail.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Darren Hart <dvhart@infradead.org> > Cc: Shuah Khan <shuah@kernel.org> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> > Cc: Josh Triplett <josh@joshtriplett.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Cc: David Laight <David.Laight@ACULAB.COM> > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > arch/arm/include/asm/atomic.h | 2 +- > include/asm-generic/atomic64.h | 2 +- > include/linux/types.h | 4 ++-- > tools/include/linux/types.h | 2 +- > tools/testing/selftests/futex/include/atomic.h | 2 +- > .../rcutorture/formal/srcu-cbmc/include/linux/types.h | 4 ++-- > 6 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h > index 66d0e215a773..2ed6d7cf1407 100644 > --- a/arch/arm/include/asm/atomic.h > +++ b/arch/arm/include/asm/atomic.h > @@ -267,7 +267,7 @@ ATOMIC_OPS(xor, ^=, eor) > > #ifndef CONFIG_GENERIC_ATOMIC64 > typedef struct { > - long long counter; > + u64 __aligned(8) counter; > } atomic64_t; ARM doesn't need or require this change, and you're changing the type from signed to unsigned, which is likely to break stuff. So, NAK on this from the ARM point of view.
Hi Peter, On Mon, 2018-07-09 at 15:35 +0200, Peter Zijlstra wrote: > On Mon, Jul 09, 2018 at 03:47:41PM +0300, Alexey Brodkin wrote: > > diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h > > index 8d28eb010d0d..b94b749b5952 100644 > > --- a/include/asm-generic/atomic64.h > > +++ b/include/asm-generic/atomic64.h > > @@ -13,7 +13,7 @@ > > #define _ASM_GENERIC_ATOMIC64_H > > > > typedef struct { > > - long long counter; > > + u64 __aligned(8) counter; > > } atomic64_t; > > The type is wrong, atomic is signed, the alignment also really doesn't > matter, generic atomic64 is utter crap. Hm, any thoughts on why it's "u64" for 32-bit x86? See https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/atomic64_32.h#L12 ------------------------->8----------------------- /* An 64bit atomic type */ typedef struct { u64 __aligned(8) counter; } atomic64_t; ------------------------->8----------------------- > > #define ATOMIC64_INIT(i) { (i) } > > diff --git a/include/linux/types.h b/include/linux/types.h > > index 9834e90aa010..e2f631782621 100644 > > --- a/include/linux/types.h > > +++ b/include/linux/types.h > > @@ -174,12 +174,12 @@ typedef phys_addr_t resource_size_t; > > typedef unsigned long irq_hw_number_t; > > > > typedef struct { > > - int counter; > > + u32 __aligned(4) counter; > > } atomic_t; > > u32 is wrong, the atomic type is signed. > > Also, if an architecture doesn't properly align its native machine word > size but requires alignment for atomics it's a broken architecture. Ok we may not touch 32-bit atomics as there's a hope most of arches properly align native machine words. > > > > #ifdef CONFIG_64BIT > > typedef struct { > > - long counter; > > + u64 __aligned(8) counter; > > } atomic64_t; > > #endif > > > > Similar for this one, on 64bit archs that support atomics the native > 64bit types (long included) had better already imply this alignment. Ok agree. -Alexey
On Mon, Jul 09, 2018 at 02:30:41PM +0000, Alexey Brodkin wrote:
> Hm, any thoughts on why it's "u64" for 32-bit x86?
Accident probably. It probably doesn't really matter all that much
because the kernel hard assumes 2s complement, but it is somewhat
inconsistent.
Hi Alexey, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc4 next-20180709] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alexey-Brodkin/atomic-64-_t-Explicitly-specify-data-storage-length-and-alignment/20180709-223920 config: x86_64-acpi-redef (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): In file included from tools/objtool/arch/x86/include/asm/orc_types.h:21:0, from orc.h:21, from orc_dump.c:19: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ In file included from tools/objtool/arch/x86/include/asm/orc_types.h:21:0, from orc.h:21, from orc_gen.c:21: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ In file included from tools/include/linux/string.h:5:0, from ../lib/string.c:19: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ cc1: all warnings being treated as errors >> mv: cannot stat 'tools/objtool/.libstring.o.tmp': No such file or directory make[4]: *** [tools/objtool/libstring.o] Error 1 In file included from tools/include/linux/string.h:5:0, from ../lib/str_error_r.c:5: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ cc1: all warnings being treated as errors In file included from tools/include/linux/list.h:5:0, from elf.h:23, from elf.c:31: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ In file included from tools/include/linux/list.h:5:0, from elf.h:23, from special.h:22, from special.c:26: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ mv: cannot stat 'tools/objtool/.str_error_r.o.tmp': No such file or directory make[4]: *** [tools/objtool/str_error_r.o] Error 1 In file included from tools/include/linux/string.h:5:0, from run-command.c:7: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ cc1: all warnings being treated as errors mv: cannot stat 'tools/objtool/.orc_dump.o.tmp': No such file or directory make[4]: *** [tools/objtool/orc_dump.o] Error 1 In file included from tools/include/linux/string.h:5:0, from help.c:5: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ cc1: all warnings being treated as errors mv: cannot stat 'tools/objtool/.special.o.tmp': No such file or directory make[4]: *** [tools/objtool/special.o] Error 1 cc1: all warnings being treated as errors cc1: all warnings being treated as errors mv: cannot stat 'tools/objtool/.run-command.o.tmp': No such file or directory make[5]: *** [tools/objtool/run-command.o] Error 1 mv: cannot stat 'tools/objtool/.orc_gen.o.tmp': No such file or directory make[4]: *** [tools/objtool/orc_gen.o] Error 1 cc1: all warnings being treated as errors mv: cannot stat 'tools/objtool/.help.o.tmp': No such file or directory make[5]: *** [tools/objtool/help.o] Error 1 cc1: all warnings being treated as errors mv: cannot stat 'tools/objtool/.elf.o.tmp': No such file or directory make[4]: *** [tools/objtool/elf.o] Error 1 In file included from tools/include/linux/list.h:5:0, from arch/x86/../../elf.h:23, from arch/x86/decode.c:26: >> tools/include/linux/types.h:62:16: error: expected declaration specifiers or '...' before numeric constant u32 __aligned(4) counter; ^ tools/include/linux/types.h:63:1: error: no semicolon at end of struct or union [-Werror] } atomic_t; ^ cc1: all warnings being treated as errors mv: cannot stat 'tools/objtool/arch/x86/.decode.o.tmp': No such file or directory make[5]: *** [tools/objtool/arch/x86/decode.o] Error 1 make[5]: Target '__build' not remade because of errors. make[4]: *** [arch/x86] Error 2 make[5]: Target '__build' not remade because of errors. make[4]: *** [tools/objtool/libsubcmd-in.o] Error 2 make[4]: Target 'all' not remade because of errors. make[3]: *** [tools/objtool/libsubcmd.a] Error 2 make[4]: Target '__build' not remade because of errors. make[3]: *** [tools/objtool/objtool-in.o] Error 2 make[3]: Target 'all' not remade because of errors. make[2]: *** [objtool] Error 2 make[1]: *** [tools/objtool] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [sub-make] Error 2 vim +62 tools/include/linux/types.h 60 61 typedef struct { > 62 u32 __aligned(4) counter; 63 } atomic_t; 64 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Alexey, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc4 next-20180709] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Alexey-Brodkin/atomic-64-_t-Explicitly-specify-data-storage-length-and-alignment/20180709-223920 config: s390-allmodconfig (attached as .config) compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=s390 All error/warnings (new ones prefixed by >>): In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_add_return': >> arch/s390/include/asm/atomic.h:129:35: error: passing argument 2 of '__atomic64_add_barrier' from incompatible pointer type [-Werror=incompatible-pointer-types] return __atomic64_add_barrier(i, &v->counter) + i; ^ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:35:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_add, long, "laag") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ >> arch/s390/include/asm/atomic_ops.h:35:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_add, long, "laag") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_fetch_add': arch/s390/include/asm/atomic.h:134:35: error: passing argument 2 of '__atomic64_add_barrier' from incompatible pointer type [-Werror=incompatible-pointer-types] return __atomic64_add_barrier(i, &v->counter); ^ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:35:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_add, long, "laag") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ >> arch/s390/include/asm/atomic_ops.h:35:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_add, long, "laag") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_add': >> arch/s390/include/asm/atomic.h:141:27: error: passing argument 2 of '__atomic64_add_const' from incompatible pointer type [-Werror=incompatible-pointer-types] __atomic64_add_const(i, &v->counter); ^ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:57:20: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_CONST_OPS(__atomic64_add_const, long, "agsi") ^ arch/s390/include/asm/atomic_ops.h:44:20: note: in definition of macro '__ATOMIC_CONST_OP' static inline void op_name(op_type val, op_type *ptr) \ ^~~~~~~ >> arch/s390/include/asm/atomic_ops.h:57:1: note: in expansion of macro '__ATOMIC_CONST_OPS' __ATOMIC_CONST_OPS(__atomic64_add_const, long, "agsi") ^~~~~~~~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: >> arch/s390/include/asm/atomic.h:145:20: error: passing argument 2 of '__atomic64_add' from incompatible pointer type [-Werror=incompatible-pointer-types] __atomic64_add(i, &v->counter); ^ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:35:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_add, long, "laag") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ >> arch/s390/include/asm/atomic_ops.h:35:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_add, long, "laag") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_cmpxchg': >> arch/s390/include/asm/atomic.h:152:28: error: passing argument 1 of '__atomic64_cmpxchg' from incompatible pointer type [-Werror=incompatible-pointer-types] return __atomic64_cmpxchg(&v->counter, old, new); ^ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:133:20: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' static inline long __atomic64_cmpxchg(long *ptr, long old, long new) ^~~~~~~~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_and': >> arch/s390/include/asm/atomic.h:158:21: error: passing argument 2 of '__atomic64_and' from incompatible pointer type [-Werror=incompatible-pointer-types] __atomic64_##op(i, &v->counter); \ ^ >> arch/s390/include/asm/atomic.h:165:1: note: in expansion of macro 'ATOMIC64_OPS' ATOMIC64_OPS(and) ^~~~~~~~~~~~ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:36:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_and, long, "lang") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ arch/s390/include/asm/atomic_ops.h:36:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_and, long, "lang") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_fetch_and': >> arch/s390/include/asm/atomic.h:162:38: error: passing argument 2 of '__atomic64_and_barrier' from incompatible pointer type [-Werror=incompatible-pointer-types] return __atomic64_##op##_barrier(i, &v->counter); \ ^ >> arch/s390/include/asm/atomic.h:165:1: note: in expansion of macro 'ATOMIC64_OPS' ATOMIC64_OPS(and) ^~~~~~~~~~~~ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:36:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_and, long, "lang") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ arch/s390/include/asm/atomic_ops.h:36:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_and, long, "lang") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_or': >> arch/s390/include/asm/atomic.h:158:21: error: passing argument 2 of '__atomic64_or' from incompatible pointer type [-Werror=incompatible-pointer-types] __atomic64_##op(i, &v->counter); \ ^ arch/s390/include/asm/atomic.h:166:1: note: in expansion of macro 'ATOMIC64_OPS' ATOMIC64_OPS(or) ^~~~~~~~~~~~ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:37:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_or, long, "laog") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ arch/s390/include/asm/atomic_ops.h:37:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_or, long, "laog") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_fetch_or': >> arch/s390/include/asm/atomic.h:162:38: error: passing argument 2 of '__atomic64_or_barrier' from incompatible pointer type [-Werror=incompatible-pointer-types] return __atomic64_##op##_barrier(i, &v->counter); \ ^ arch/s390/include/asm/atomic.h:166:1: note: in expansion of macro 'ATOMIC64_OPS' ATOMIC64_OPS(or) ^~~~~~~~~~~~ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:37:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_or, long, "laog") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ arch/s390/include/asm/atomic_ops.h:37:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_or, long, "laog") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_xor': >> arch/s390/include/asm/atomic.h:158:21: error: passing argument 2 of '__atomic64_xor' from incompatible pointer type [-Werror=incompatible-pointer-types] __atomic64_##op(i, &v->counter); \ ^ arch/s390/include/asm/atomic.h:167:1: note: in expansion of macro 'ATOMIC64_OPS' ATOMIC64_OPS(xor) ^~~~~~~~~~~~ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:38:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_xor, long, "laxg") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ arch/s390/include/asm/atomic_ops.h:38:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_xor, long, "laxg") ^~~~~~~~~~~~ In file included from include/linux/atomic.h:5:0, from include/linux/debug_locks.h:6, from include/linux/lockdep.h:28, from include/linux/spinlock_types.h:18, from kernel/bounds.c:14: arch/s390/include/asm/atomic.h: In function 'atomic64_fetch_xor': >> arch/s390/include/asm/atomic.h:162:38: error: passing argument 2 of '__atomic64_xor_barrier' from incompatible pointer type [-Werror=incompatible-pointer-types] return __atomic64_##op##_barrier(i, &v->counter); \ ^ arch/s390/include/asm/atomic.h:167:1: note: in expansion of macro 'ATOMIC64_OPS' ATOMIC64_OPS(xor) ^~~~~~~~~~~~ In file included from arch/s390/include/asm/bitops.h:38:0, from include/linux/bitops.h:38, from include/linux/kernel.h:11, from arch/s390/include/asm/bug.h:5, from include/linux/bug.h:5, from include/linux/page-flags.h:10, from kernel/bounds.c:10: arch/s390/include/asm/atomic_ops.h:38:14: note: expected 'long int *' but argument is of type 'u64 * {aka long long unsigned int *}' __ATOMIC_OPS(__atomic64_xor, long, "laxg") ^ arch/s390/include/asm/atomic_ops.h:14:23: note: in definition of macro '__ATOMIC_OP' static inline op_type op_name(op_type val, op_type *ptr) \ ^~~~~~~ arch/s390/include/asm/atomic_ops.h:38:1: note: in expansion of macro '__ATOMIC_OPS' __ATOMIC_OPS(__atomic64_xor, long, "laxg") ^~~~~~~~~~~~ cc1: some warnings being treated as errors make[2]: *** [kernel/bounds.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [sub-make] Error 2 vim +/__atomic64_add_barrier +129 arch/s390/include/asm/atomic.h ^1da177e include/asm-s390/atomic.h Linus Torvalds 2005-04-16 126 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 127 static inline long atomic64_add_return(long i, atomic64_t *v) ^1da177e include/asm-s390/atomic.h Linus Torvalds 2005-04-16 128 { 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @129 return __atomic64_add_barrier(i, &v->counter) + i; 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 130 } 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 131 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 132 static inline long atomic64_fetch_add(long i, atomic64_t *v) 56fefbbc arch/s390/include/asm/atomic.h Peter Zijlstra 2016-04-18 133 { 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 134 return __atomic64_add_barrier(i, &v->counter); 56fefbbc arch/s390/include/asm/atomic.h Peter Zijlstra 2016-04-18 135 } 56fefbbc arch/s390/include/asm/atomic.h Peter Zijlstra 2016-04-18 136 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 137 static inline void atomic64_add(long i, atomic64_t *v) 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 138 { 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 139 #ifdef CONFIG_HAVE_MARCH_Z196_FEATURES 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 140 if (__builtin_constant_p(i) && (i > -129) && (i < 128)) { 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @141 __atomic64_add_const(i, &v->counter); 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 142 return; 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 143 } 0ccc8b7a arch/s390/include/asm/atomic.h Heiko Carstens 2014-03-20 144 #endif 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @145 __atomic64_add(i, &v->counter); ^1da177e include/asm-s390/atomic.h Linus Torvalds 2005-04-16 146 } 973bd993 include/asm-s390/atomic.h Martin Schwidefsky 2006-01-06 147 3a5f10e3 include/asm-s390/atomic.h Mathieu Desnoyers 2007-02-21 148 #define atomic64_xchg(v, new) (xchg(&((v)->counter), new)) 3a5f10e3 include/asm-s390/atomic.h Mathieu Desnoyers 2007-02-21 149 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 150 static inline long atomic64_cmpxchg(atomic64_t *v, long old, long new) 973bd993 include/asm-s390/atomic.h Martin Schwidefsky 2006-01-06 151 { 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @152 return __atomic64_cmpxchg(&v->counter, old, new); 973bd993 include/asm-s390/atomic.h Martin Schwidefsky 2006-01-06 153 } ^1da177e include/asm-s390/atomic.h Linus Torvalds 2005-04-16 154 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 155 #define ATOMIC64_OPS(op) \ ae8c35c8 arch/s390/include/asm/atomic.h Peter Zijlstra 2014-04-23 156 static inline void atomic64_##op(long i, atomic64_t *v) \ ae8c35c8 arch/s390/include/asm/atomic.h Peter Zijlstra 2014-04-23 157 { \ 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @158 __atomic64_##op(i, &v->counter); \ 56fefbbc arch/s390/include/asm/atomic.h Peter Zijlstra 2016-04-18 159 } \ 56fefbbc arch/s390/include/asm/atomic.h Peter Zijlstra 2016-04-18 160 static inline long atomic64_fetch_##op(long i, atomic64_t *v) \ 56fefbbc arch/s390/include/asm/atomic.h Peter Zijlstra 2016-04-18 161 { \ 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @162 return __atomic64_##op##_barrier(i, &v->counter); \ ae8c35c8 arch/s390/include/asm/atomic.h Peter Zijlstra 2014-04-23 163 } ae8c35c8 arch/s390/include/asm/atomic.h Peter Zijlstra 2014-04-23 164 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 @165 ATOMIC64_OPS(and) 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 166 ATOMIC64_OPS(or) 126b30c3 arch/s390/include/asm/atomic.h Martin Schwidefsky 2016-11-11 167 ATOMIC64_OPS(xor) ae8c35c8 arch/s390/include/asm/atomic.h Peter Zijlstra 2014-04-23 168 :::::: The code at line 129 was first introduced by commit :::::: 126b30c3cb476ce68489a657a7defb8e73775e6f s390/atomic: refactor atomic primitives :::::: TO: Martin Schwidefsky <schwidefsky@de.ibm.com> :::::: CC: Martin Schwidefsky <schwidefsky@de.ibm.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h index 66d0e215a773..2ed6d7cf1407 100644 --- a/arch/arm/include/asm/atomic.h +++ b/arch/arm/include/asm/atomic.h @@ -267,7 +267,7 @@ ATOMIC_OPS(xor, ^=, eor) #ifndef CONFIG_GENERIC_ATOMIC64 typedef struct { - long long counter; + u64 __aligned(8) counter; } atomic64_t; #define ATOMIC64_INIT(i) { (i) } diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h index 8d28eb010d0d..b94b749b5952 100644 --- a/include/asm-generic/atomic64.h +++ b/include/asm-generic/atomic64.h @@ -13,7 +13,7 @@ #define _ASM_GENERIC_ATOMIC64_H typedef struct { - long long counter; + u64 __aligned(8) counter; } atomic64_t; #define ATOMIC64_INIT(i) { (i) } diff --git a/include/linux/types.h b/include/linux/types.h index 9834e90aa010..e2f631782621 100644 --- a/include/linux/types.h +++ b/include/linux/types.h @@ -174,12 +174,12 @@ typedef phys_addr_t resource_size_t; typedef unsigned long irq_hw_number_t; typedef struct { - int counter; + u32 __aligned(4) counter; } atomic_t; #ifdef CONFIG_64BIT typedef struct { - long counter; + u64 __aligned(8) counter; } atomic64_t; #endif diff --git a/tools/include/linux/types.h b/tools/include/linux/types.h index 154eb4e3ca7c..c913e26ea4eb 100644 --- a/tools/include/linux/types.h +++ b/tools/include/linux/types.h @@ -59,7 +59,7 @@ typedef __u64 __bitwise __le64; typedef __u64 __bitwise __be64; typedef struct { - int counter; + u32 __aligned(4) counter; } atomic_t; #ifndef __aligned_u64 diff --git a/tools/testing/selftests/futex/include/atomic.h b/tools/testing/selftests/futex/include/atomic.h index f861da3e31ab..34e14295e492 100644 --- a/tools/testing/selftests/futex/include/atomic.h +++ b/tools/testing/selftests/futex/include/atomic.h @@ -23,7 +23,7 @@ #define _ATOMIC_H typedef struct { - volatile int val; + volatile u32 __aligned(4) val; } atomic_t; #define ATOMIC_INITIALIZER { 0 } diff --git a/tools/testing/selftests/rcutorture/formal/srcu-cbmc/include/linux/types.h b/tools/testing/selftests/rcutorture/formal/srcu-cbmc/include/linux/types.h index 891ad13e95b2..32ce965187b3 100644 --- a/tools/testing/selftests/rcutorture/formal/srcu-cbmc/include/linux/types.h +++ b/tools/testing/selftests/rcutorture/formal/srcu-cbmc/include/linux/types.h @@ -100,12 +100,12 @@ typedef phys_addr_t resource_size_t; typedef unsigned long irq_hw_number_t; typedef struct { - int counter; + u32 __aligned(4) counter; } atomic_t; #ifdef CONFIG_64BIT typedef struct { - long counter; + u64 __aligned(8) counter; } atomic64_t; #endif
Atomic instructions require data they operate on to be aligned according to data size. I.e. 32-bit atomic values must be 32-bit aligned while 64-bit values must be 64-bit aligned. Otherwise even if CPU may handle not-aligend normal data access, still atomic instructions fail and typically raise an exception leaving us dead in the water. This came-up during lengthly discussion here: http://lists.infradead.org/pipermail/linux-snps-arc/2018-July/004022.html Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Boqun Feng <boqun.feng@gmail.com> Cc: Russell King <linux@armlinux.org.uk> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Darren Hart <dvhart@infradead.org> Cc: Shuah Khan <shuah@kernel.org> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> Cc: Josh Triplett <josh@joshtriplett.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: David Laight <David.Laight@ACULAB.COM> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- arch/arm/include/asm/atomic.h | 2 +- include/asm-generic/atomic64.h | 2 +- include/linux/types.h | 4 ++-- tools/include/linux/types.h | 2 +- tools/testing/selftests/futex/include/atomic.h | 2 +- .../rcutorture/formal/srcu-cbmc/include/linux/types.h | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-)