* Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h [not found] <20180803195344.22271-1-palmer@sifive.com> @ 2018-08-04 8:54 ` Christoph Hellwig 2018-08-06 20:42 ` Palmer Dabbelt 2018-08-09 13:26 ` [PATCH] " Guenter Roeck 1 sibling, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2018-08-04 8:54 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: linux-riscv, Marcus Comstedt, linux-kernel, aou > index 818655b0d535..882a6aa09a33 100644 > --- a/arch/riscv/include/uapi/asm/syscalls.h > +++ b/arch/riscv/include/uapi/asm/syscalls.h > @@ -1,10 +1,11 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > +// SPDX-License-Identifier: GPL-2.0 /* */ is the required style for headers, // is only for other files. > +/* There is explicitly no include guard here because this file is expected to > + * be included multiple times in order to define the syscall macros via > + * __SYSCALL. */ Normal Linux comment style would be: /* * There is explicitly no include guard here because this file is expected to * be included multiple times in order to define the syscall macros via * __SYSCALL. */ Also syscalls.h isn't included directly anywhere, but through <asm/unistd.h>, so we'll probably need a similar comment there as well. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-04 8:54 ` [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h Christoph Hellwig @ 2018-08-06 20:42 ` Palmer Dabbelt 2018-08-06 20:42 ` [PATCH v2] " Palmer Dabbelt 0 siblings, 1 reply; 10+ messages in thread From: Palmer Dabbelt @ 2018-08-06 20:42 UTC (permalink / raw) To: linux-riscv, Christoph Hellwig Cc: Palmer Dabbelt, aou, Arnd Bergmann, tklauser, linux-kernel On Sat, 04 Aug 2018 01:54:38 PDT (-0700), Christoph Hellwig wrote: >> index 818655b0d535..882a6aa09a33 100644 >> --- a/arch/riscv/include/uapi/asm/syscalls.h >> +++ b/arch/riscv/include/uapi/asm/syscalls.h >> @@ -1,10 +1,11 @@ >> -/* SPDX-License-Identifier: GPL-2.0 */ >> +// SPDX-License-Identifier: GPL-2.0 > > /* */ is the required style for headers, // is only for other files. > >> +/* There is explicitly no include guard here because this file is expected to >> + * be included multiple times in order to define the syscall macros via >> + * __SYSCALL. */ > > Normal Linux comment style would be: > > /* > * There is explicitly no include guard here because this file is expected to > * be included multiple times in order to define the syscall macros via > * __SYSCALL. > */ > > Also syscalls.h isn't included directly anywhere, but through > <asm/unistd.h>, so we'll probably need a similar comment there as well. I've attached a v2 with both of these fixed. Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-06 20:42 ` Palmer Dabbelt @ 2018-08-06 20:42 ` Palmer Dabbelt 2018-08-06 21:00 ` Randy Dunlap ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Palmer Dabbelt @ 2018-08-06 20:42 UTC (permalink / raw) To: linux-riscv, Christoph Hellwig Cc: Palmer Dabbelt, aou, Arnd Bergmann, tklauser, linux-kernel, Marcus Comstedt This file is expected to be included multiple times in the same file in order to allow the __SYSCALL macro to generate system call tables. With a global include guard we end up missing __NR_riscv_flush_icache in the syscall table, which results in icache flushes that escape the vDSO call to not actually do anything. The fix is to move to per-#define include guards, which allows the system call tables to actually be populated. Thanks to Macrus Comstedt for finding and fixing the bug! I also went ahead and fixed the SPDX header to use a //-style comment, which I've been told is the canonical way to do it. Cc: Marcus Comstedt <marcus@mc.pp.se> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> --- arch/riscv/include/asm/unistd.h | 5 +++++ arch/riscv/include/uapi/asm/syscalls.h | 15 +++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h index 080fb28061de..508be1780323 100644 --- a/arch/riscv/include/asm/unistd.h +++ b/arch/riscv/include/asm/unistd.h @@ -11,6 +11,11 @@ * GNU General Public License for more details. */ +/* + * There is explicitly no include guard here because this file is expected to + * included multiple times. See uapi/asm/syscalls.h for more info. + */ + #define __ARCH_WANT_SYS_CLONE #include <uapi/asm/unistd.h> #include <uapi/asm/syscalls.h> diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h index 818655b0d535..690beb002d1d 100644 --- a/arch/riscv/include/uapi/asm/syscalls.h +++ b/arch/riscv/include/uapi/asm/syscalls.h @@ -1,10 +1,13 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +// SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2017 SiFive + * Copyright (C) 2017-2018 SiFive */ -#ifndef _ASM__UAPI__SYSCALLS_H -#define _ASM__UAPI__SYSCALLS_H +/* + * There is explicitly no include guard here because this file is expected to + * be included multiple times in order to define the syscall macros via + * __SYSCALL. + */ /* * Allows the instruction cache to be flushed from userspace. Despite RISC-V @@ -20,7 +23,7 @@ * caller. We don't currently do anything with the address range, that's just * in there for forwards compatibility. */ +#ifndef __NR_riscv_flush_icache #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15) -__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) - #endif +__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) -- 2.16.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-06 20:42 ` [PATCH v2] " Palmer Dabbelt @ 2018-08-06 21:00 ` Randy Dunlap 2018-08-07 0:11 ` Palmer Dabbelt 2018-08-07 12:45 ` Christoph Hellwig 2018-08-09 5:58 ` Christoph Hellwig 2 siblings, 1 reply; 10+ messages in thread From: Randy Dunlap @ 2018-08-06 21:00 UTC (permalink / raw) To: Palmer Dabbelt, linux-riscv, Christoph Hellwig Cc: aou, Arnd Bergmann, tklauser, linux-kernel, Marcus Comstedt On 08/06/2018 01:42 PM, Palmer Dabbelt wrote: > This file is expected to be included multiple times in the same file in > order to allow the __SYSCALL macro to generate system call tables. With > a global include guard we end up missing __NR_riscv_flush_icache in the > syscall table, which results in icache flushes that escape the vDSO call > to not actually do anything. > > The fix is to move to per-#define include guards, which allows the > system call tables to actually be populated. Thanks to Macrus Comstedt > for finding and fixing the bug! > > I also went ahead and fixed the SPDX header to use a //-style comment, > which I've been told is the canonical way to do it. > > Cc: Marcus Comstedt <marcus@mc.pp.se> > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > --- > arch/riscv/include/asm/unistd.h | 5 +++++ > arch/riscv/include/uapi/asm/syscalls.h | 15 +++++++++------ > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h > index 080fb28061de..508be1780323 100644 > --- a/arch/riscv/include/asm/unistd.h > +++ b/arch/riscv/include/asm/unistd.h > @@ -11,6 +11,11 @@ > * GNU General Public License for more details. > */ > > +/* > + * There is explicitly no include guard here because this file is expected to to .. be .. included > + * included multiple times. See uapi/asm/syscalls.h for more info. > + */ > + > #define __ARCH_WANT_SYS_CLONE > #include <uapi/asm/unistd.h> > #include <uapi/asm/syscalls.h> > diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h > index 818655b0d535..690beb002d1d 100644 > --- a/arch/riscv/include/uapi/asm/syscalls.h > +++ b/arch/riscv/include/uapi/asm/syscalls.h > @@ -1,10 +1,13 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > +// SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) 2017 SiFive > + * Copyright (C) 2017-2018 SiFive > */ > > -#ifndef _ASM__UAPI__SYSCALLS_H > -#define _ASM__UAPI__SYSCALLS_H > +/* > + * There is explicitly no include guard here because this file is expected to > + * be included multiple times in order to define the syscall macros via like that one :) > + * __SYSCALL. > + */ > > /* > * Allows the instruction cache to be flushed from userspace. Despite RISC-V -- ~Randy ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-06 21:00 ` Randy Dunlap @ 2018-08-07 0:11 ` Palmer Dabbelt 0 siblings, 0 replies; 10+ messages in thread From: Palmer Dabbelt @ 2018-08-07 0:11 UTC (permalink / raw) To: rdunlap Cc: linux-riscv, Christoph Hellwig, aou, Arnd Bergmann, tklauser, linux-kernel, marcus On Mon, 06 Aug 2018 14:00:53 PDT (-0700), rdunlap@infradead.org wrote: > On 08/06/2018 01:42 PM, Palmer Dabbelt wrote: >> This file is expected to be included multiple times in the same file in >> order to allow the __SYSCALL macro to generate system call tables. With >> a global include guard we end up missing __NR_riscv_flush_icache in the >> syscall table, which results in icache flushes that escape the vDSO call >> to not actually do anything. >> >> The fix is to move to per-#define include guards, which allows the >> system call tables to actually be populated. Thanks to Macrus Comstedt >> for finding and fixing the bug! >> >> I also went ahead and fixed the SPDX header to use a //-style comment, >> which I've been told is the canonical way to do it. >> >> Cc: Marcus Comstedt <marcus@mc.pp.se> >> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> >> --- >> arch/riscv/include/asm/unistd.h | 5 +++++ >> arch/riscv/include/uapi/asm/syscalls.h | 15 +++++++++------ >> 2 files changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h >> index 080fb28061de..508be1780323 100644 >> --- a/arch/riscv/include/asm/unistd.h >> +++ b/arch/riscv/include/asm/unistd.h >> @@ -11,6 +11,11 @@ >> * GNU General Public License for more details. >> */ >> >> +/* >> + * There is explicitly no include guard here because this file is expected to > > to .. be .. included Thanks. I'm not going to spin a v3 unless there's more feedback, but I'll include it in the PR. > >> + * included multiple times. See uapi/asm/syscalls.h for more info. >> + */ >> + >> #define __ARCH_WANT_SYS_CLONE >> #include <uapi/asm/unistd.h> >> #include <uapi/asm/syscalls.h> >> diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h >> index 818655b0d535..690beb002d1d 100644 >> --- a/arch/riscv/include/uapi/asm/syscalls.h >> +++ b/arch/riscv/include/uapi/asm/syscalls.h >> @@ -1,10 +1,13 @@ >> -/* SPDX-License-Identifier: GPL-2.0 */ >> +// SPDX-License-Identifier: GPL-2.0 >> /* >> - * Copyright (C) 2017 SiFive >> + * Copyright (C) 2017-2018 SiFive >> */ >> >> -#ifndef _ASM__UAPI__SYSCALLS_H >> -#define _ASM__UAPI__SYSCALLS_H >> +/* >> + * There is explicitly no include guard here because this file is expected to >> + * be included multiple times in order to define the syscall macros via > > like that one :) > >> + * __SYSCALL. >> + */ >> >> /* >> * Allows the instruction cache to be flushed from userspace. Despite RISC-V ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-06 20:42 ` [PATCH v2] " Palmer Dabbelt 2018-08-06 21:00 ` Randy Dunlap @ 2018-08-07 12:45 ` Christoph Hellwig 2018-08-09 5:58 ` Christoph Hellwig 2 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2018-08-07 12:45 UTC (permalink / raw) To: Palmer Dabbelt Cc: linux-riscv, Christoph Hellwig, aou, Arnd Bergmann, tklauser, linux-kernel, Marcus Comstedt Looks fine (modulo any typos): Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-06 20:42 ` [PATCH v2] " Palmer Dabbelt 2018-08-06 21:00 ` Randy Dunlap 2018-08-07 12:45 ` Christoph Hellwig @ 2018-08-09 5:58 ` Christoph Hellwig 2018-08-09 20:58 ` Palmer Dabbelt 2 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2018-08-09 5:58 UTC (permalink / raw) To: Palmer Dabbelt Cc: linux-riscv, Christoph Hellwig, aou, Arnd Bergmann, linux-kernel, tklauser, Marcus Comstedt This actually seems to break the compilation for me in for-next: hch@carbon:~/work/linux$ make ARCH=riscv CALL scripts/checksyscalls.sh <stdin>:1335:2: warning: #warning syscall rseq not implemented [-Wcpp] CHK include/generated/compile.h CC arch/riscv/kernel/syscall_table.o ./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: 'sys_riscv_flush_icache' undeclared here (not in a function); did you mean '__NR_riscv_flush_icache'? __SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) ^ arch/riscv/kernel/syscall_table.c:21:37: note: in definition of macro '__SYSCALL' #define __SYSCALL(nr, call) [nr] = (call), ^~~~ make[1]: *** [scripts/Makefile.build:318: arch/riscv/kernel/syscall_table.o] Error 1 make: *** [Makefile:1029: arch/riscv/kernel] Error 2 Reverting it for now, but I guess this might hit others too.. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-09 5:58 ` Christoph Hellwig @ 2018-08-09 20:58 ` Palmer Dabbelt 0 siblings, 0 replies; 10+ messages in thread From: Palmer Dabbelt @ 2018-08-09 20:58 UTC (permalink / raw) To: Christoph Hellwig Cc: linux-riscv, Christoph Hellwig, aou, Arnd Bergmann, linux-kernel, tklauser, marcus On Wed, 08 Aug 2018 22:58:30 PDT (-0700), Christoph Hellwig wrote: > This actually seems to break the compilation for me in for-next: > > hch@carbon:~/work/linux$ make ARCH=riscv > CALL scripts/checksyscalls.sh > <stdin>:1335:2: warning: #warning syscall rseq not implemented [-Wcpp] > CHK include/generated/compile.h > CC arch/riscv/kernel/syscall_table.o > ./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: 'sys_riscv_flush_icache' undeclared here (not in a function); did you mean '__NR_riscv_flush_icache'? > __SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) > ^ > arch/riscv/kernel/syscall_table.c:21:37: note: in definition of macro '__SYSCALL' > #define __SYSCALL(nr, call) [nr] = (call), > ^~~~ > make[1]: *** [scripts/Makefile.build:318: arch/riscv/kernel/syscall_table.o] Error 1 > make: *** [Makefile:1029: arch/riscv/kernel] Error 2 > > Reverting it for now, but I guess this might hit others too.. Thanks. I dropped this from for-next and just sent out a replacement. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h [not found] <20180803195344.22271-1-palmer@sifive.com> 2018-08-04 8:54 ` [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h Christoph Hellwig @ 2018-08-09 13:26 ` Guenter Roeck 2018-08-09 20:58 ` Palmer Dabbelt 1 sibling, 1 reply; 10+ messages in thread From: Guenter Roeck @ 2018-08-09 13:26 UTC (permalink / raw) To: Palmer Dabbelt; +Cc: linux-riscv, aou, linux-kernel, Marcus Comstedt On Fri, Aug 03, 2018 at 12:53:44PM -0700, Palmer Dabbelt wrote: > This file is expected to be included multiple times in the same file in > order to allow the __SYSCALL macro to generate system call tables. With > a global include guard we end up missing __NR_riscv_flush_icache in the > syscall table, which results in icache flushes that escape the vDSO call > to not actually do anything. > > The fix is to move to per-#define include guards, which allows the > system call tables to actually be populated. Thanks to Macrus Comstedt > for finding and fixing the bug! > > I also went ahead and fixed the SPDX header to use a //-style comment, > which I've been told is the canonical way to do it. > > Cc: Marcus Comstedt <marcus@mc.pp.se> > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> Fails to build riscv:allnoconfig. CC arch/riscv/kernel/syscall_table.o ./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: ‘sys_riscv_flush_icache’ undeclared here (not in a function); did you mean ‘__NR_riscv_flush_icache’? Guenter > --- > arch/riscv/include/uapi/asm/syscalls.h | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/syscalls.h > index 818655b0d535..882a6aa09a33 100644 > --- a/arch/riscv/include/uapi/asm/syscalls.h > +++ b/arch/riscv/include/uapi/asm/syscalls.h > @@ -1,10 +1,11 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > +// SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) 2017 SiFive > + * Copyright (C) 2017-2018 SiFive > */ > > -#ifndef _ASM__UAPI__SYSCALLS_H > -#define _ASM__UAPI__SYSCALLS_H > +/* There is explicitly no include guard here because this file is expected to > + * be included multiple times in order to define the syscall macros via > + * __SYSCALL. */ > > /* > * Allows the instruction cache to be flushed from userspace. Despite RISC-V > @@ -20,7 +21,7 @@ > * caller. We don't currently do anything with the address range, that's just > * in there for forwards compatibility. > */ > +#ifndef __NR_riscv_flush_icache > #define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15) > -__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) > - > #endif > +__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h 2018-08-09 13:26 ` [PATCH] " Guenter Roeck @ 2018-08-09 20:58 ` Palmer Dabbelt 0 siblings, 0 replies; 10+ messages in thread From: Palmer Dabbelt @ 2018-08-09 20:58 UTC (permalink / raw) To: linux; +Cc: linux-riscv, aou, linux-kernel, marcus On Thu, 09 Aug 2018 06:26:12 PDT (-0700), linux@roeck-us.net wrote: > On Fri, Aug 03, 2018 at 12:53:44PM -0700, Palmer Dabbelt wrote: >> This file is expected to be included multiple times in the same file in >> order to allow the __SYSCALL macro to generate system call tables. With >> a global include guard we end up missing __NR_riscv_flush_icache in the >> syscall table, which results in icache flushes that escape the vDSO call >> to not actually do anything. >> >> The fix is to move to per-#define include guards, which allows the >> system call tables to actually be populated. Thanks to Macrus Comstedt >> for finding and fixing the bug! >> >> I also went ahead and fixed the SPDX header to use a //-style comment, >> which I've been told is the canonical way to do it. >> >> Cc: Marcus Comstedt <marcus@mc.pp.se> >> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > > Fails to build riscv:allnoconfig. > > CC arch/riscv/kernel/syscall_table.o > ./arch/riscv/include/uapi/asm/syscalls.h:29:36: error: > ‘sys_riscv_flush_icache’ undeclared here (not in a function); did you mean ‘__NR_riscv_flush_icache’? Thanks. I added you to another patch set. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-08-09 20:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20180803195344.22271-1-palmer@sifive.com>
2018-08-04 8:54 ` [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h Christoph Hellwig
2018-08-06 20:42 ` Palmer Dabbelt
2018-08-06 20:42 ` [PATCH v2] " Palmer Dabbelt
2018-08-06 21:00 ` Randy Dunlap
2018-08-07 0:11 ` Palmer Dabbelt
2018-08-07 12:45 ` Christoph Hellwig
2018-08-09 5:58 ` Christoph Hellwig
2018-08-09 20:58 ` Palmer Dabbelt
2018-08-09 13:26 ` [PATCH] " Guenter Roeck
2018-08-09 20:58 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).