All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "André Almeida" <andrealmeid@igalia.com>
Cc: "Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	linux-s390@vger.kernel.org
Subject: Re: [PATCH v6 03/13] scripts/syscall.tbl: Use the common table
Date: Fri, 28 Aug 2026 23:32:40 +0000	[thread overview]
Message-ID: <20260828233241.1DAA31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260828-tonyk-syscall_table-v6-3-fdd358456cf1@igalia.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The perf tool's copy of `scripts/syscall.tbl` is left out of sync, and its syscall generator is unaware of the new common table.
- [Low] The use of `mktemp -p .` in build scripts is a GNU extension causing build failures on BSD-based hosts, and temporary files are leaked on error paths.
--

commit 81fbe53cf08247610dc3aa87d8f9c9e8106a9994
Author: André Almeida <andrealmeid@igalia.com>

scripts/syscall.tbl: Use the common table

Remove some of the duplicated code by using the common syscall number
table.

> diff --git a/scripts/Makefile.asm-headers b/scripts/Makefile.asm-headers
> index b38931314ad7c..e81d3ef8938ca 100644
> --- a/scripts/Makefile.asm-headers
> +++ b/scripts/Makefile.asm-headers
[ ... ]
> @@ -57,14 +58,14 @@ quiet_cmd_remove = REMOVE  $(unwanted)
>        cmd_remove = rm -f $(unwanted); find $(obj) -type d -empty -delete
>  
>  quiet_cmd_syshdr = SYSHDR  $@
> -      cmd_syshdr = $(CONFIG_SHELL) $(syshdr) \
> +      cmd_syshdr = $(CONFIG_SHELL) $(syshdr) --common-tbl $(syscalltbl_common) \

[Severity: Low]
Does this build script rely on a GNU-specific extension?

While not modified directly in this diff, scripts/syscallhdr.sh uses
mktemp -p ., which is a GNU extension:

scripts/syscallhdr.sh:gen_hdr() {
    ...
    tmpfile=$(mktemp -p .)
    ...
}

This will cause build failures on non-GNU hosts (such as macOS or FreeBSD)
where their mktemp implementations do not support the -p flag.

Additionally, if the script exits early via exit 1 in scripts/syscalltbl.sh
within gen_tbl(), the temporary file is permanently leaked in the build tree
because cleanup relies on a direct rm at the end instead of a shell trap:

scripts/syscalltbl.sh:gen_tbl() {
    ...
    tmpfile=$(mktemp -p .)
    ...
    exit 1
    ...
}

>  		   $(if $(syshdr-args-$*),$(syshdr-args-$*),$(syshdr-args)) \
>  		   $(if $(syscall_compat),--prefix "compat$*_") \
>  		   --abis $(subst $(space),$(comma),$(strip $(syscall_abis_$*))) \
>  		   $< $@
[ ... ]
> diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
> index 0ab5316051202..ee828d03094ed 100644
> --- a/scripts/syscall.tbl
> +++ b/scripts/syscall.tbl
> @@ -344,72 +344,3 @@
>  293	common	rseq				sys_rseq
>  294	common	kexec_file_load			sys_kexec_file_load
>  # 295 through 402 are unassigned to sync up with generic numbers don't use
> -403	32	clock_gettime64			sys_clock_gettime

[Severity: Medium]
Will this removal break perf's syscall resolution for architectures relying
on the generic syscall table?

By removing syscalls 403-472 from scripts/syscall.tbl, perf trace may fail
to resolve these syscalls by name (mapping them to unknown) because perf's
generator script is unaware of the new common table.

Looking at tools/perf/trace/beauty/syscalltbl.sh, the build_tables function
relies solely on scripts/syscall.tbl and has not been updated to include
scripts/syscall_common.tbl:

tools/perf/trace/beauty/syscalltbl.sh:build_tables() {
    ...
    build_tables "$tools_dir/scripts/syscall.tbl" "$outfile" common,32,riscv,memfd_secret EM_RISCV
    ...
}

This will also cause the perf build to emit warnings due to
tools/scripts/syscall.tbl being out of sync with scripts/syscall.tbl.

> -404	32	clock_settime64			sys_clock_settime
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-tonyk-syscall_table-v6-0-fdd358456cf1@igalia.com?part=3

  reply	other threads:[~2026-08-28 23:32 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28 23:19 [PATCH v6 00/13] syscalls: Add a shared table for all archs André Almeida
2026-08-28 23:19 ` [PATCH v6 01/13] syscalls: Make --abis parsing more robust André Almeida
2026-08-28 23:32   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 02/13] syscalls: Create unified partial table for all archs André Almeida
2026-08-28 23:31   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 03/13] scripts/syscall.tbl: Use the common table André Almeida
2026-08-28 23:32   ` sashiko-bot [this message]
2026-08-28 23:19 ` [PATCH v6 04/13] arm: Use the common syscall table André Almeida
2026-08-28 23:35   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 05/13] s390: " André Almeida
2026-08-28 23:27   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 06/13] sparc: " André Almeida
2026-08-28 23:26   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 07/13] mips: Remove duplicated syscallnr.sh André Almeida
2026-08-28 23:27   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 08/13] mips: Get rid of custom mips ABIs for syscall tables André Almeida
2026-08-28 23:29   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 09/13] mips: Use the common syscall table André Almeida
2026-08-28 23:35   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 10/13] syscalls: Add an option for offsetting the common table André Almeida
2026-08-28 23:34   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 11/13] alpha: Define entry point for set_mempolicy_home_node syscall André Almeida
2026-08-28 23:32   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 12/13] alpha: Remove alpha_ prefix from custom syscall entries André Almeida
2026-08-28 23:35   ` sashiko-bot
2026-08-28 23:19 ` [PATCH v6 13/13] alpha: Use the common syscall table André Almeida
2026-08-28 23:39   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260828233241.1DAA31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=andrealmeid@igalia.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.