Re: [PATCH] asm-generic: unistd.h: make 'compat_sys_fadvise64_64' conditional

[Arnd Bergmann <arnd@arndb.de>]

On Sun, Aug 7, 2022 at 7:28 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Don't require 'compat_sys_fadvise64_64' when
> __ARCH_WANT_COMPAT_FADVISE64_64 is not set.
>
> Fixes this build error when CONFIG_ADVISE_SYSCALLS is not set:
>
> include/uapi/asm-generic/unistd.h:649:49: error: 'compat_sys_fadvise64_64' undeclared here (not in a function); did you mean 'ksys_fadvise64_64'?
>   649 | __SC_COMP(__NR3264_fadvise64, sys_fadvise64_64, compat_sys_fadvise64_64)
> arch/riscv/kernel/compat_syscall_table.c:12:42: note: in definition of macro '__SYSCALL'
>    12 | #define __SYSCALL(nr, call)      [nr] = (call),
> include/uapi/asm-generic/unistd.h:649:1: note: in expansion of macro '__SC_COMP'
>   649 | __SC_COMP(__NR3264_fadvise64, sys_fadvise64_64, compat_sys_fadvise64_64)
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: linux-riscv@lists.infradead.org
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-arch@vger.kernel.org
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> ---
>  include/uapi/asm-generic/unistd.h |    2 ++
>  1 file changed, 2 insertions(+)
>
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -645,8 +645,10 @@ __SC_COMP(__NR_execve, sys_execve, compa
>  #define __NR3264_mmap 222
>  __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
>  /* mm/fadvise.c */
> +#ifdef __ARCH_WANT_COMPAT_FADVISE64_64
>  #define __NR3264_fadvise64 223
>  __SC_COMP(__NR3264_fadvise64, sys_fadvise64_64, compat_sys_fadvise64_64)
> +#endif
>

This does not work: __ARCH_WANT_COMPAT_FADVISE64_64 is defined in
arch/riscv/include/asm/unistd.h, which is not a UAPI header. By making the line
conditional on this, user space no longer sees the macro definition.

It looks like you also drop the native definition on all architectures other
than riscv here. What we probably want is to just make all the
declarations in include/linux/compat.h unconditional and not have them
depend on architecture specific macros. Some of these may have
incompatible prototypes depending on the architecture, but if we run
into those, I would suggest we just give them unique names.

       Arnd