2019-08-10 01:14:54

by Alistair Francis

[permalink] [raw]
Subject: [PATCH] syscalls: Update the syscall #defines to match uapi

Update the #defines around sys_fstat64() and sys_fstatat64() to match
the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
one is defined.

Signed-off-by: Alistair Francis <[email protected]>
---
include/linux/syscalls.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2bcef4c70183..e4bf5e480d60 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
struct stat __user *statbuf, int flag);
asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
-#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
+#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
struct stat64 __user *statbuf, int flag);
--
2.22.0


2019-08-12 00:01:46

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH] syscalls: Update the syscall #defines to match uapi

On Fri, Aug 9, 2019 at 6:11 PM Alistair Francis
<[email protected]> wrote:
>
> Update the #defines around sys_fstat64() and sys_fstatat64() to match
> the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> one is defined.

What do you mean by "if one is defined"?

The patch seems like it's probably okay, but I can't understand the changelog.

--Andy

2019-08-12 03:05:39

by Alistair Francis

[permalink] [raw]
Subject: Re: [PATCH] syscalls: Update the syscall #defines to match uapi

On Sun, Aug 11, 2019 at 5:00 PM Andy Lutomirski <[email protected]> wrote:
>
> On Fri, Aug 9, 2019 at 6:11 PM Alistair Francis
> <[email protected]> wrote:
> >
> > Update the #defines around sys_fstat64() and sys_fstatat64() to match
> > the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> > in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> > one is defined.
>
> What do you mean by "if one is defined"?

Yeah, I guess that isn't clear. What I mean is that if
__ARCH_WANT_NEW_STAT is defined but __ARCH_WANT_STAT64 isn't currently
it will fail to build.

>
> The patch seems like it's probably okay, but I can't understand the changelog.

I can send a v2 with an updated commit message.

Alistair

>
> --Andy

2019-08-12 09:50:04

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] syscalls: Update the syscall #defines to match uapi

On Sat, Aug 10, 2019 at 3:11 AM Alistair Francis
<[email protected]> wrote:
>
> Update the #defines around sys_fstat64() and sys_fstatat64() to match
> the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> one is defined.

What is the compiler failure you get?

> Signed-off-by: Alistair Francis <[email protected]>
> ---
> include/linux/syscalls.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 2bcef4c70183..e4bf5e480d60 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> struct stat __user *statbuf, int flag);
> asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> struct stat64 __user *statbuf, int flag);

I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
on a 64-bit architecture and only want the sys_newfstat{,at} system
calls, not sys_fstat{,at}64 that gets used on 32-bit machines.

The #if check in the syscalls.h file also matches the definition of
the function.

Arnd

2019-08-13 19:05:09

by Alistair Francis

[permalink] [raw]
Subject: Re: [PATCH] syscalls: Update the syscall #defines to match uapi

On Mon, Aug 12, 2019 at 2:49 AM Arnd Bergmann <[email protected]> wrote:
>
> On Sat, Aug 10, 2019 at 3:11 AM Alistair Francis
> <[email protected]> wrote:
> >
> > Update the #defines around sys_fstat64() and sys_fstatat64() to match
> > the #defines around the __NR3264_fstatat and __NR3264_fstat definitions
> > in include/uapi/asm-generic/unistd.h. This avoids compiler failures if
> > one is defined.
>
> What is the compiler failure you get?

I don't have it infornt of me but it was along the lines of
sys_fstat64/sys_fstatat64 not being defined when __ARCH_WANT_NEW_STAT
is defined but __ARCH_WANT_STAT64 isn't.

>
> > Signed-off-by: Alistair Francis <[email protected]>
> > ---
> > include/linux/syscalls.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > index 2bcef4c70183..e4bf5e480d60 100644
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
> > @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> > asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> > struct stat __user *statbuf, int flag);
> > asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> > -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> > +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> > asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> > asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> > struct stat64 __user *statbuf, int flag);
>
> I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
> on a 64-bit architecture and only want the sys_newfstat{,at} system
> calls, not sys_fstat{,at}64 that gets used on 32-bit machines.

Ah, that would make sense then. I don't think you will see the error then.

Alistair

>
> The #if check in the syscalls.h file also matches the definition of
> the function.
>
> Arnd

2019-08-13 19:42:53

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] syscalls: Update the syscall #defines to match uapi

On Tue, Aug 13, 2019 at 9:01 PM Alistair Francis <[email protected]> wrote:
> On Mon, Aug 12, 2019 at 2:49 AM Arnd Bergmann <[email protected]> wrote:

> > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > > index 2bcef4c70183..e4bf5e480d60 100644
> > > --- a/include/linux/syscalls.h
> > > +++ b/include/linux/syscalls.h
> > > @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> > > asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> > > struct stat __user *statbuf, int flag);
> > > asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> > > -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> > > +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> > > asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> > > asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> > > struct stat64 __user *statbuf, int flag);
> >
> > I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
> > on a 64-bit architecture and only want the sys_newfstat{,at} system
> > calls, not sys_fstat{,at}64 that gets used on 32-bit machines.
>
> Ah, that would make sense then. I don't think you will see the error then.

So we don't need this patch to build riscv32 kernels, right? It's possible
that it was the result of an incorrect forward port of some other patch,
as older riscv32 kernels did provide stat64(), but newer ones only have
statx().

Arnd

2019-08-13 22:09:38

by Alistair Francis

[permalink] [raw]
Subject: Re: [PATCH] syscalls: Update the syscall #defines to match uapi

On Tue, Aug 13, 2019 at 12:41 PM Arnd Bergmann <[email protected]> wrote:
>
> On Tue, Aug 13, 2019 at 9:01 PM Alistair Francis <[email protected]> wrote:
> > On Mon, Aug 12, 2019 at 2:49 AM Arnd Bergmann <[email protected]> wrote:
>
> > > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> > > > index 2bcef4c70183..e4bf5e480d60 100644
> > > > --- a/include/linux/syscalls.h
> > > > +++ b/include/linux/syscalls.h
> > > > @@ -512,7 +512,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *bu
> > > > asmlinkage long sys_newfstatat(int dfd, const char __user *filename,
> > > > struct stat __user *statbuf, int flag);
> > > > asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf);
> > > > -#if defined(__ARCH_WANT_STAT64) || defined(__ARCH_WANT_COMPAT_STAT64)
> > > > +#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
> > > > asmlinkage long sys_fstat64(unsigned long fd, struct stat64 __user *statbuf);
> > > > asmlinkage long sys_fstatat64(int dfd, const char __user *filename,
> > > > struct stat64 __user *statbuf, int flag);
> > >
> > > I think this is wrong: when __ARCH_WANT_NEW_STAT is set, we are
> > > on a 64-bit architecture and only want the sys_newfstat{,at} system
> > > calls, not sys_fstat{,at}64 that gets used on 32-bit machines.
> >
> > Ah, that would make sense then. I don't think you will see the error then.
>
> So we don't need this patch to build riscv32 kernels, right? It's possible
> that it was the result of an incorrect forward port of some other patch,
> as older riscv32 kernels did provide stat64(), but newer ones only have
> statx().

The issue came up when I was just changing some things for testing and
I thought it was a bug that others might run into. It isn't directly
related to the riscv32 kernel.

Alistair

>
> Arnd