* [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-03 19:53 Palmer Dabbelt
2018-08-04 8:54 ` Christoph Hellwig
2018-08-09 13:26 ` Guenter Roeck
0 siblings, 2 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2018-08-03 19:53 UTC (permalink / raw)
To: linux-riscv
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/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)
--
2.16.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
2018-08-03 19:53 [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h Palmer Dabbelt
@ 2018-08-04 8:54 ` Christoph Hellwig
2018-08-09 13:26 ` Guenter Roeck
1 sibling, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-04 8:54 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2018-08-04 8:54 UTC (permalink / raw)
To: linux-riscv
> 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] 21+ messages in thread
* Re: RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
2018-08-04 8:54 ` Christoph Hellwig
@ 2018-08-06 20:42 ` Palmer Dabbelt
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-06 20:42 ` Palmer Dabbelt
0 siblings, 0 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2018-08-06 20:42 UTC (permalink / raw)
To: linux-riscv
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] 21+ 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
-1 siblings, 0 replies; 21+ 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] 21+ 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
0 siblings, 0 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2018-08-06 20:42 UTC (permalink / raw)
To: linux-riscv
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] 21+ 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 ` Palmer Dabbelt
@ 2018-08-06 21:00 ` Randy Dunlap
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-06 21:00 ` Randy Dunlap
0 siblings, 0 replies; 21+ messages in thread
From: Randy Dunlap @ 2018-08-06 21:00 UTC (permalink / raw)
To: linux-riscv
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] 21+ 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
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-07 0:11 ` Palmer Dabbelt
0 siblings, 0 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2018-08-07 0:11 UTC (permalink / raw)
To: linux-riscv
On Mon, 06 Aug 2018 14:00:53 PDT (-0700), rdunlap at 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] 21+ 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 ` Palmer Dabbelt
@ 2018-08-07 12:45 ` Christoph Hellwig
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-07 12:45 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2018-08-07 12:45 UTC (permalink / raw)
To: linux-riscv
Looks fine (modulo any typos):
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 21+ 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 ` Palmer Dabbelt
@ 2018-08-09 5:58 ` Christoph Hellwig
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-09 5:58 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2018-08-09 5:58 UTC (permalink / raw)
To: linux-riscv
This actually seems to break the compilation for me in for-next:
hch at 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] 21+ messages in thread
* Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
2018-08-03 19:53 [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h Palmer Dabbelt
@ 2018-08-09 13:26 ` Guenter Roeck
2018-08-09 13:26 ` Guenter Roeck
1 sibling, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-09 13:26 ` Guenter Roeck
0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2018-08-09 13:26 UTC (permalink / raw)
To: linux-riscv
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] 21+ 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
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH v2] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-09 20:58 ` Palmer Dabbelt
0 siblings, 0 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2018-08-09 20:58 UTC (permalink / raw)
To: linux-riscv
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 at 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] 21+ messages in thread
* Re: [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
2018-08-09 13:26 ` Guenter Roeck
@ 2018-08-09 20:58 ` Palmer Dabbelt
-1 siblings, 0 replies; 21+ 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] 21+ messages in thread
* [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h
@ 2018-08-09 20:58 ` Palmer Dabbelt
0 siblings, 0 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2018-08-09 20:58 UTC (permalink / raw)
To: linux-riscv
On Thu, 09 Aug 2018 06:26:12 PDT (-0700), linux at 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] 21+ messages in thread
end of thread, other threads:[~2018-08-09 20:58 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 19:53 [PATCH] RISC-V: Don't use a global include guard for uapi/asm/syscalls.h Palmer Dabbelt
2018-08-04 8:54 ` Christoph Hellwig
2018-08-04 8:54 ` Christoph Hellwig
2018-08-06 20:42 ` Palmer Dabbelt
2018-08-06 20:42 ` Palmer Dabbelt
2018-08-06 20:42 ` [PATCH v2] " Palmer Dabbelt
2018-08-06 20:42 ` Palmer Dabbelt
2018-08-06 21:00 ` Randy Dunlap
2018-08-06 21:00 ` Randy Dunlap
2018-08-07 0:11 ` Palmer Dabbelt
2018-08-07 0:11 ` Palmer Dabbelt
2018-08-07 12:45 ` Christoph Hellwig
2018-08-07 12:45 ` Christoph Hellwig
2018-08-09 5:58 ` Christoph Hellwig
2018-08-09 5:58 ` Christoph Hellwig
2018-08-09 20:58 ` Palmer Dabbelt
2018-08-09 20:58 ` Palmer Dabbelt
2018-08-09 13:26 ` [PATCH] " Guenter Roeck
2018-08-09 13:26 ` Guenter Roeck
2018-08-09 20:58 ` Palmer Dabbelt
2018-08-09 20:58 ` Palmer Dabbelt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.