When using "sort -nu", arm64 syscalls were lost. That is, the
io_setup syscall (number 0) and all but one (typically
ftruncate; 64) of the syscalls that are defined symbolically
(like "#define __NR_ftruncate __NR3264_ftruncate") at the point
where "sort" is applied.
This creation-of-syscalls.c-scheme is, judging from comments,
copy-pasted from powerpc, and worked there because at the time,
its tools/arch/powerpc/include/uapi/asm/unistd.h had *literals*,
like "#define __NR_ftruncate 93".
With sort being numeric and the non-numeric key effectively
evaluating to 0, the sort option "-u" means these "duplicates"
are removed. There's no need to remove syscall lines with
duplicate numbers for arm64 because there are none, so let's fix
that by just losing the "-u". Having the table numerically
sorted on syscall-number for the rest of the syscalls looks
nice, so keep the "-n".
Signed-off-by: Hans-Peter Nilsson <[email protected]>
Cc: John Garry <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Mathieu Poirier <[email protected]>
Cc: Leo Yan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Kim Phillips <[email protected]>
---
tools/perf/arch/arm64/entry/syscalls/mksyscalltbl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl b/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl
index 459469b7222c..a7ca48d1e37b 100755
--- a/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl
+++ b/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl
@@ -58,5 +58,5 @@ create_table()
$gcc -E -dM -x c -I $incpath/include/uapi $input \
|sed -ne 's/^#define __NR_//p' \
- |sort -t' ' -k2 -nu \
+ |sort -t' ' -k2 -n \
|create_table
--
2.11.0
brgds, H-P
Hi Hans-Peter,
On Mon, Dec 28, 2020 at 03:39:41AM +0100, Hans-Peter Nilsson wrote:
> When using "sort -nu", arm64 syscalls were lost. That is, the
> io_setup syscall (number 0) and all but one (typically
> ftruncate; 64) of the syscalls that are defined symbolically
> (like "#define __NR_ftruncate __NR3264_ftruncate") at the point
> where "sort" is applied.
>
> This creation-of-syscalls.c-scheme is, judging from comments,
> copy-pasted from powerpc, and worked there because at the time,
> its tools/arch/powerpc/include/uapi/asm/unistd.h had *literals*,
> like "#define __NR_ftruncate 93".
>
> With sort being numeric and the non-numeric key effectively
> evaluating to 0, the sort option "-u" means these "duplicates"
> are removed. There's no need to remove syscall lines with
> duplicate numbers for arm64 because there are none, so let's fix
> that by just losing the "-u". Having the table numerically
> sorted on syscall-number for the rest of the syscalls looks
> nice, so keep the "-n".
>
> Signed-off-by: Hans-Peter Nilsson <[email protected]>
Very good catching! I tested this patch with the commands:
$ cd $LINUX_KERN
$ tools/perf/arch/arm64/entry/syscalls/mksyscalltbl \
$ARM64_TOOLCHAIN_PATH/aarch64-linux-gnu-gcc \
gcc tools tools/include/uapi/asm-generic/unistd.h
It gives out complete syscall tables:
$ diff /tmp/mksyscall_before.txt /tmp/mksyscall_after.txt
1a2,4
> [223] = "fadvise64",
> [25] = "fcntl",
> [44] = "fstatfs",
2a6,11
> [0] = "io_setup",
> [62] = "lseek",
> [222] = "mmap",
> [71] = "sendfile",
> [43] = "statfs",
> [45] = "truncate",
Rather than dropping option "-u" for sort command, I googled and read
the manual of "sort", but cannot find other better method. So this
patch looks good for me:
Reviewed-by: Leo Yan <[email protected]>
Tested-by: Leo Yan <[email protected]>
> Cc: John Garry <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Mathieu Poirier <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Kim Phillips <[email protected]>
> ---
> tools/perf/arch/arm64/entry/syscalls/mksyscalltbl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl b/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl
> index 459469b7222c..a7ca48d1e37b 100755
> --- a/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl
> +++ b/tools/perf/arch/arm64/entry/syscalls/mksyscalltbl
> @@ -58,5 +58,5 @@ create_table()
>
> $gcc -E -dM -x c -I $incpath/include/uapi $input \
> |sed -ne 's/^#define __NR_//p' \
> - |sort -t' ' -k2 -nu \
> + |sort -t' ' -k2 -n \
> |create_table
> --
> 2.11.0
>
> brgds, H-P
On Tue, Dec 29, 2020 at 11:09:33AM +0800, Leo Yan wrote:
> On Mon, Dec 28, 2020 at 03:39:41AM +0100, Hans-Peter Nilsson wrote:
> > When using "sort -nu", arm64 syscalls were lost. That is, the
> > io_setup syscall (number 0) and all but one (typically
> > ftruncate; 64) of the syscalls that are defined symbolically
> > (like "#define __NR_ftruncate __NR3264_ftruncate") at the point
> > where "sort" is applied.
> >
> > This creation-of-syscalls.c-scheme is, judging from comments,
> > copy-pasted from powerpc, and worked there because at the time,
> > its tools/arch/powerpc/include/uapi/asm/unistd.h had *literals*,
> > like "#define __NR_ftruncate 93".
> >
> > With sort being numeric and the non-numeric key effectively
> > evaluating to 0, the sort option "-u" means these "duplicates"
> > are removed. There's no need to remove syscall lines with
> > duplicate numbers for arm64 because there are none, so let's fix
> > that by just losing the "-u". Having the table numerically
> > sorted on syscall-number for the rest of the syscalls looks
> > nice, so keep the "-n".
> >
> > Signed-off-by: Hans-Peter Nilsson <[email protected]>
>
> Very good catching! I tested this patch with the commands:
>
> $ cd $LINUX_KERN
> $ tools/perf/arch/arm64/entry/syscalls/mksyscalltbl \
> $ARM64_TOOLCHAIN_PATH/aarch64-linux-gnu-gcc \
> gcc tools tools/include/uapi/asm-generic/unistd.h
>
> It gives out complete syscall tables:
>
> $ diff /tmp/mksyscall_before.txt /tmp/mksyscall_after.txt
> 1a2,4
> > [223] = "fadvise64",
> > [25] = "fcntl",
> > [44] = "fstatfs",
> 2a6,11
> > [0] = "io_setup",
> > [62] = "lseek",
> > [222] = "mmap",
> > [71] = "sendfile",
> > [43] = "statfs",
> > [45] = "truncate",
>
> Rather than dropping option "-u" for sort command, I googled and read
> the manual of "sort", but cannot find other better method. So this
> patch looks good for me:
>
> Reviewed-by: Leo Yan <[email protected]>
> Tested-by: Leo Yan <[email protected]>
It looks like this patch was never applied? AFAICS it is still needed
on current HEAD and it still applies cleanly.
Hi Vincent,
[ + Arnd ]
On Fri, Nov 25, 2022 at 12:53:10PM +0100, Vincent Whitchurch wrote:
> On Tue, Dec 29, 2020 at 11:09:33AM +0800, Leo Yan wrote:
> > On Mon, Dec 28, 2020 at 03:39:41AM +0100, Hans-Peter Nilsson wrote:
> > > When using "sort -nu", arm64 syscalls were lost. That is, the
> > > io_setup syscall (number 0) and all but one (typically
> > > ftruncate; 64) of the syscalls that are defined symbolically
> > > (like "#define __NR_ftruncate __NR3264_ftruncate") at the point
> > > where "sort" is applied.
> > >
> > > This creation-of-syscalls.c-scheme is, judging from comments,
> > > copy-pasted from powerpc, and worked there because at the time,
> > > its tools/arch/powerpc/include/uapi/asm/unistd.h had *literals*,
> > > like "#define __NR_ftruncate 93".
> > >
> > > With sort being numeric and the non-numeric key effectively
> > > evaluating to 0, the sort option "-u" means these "duplicates"
> > > are removed. There's no need to remove syscall lines with
> > > duplicate numbers for arm64 because there are none, so let's fix
> > > that by just losing the "-u". Having the table numerically
> > > sorted on syscall-number for the rest of the syscalls looks
> > > nice, so keep the "-n".
> > >
> > > Signed-off-by: Hans-Peter Nilsson <[email protected]>
> >
> > Very good catching! I tested this patch with the commands:
> >
> > $ cd $LINUX_KERN
> > $ tools/perf/arch/arm64/entry/syscalls/mksyscalltbl \
> > $ARM64_TOOLCHAIN_PATH/aarch64-linux-gnu-gcc \
> > gcc tools tools/include/uapi/asm-generic/unistd.h
> >
> > It gives out complete syscall tables:
> >
> > $ diff /tmp/mksyscall_before.txt /tmp/mksyscall_after.txt
> > 1a2,4
> > > [223] = "fadvise64",
> > > [25] = "fcntl",
> > > [44] = "fstatfs",
> > 2a6,11
> > > [0] = "io_setup",
> > > [62] = "lseek",
> > > [222] = "mmap",
> > > [71] = "sendfile",
> > > [43] = "statfs",
> > > [45] = "truncate",
> >
> > Rather than dropping option "-u" for sort command, I googled and read
> > the manual of "sort", but cannot find other better method. So this
> > patch looks good for me:
> >
> > Reviewed-by: Leo Yan <[email protected]>
> > Tested-by: Leo Yan <[email protected]>
>
> It looks like this patch was never applied? AFAICS it is still needed
> on current HEAD and it still applies cleanly.
Thanks a lot for bringing up this.
Before there have a discussion [1] for refactoring Arm64 system call
table but it didn't really happen. I think it's the right thing to merge
this patch, @Arnaldo, could you pick up this patch?
Thanks,
Leo
[1] https://lore.kernel.org/lkml/CAK8P3a1G2YHh2FRd=vBY4xxsJ1wZFh2pB1wyw82QmQOaiV4+CA@mail.gmail.com/
On Fri, Nov 25, 2022, at 13:54, Leo Yan wrote:
> On Fri, Nov 25, 2022 at 12:53:10PM +0100, Vincent Whitchurch wrote:
>> It looks like this patch was never applied? AFAICS it is still needed
>> on current HEAD and it still applies cleanly.
>
> Thanks a lot for bringing up this.
>
> Before there have a discussion [1] for refactoring Arm64 system call
> table but it didn't really happen.
I actually worked on this last week and did a new series to convert
the old asm-generic/unistd.h header into the syscall.tbl format,
and change arm64 to use that.
You can find my work in the 'syscall-tbl' branch of my asm-generic
tree [1]. This has only seen light build testing so far, and is
probably still buggy, but most of the work is there. The missing
bits are the Makefiles for the other seven architectures using
asm-generic/unistd.h, and checking the output to ensure the
contents are still the same.
> I think it's the right thing to merge
> this patch, @Arnaldo, could you pick up this patch?
That sounds fine to me.
Arnd
[1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git/
Em Fri, Nov 25, 2022 at 02:56:31PM +0100, Arnd Bergmann escreveu:
> On Fri, Nov 25, 2022, at 13:54, Leo Yan wrote:
> > On Fri, Nov 25, 2022 at 12:53:10PM +0100, Vincent Whitchurch wrote:
>
> >> It looks like this patch was never applied? AFAICS it is still needed
> >> on current HEAD and it still applies cleanly.
> >
> > Thanks a lot for bringing up this.
> >
> > Before there have a discussion [1] for refactoring Arm64 system call
> > table but it didn't really happen.
>
> I actually worked on this last week and did a new series to convert
> the old asm-generic/unistd.h header into the syscall.tbl format,
> and change arm64 to use that.
>
> You can find my work in the 'syscall-tbl' branch of my asm-generic
> tree [1]. This has only seen light build testing so far, and is
> probably still buggy, but most of the work is there. The missing
> bits are the Makefiles for the other seven architectures using
> asm-generic/unistd.h, and checking the output to ensure the
> contents are still the same.
>
> > I think it's the right thing to merge
> > this patch, @Arnaldo, could you pick up this patch?
>
> That sounds fine to me.
Sure, and adding an Acked-by: Arnd
> Arnd
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git/
--
- Arnaldo
Hi Arnd,
On Fri, Nov 25, 2022 at 02:56:31PM +0100, Arnd Bergmann wrote:
> On Fri, Nov 25, 2022, at 13:54, Leo Yan wrote:
> > On Fri, Nov 25, 2022 at 12:53:10PM +0100, Vincent Whitchurch wrote:
>
> >> It looks like this patch was never applied? AFAICS it is still needed
> >> on current HEAD and it still applies cleanly.
> >
> > Thanks a lot for bringing up this.
> >
> > Before there have a discussion [1] for refactoring Arm64 system call
> > table but it didn't really happen.
>
> I actually worked on this last week and did a new series to convert
> the old asm-generic/unistd.h header into the syscall.tbl format,
> and change arm64 to use that.
>
> You can find my work in the 'syscall-tbl' branch of my asm-generic
> tree [1]. This has only seen light build testing so far, and is
> probably still buggy, but most of the work is there. The missing
> bits are the Makefiles for the other seven architectures using
> asm-generic/unistd.h, and checking the output to ensure the
> contents are still the same.
Thanks a lot for sharing the patch set.
I went through the whole patch set, below are several things I observed:
- I did a quick compilation but found building failure, this failure
is caused by the first patch "arm64: convert unistd_32.h to
syscall.tbl format", it removes the macro __NR_compat_syscalls.
In file included from ./include/vdso/const.h:5,
from ./include/linux/const.h:4,
from ./arch/arm64/include/asm/alternative-macros.h:5,
from ./arch/arm64/include/asm/alternative.h:5,
from ./arch/arm64/include/asm/lse.h:15,
from ./arch/arm64/include/asm/cmpxchg.h:14,
from ./arch/arm64/include/asm/atomic.h:16,
from ./include/linux/atomic.h:7,
from ./include/linux/refcount.h:95,
from kernel/seccomp.c:18:
./arch/arm64/include/asm/seccomp.h:27:33: error: ‘__NR_compat_syscalls’ undeclared here (not in a function); did you mean ‘in_compat_syscall’?
27 | # define SECCOMP_ARCH_COMPAT_NR __NR_compat_syscalls
| ^~~~~~~~~~~~~~~~~~~~
./include/uapi/linux/const.h:34:40: note: in definition of macro ‘__KERNEL_DIV_ROUND_UP’
34 | #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
| ^
./include/linux/types.h:11:28: note: in expansion of macro ‘BITS_TO_LONGS’
11 | unsigned long name[BITS_TO_LONGS(bits)]
| ^~~~~~~~~~~~~
kernel/seccomp.c:168:9: note: in expansion of macro ‘DECLARE_BITMAP’
168 | DECLARE_BITMAP(allow_compat, SECCOMP_ARCH_COMPAT_NR);
| ^~~~~~~~~~~~~~
kernel/seccomp.c:168:38: note: in expansion of macro ‘SECCOMP_ARCH_COMPAT_NR’
168 | DECLARE_BITMAP(allow_compat, SECCOMP_ARCH_COMPAT_NR);
| ^~~~~~~~~~~~~~~~~~~~~~
- The patch set breaks git bisection, when I use "git bisect" I can
find more building failures caused by middle patches.
- The patch "arm64: generate 64-bit syscall.tbl" removes macros:
__ARCH_WANT_RENAMEAT
__ARCH_WANT_NEW_STAT
__ARCH_WANT_SET_GET_RLIMIT
__ARCH_WANT_TIME32_SYSCALLS
__ARCH_WANT_SYS_CLONE3
__ARCH_WANT_MEMFD_SECRET
Seems to me we still need to enable these macros so can enable Arm64
specific system calls?
- We also need to update the header files:
tools/include/uapi/asm-generic/unistd.h
You are welcome to CC me when you send out formal patches to mailing
list and I can test it (and look if can refine perf code for this).
Thanks,
Leo
On Mon, Dec 12, 2022, at 11:52, Leo Yan wrote:
> Hi Arnd,
>
> On Fri, Nov 25, 2022 at 02:56:31PM +0100, Arnd Bergmann wrote:
>> On Fri, Nov 25, 2022, at 13:54, Leo Yan wrote:
>> > On Fri, Nov 25, 2022 at 12:53:10PM +0100, Vincent Whitchurch wrote:
>>
>> >> It looks like this patch was never applied? AFAICS it is still needed
>> >> on current HEAD and it still applies cleanly.
>> >
>> > Thanks a lot for bringing up this.
>> >
>> > Before there have a discussion [1] for refactoring Arm64 system call
>> > table but it didn't really happen.
>>
>> I actually worked on this last week and did a new series to convert
>> the old asm-generic/unistd.h header into the syscall.tbl format,
>> and change arm64 to use that.
>>
>> You can find my work in the 'syscall-tbl' branch of my asm-generic
>> tree [1]. This has only seen light build testing so far, and is
>> probably still buggy, but most of the work is there. The missing
>> bits are the Makefiles for the other seven architectures using
>> asm-generic/unistd.h, and checking the output to ensure the
>> contents are still the same.
>
> Thanks a lot for sharing the patch set.
>
> I went through the whole patch set, below are several things I observed:
Thanks for the review!
> - I did a quick compilation but found building failure, this failure
> is caused by the first patch "arm64: convert unistd_32.h to
> syscall.tbl format", it removes the macro __NR_compat_syscalls.
>
> In file included from ./include/vdso/const.h:5,
> from ./include/linux/const.h:4,
> from ./arch/arm64/include/asm/alternative-macros.h:5,
> from ./arch/arm64/include/asm/alternative.h:5,
> from ./arch/arm64/include/asm/lse.h:15,
> from ./arch/arm64/include/asm/cmpxchg.h:14,
> from ./arch/arm64/include/asm/atomic.h:16,
> from ./include/linux/atomic.h:7,
> from ./include/linux/refcount.h:95,
> from kernel/seccomp.c:18:
> ./arch/arm64/include/asm/seccomp.h:27:33: error:
> ‘__NR_compat_syscalls’ undeclared here (not in a function); did you
> mean ‘in_compat_syscall’?
> 27 | # define SECCOMP_ARCH_COMPAT_NR __NR_compat_syscalls
> | ^~~~~~~~~~~~~~~~~~~~
> ./include/uapi/linux/const.h:34:40: note: in definition of macro
> ‘__KERNEL_DIV_ROUND_UP’
> 34 | #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> | ^
> ./include/linux/types.h:11:28: note: in expansion of macro
> ‘BITS_TO_LONGS’
> 11 | unsigned long name[BITS_TO_LONGS(bits)]
> | ^~~~~~~~~~~~~
> kernel/seccomp.c:168:9: note: in expansion of macro ‘DECLARE_BITMAP’
> 168 | DECLARE_BITMAP(allow_compat, SECCOMP_ARCH_COMPAT_NR);
> | ^~~~~~~~~~~~~~
> kernel/seccomp.c:168:38: note: in expansion of macro
> ‘SECCOMP_ARCH_COMPAT_NR’
> 168 | DECLARE_BITMAP(allow_compat, SECCOMP_ARCH_COMPAT_NR);
> | ^~~~~~~~~~~~~~~~~~~~~~
Sorry about that, I thought I had fixed it already. I'll need to
get back to the series and make sure this works.
> - The patch set breaks git bisection, when I use "git bisect" I can
> find more building failures caused by middle patches.
Ok, I'll check these out as well.
> - The patch "arm64: generate 64-bit syscall.tbl" removes macros:
>
> __ARCH_WANT_RENAMEAT
> __ARCH_WANT_NEW_STAT
> __ARCH_WANT_SET_GET_RLIMIT
> __ARCH_WANT_TIME32_SYSCALLS
> __ARCH_WANT_SYS_CLONE3
> __ARCH_WANT_MEMFD_SECRET
>
> Seems to me we still need to enable these macros so can enable Arm64
> specific system calls?
The system call selection is now controlled with the line
ABIS_unistd64.h := common,64,renameat,newstat,rlimit,mmu,clone3,memfd_secret
which picks the types of ABIs that are enabled when converting
the syscall.tbl file into the headers. Removing these was
intentional, but I now see that we still need to define __ARCH_WANT_NEW_STAT
and __ARCH_WANT_SYS_CLONE3 because those two are referenced not
just in unistd.h but also in fs/stat.c and kernel/fork.c.
I'll put this on the list of things to check for the other architectures,
in addition to comparing the resulting syscall table object and the
generated asm/unistd.h.
> - We also need to update the header files:
> tools/include/uapi/asm-generic/unistd.h
>
> You are welcome to CC me when you send out formal patches to mailing
> list and I can test it (and look if can refine perf code for this).
Ok, thanks again.
Arnd
On Wed, Dec 21, 2022, at 21:15, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 25, 2022 at 02:56:31PM +0100, Arnd Bergmann escreveu:
>> On Fri, Nov 25, 2022, at 13:54, Leo Yan wrote:
>> > I think it's the right thing to merge
>> > this patch, @Arnaldo, could you pick up this patch?
>
> So I'm adding the provided:
>
> Reviewed-by: Leo Yan <[email protected]>
> Tested-by: Leo Yan <[email protected]>
>
>
>> That sounds fine to me.
>
> And taking this as an:
>
> Acked-by: Arnd Bergmann <[email protected]>
>
> ok?
Yes, sounds good.
Arnd
Em Fri, Nov 25, 2022 at 02:56:31PM +0100, Arnd Bergmann escreveu:
> On Fri, Nov 25, 2022, at 13:54, Leo Yan wrote:
> > On Fri, Nov 25, 2022 at 12:53:10PM +0100, Vincent Whitchurch wrote:
>
> >> It looks like this patch was never applied? AFAICS it is still needed
> >> on current HEAD and it still applies cleanly.
> >
> > Thanks a lot for bringing up this.
> >
> > Before there have a discussion [1] for refactoring Arm64 system call
> > table but it didn't really happen.
>
> I actually worked on this last week and did a new series to convert
> the old asm-generic/unistd.h header into the syscall.tbl format,
> and change arm64 to use that.
>
> You can find my work in the 'syscall-tbl' branch of my asm-generic
> tree [1]. This has only seen light build testing so far, and is
> probably still buggy, but most of the work is there. The missing
> bits are the Makefiles for the other seven architectures using
> asm-generic/unistd.h, and checking the output to ensure the
> contents are still the same.
>
> > I think it's the right thing to merge
> > this patch, @Arnaldo, could you pick up this patch?
So I'm adding the provided:
Reviewed-by: Leo Yan <[email protected]>
Tested-by: Leo Yan <[email protected]>
> That sounds fine to me.
And taking this as an:
Acked-by: Arnd Bergmann <[email protected]>
ok?
- Arnaldo
> Arnd
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git/