[PATCH 0/3] syscalls: clean up stub naming convention

[PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

Ingo,

as discussed yesterday, here is a mini-series (mostly) implementing your
suggestion on how to tidy up the naming convention for syscall stubs.

For the generic case, we now will have:

t	        kernel_waitid	# common C function (see kernel/exit.c)

i	      __in_sys_waitid	# inlined helper doing the actual work
				# (takes parameters as declared)

T	      __do_sys_waitid	# C function calling inlined helper
				# (takes parameters of type long; casts
				#  them to the declared type)

i	__in_compat_sys_waitid	# inlined helper doing the actual work
				  (takes parameters as declared)

T	__do_compat_sys_waitid	# compat C function calling inlined helper
				# (takes parameters of type long, casts
				#  them to unsigned long and then to the
				#  declared type)

T	            sys_waitid	# alias to __do_sys_waitid() (taking
				# parameters as declared), to be included
				# in syscall table

T	     compat_sys_waitid	# alias to __do_compat_sys_waitid()
				# (taking parameters as declared), to
				# be included in syscall table


For 64-bit x86, kernel_waitid, __do_sys_waitid and __in_sys_waitid are the
same. But instead of sys_waitid and compat_sys_waitid, there are:

T	      __x64_sys_waitid	# x86 64-bit-ptregs -> C stub, calls
				# __do_sys_waitid(); to be included in
				# syscall table

T	     __ia32_sys_waitid	# IA32_EMULATION 32-bit-ptregs -> C stub,
				# calls __do_sys_waitid(); to be included
				# in syscall table unless there is a
				# compat syscall stub [in that case, it is
				# unused]

T     __ia32_compat_sys_waitid	# IA32_EMULATION 32-bit-ptregs -> C stub,
				# calls __do_compat_sys_waitid(); to be
				# included in syscall table

T      __x32_compat_sys_waitid	# x32 64-bit-ptregs -> C stub, calls
				# __do_compat_sys_waitid(); to be included
				# in syscall table

In short (0xffffffff prefix removed, re-ordered):

810f0af0 t            kernel_waitid	# common (32/64) kernel helper

<inline>            __in_sys_waitid	# inlined helper doing actual work
810f0be0 t          __do_sys_waitid	# C func calling inlined helper

<inline>     __in_compat_sys_waitid	# inlined helper doing actual work
810f0d80 t   __do_compat_sys_waitid	# compat C func calling inlined helper

810f2080 T         __x64_sys_waitid	# x64 64-bit-ptregs -> C stub
810f20b0 T        __ia32_sys_waitid	# ia32 32-bit-ptregs -> C stub [unused]
810f2470 T __ia32_compat_sys_waitid	# ia32 32-bit-ptregs -> compat C stub
810f2490 T  __x32_compat_sys_waitid	# x32 64-bit-ptregs -> compat C stub

The kbuild test robot barked at an alleged +20038 bytes kernel size regression
for i386-tinyconfig due to the first patch of this series. That seems to be a
false positive, as it likely doesn't take into account the change to
scripts/bloat-o-meter. Moreover, I could not reproduce such a size regression
on local i386 builds.

Thanks,
	Dominik

Dominik Brodowski (3):
  syscalls: clean up syscall stub naming convention
  syscalls: clean up compat syscall stub naming convention
  syscalls: rename struct pt_regs-based sys_*() to __x64_sys_*()

 Documentation/process/adding-syscalls.rst |   4 +-
 arch/x86/entry/syscalls/syscall_32.tbl    | 720 +++++++++++-----------
 arch/x86/entry/syscalls/syscall_64.tbl    | 710 ++++++++++-----------
 arch/x86/entry/syscalls/syscalltbl.sh     |  14 +-
 arch/x86/entry/vsyscall/vsyscall_64.c     |   6 +-
 arch/x86/include/asm/syscall_wrapper.h    | 142 +++--
 arch/x86/include/asm/syscalls.h           |   2 +-
 include/linux/compat.h                    |  24 +-
 include/linux/syscalls.h                  |  12 +-
 scripts/bloat-o-meter                     |   4 +-
 10 files changed, 841 insertions(+), 797 deletions(-)

-- 
2.17.0

[PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

Ingo,

as discussed yesterday, here is a mini-series (mostly) implementing your
suggestion on how to tidy up the naming convention for syscall stubs.

For the generic case, we now will have:

t	        kernel_waitid	# common C function (see kernel/exit.c)

i	      __in_sys_waitid	# inlined helper doing the actual work
				# (takes parameters as declared)

T	      __do_sys_waitid	# C function calling inlined helper
				# (takes parameters of type long; casts
				#  them to the declared type)

i	__in_compat_sys_waitid	# inlined helper doing the actual work
				  (takes parameters as declared)

T	__do_compat_sys_waitid	# compat C function calling inlined helper
				# (takes parameters of type long, casts
				#  them to unsigned long and then to the
				#  declared type)

T	            sys_waitid	# alias to __do_sys_waitid() (taking
				# parameters as declared), to be included
				# in syscall table

T	     compat_sys_waitid	# alias to __do_compat_sys_waitid()
				# (taking parameters as declared), to
				# be included in syscall table


For 64-bit x86, kernel_waitid, __do_sys_waitid and __in_sys_waitid are the
same. But instead of sys_waitid and compat_sys_waitid, there are:

T	      __x64_sys_waitid	# x86 64-bit-ptregs -> C stub, calls
				# __do_sys_waitid(); to be included in
				# syscall table

T	     __ia32_sys_waitid	# IA32_EMULATION 32-bit-ptregs -> C stub,
				# calls __do_sys_waitid(); to be included
				# in syscall table unless there is a
				# compat syscall stub [in that case, it is
				# unused]

T     __ia32_compat_sys_waitid	# IA32_EMULATION 32-bit-ptregs -> C stub,
				# calls __do_compat_sys_waitid(); to be
				# included in syscall table

T      __x32_compat_sys_waitid	# x32 64-bit-ptregs -> C stub, calls
				# __do_compat_sys_waitid(); to be included
				# in syscall table

In short (0xffffffff prefix removed, re-ordered):

810f0af0 t            kernel_waitid	# common (32/64) kernel helper

<inline>            __in_sys_waitid	# inlined helper doing actual work
810f0be0 t          __do_sys_waitid	# C func calling inlined helper

<inline>     __in_compat_sys_waitid	# inlined helper doing actual work
810f0d80 t   __do_compat_sys_waitid	# compat C func calling inlined helper

810f2080 T         __x64_sys_waitid	# x64 64-bit-ptregs -> C stub
810f20b0 T        __ia32_sys_waitid	# ia32 32-bit-ptregs -> C stub [unused]
810f2470 T __ia32_compat_sys_waitid	# ia32 32-bit-ptregs -> compat C stub
810f2490 T  __x32_compat_sys_waitid	# x32 64-bit-ptregs -> compat C stub

The kbuild test robot barked at an alleged +20038 bytes kernel size regression
for i386-tinyconfig due to the first patch of this series. That seems to be a
false positive, as it likely doesn't take into account the change to
scripts/bloat-o-meter. Moreover, I could not reproduce such a size regression
on local i386 builds.

Thanks,
	Dominik

Dominik Brodowski (3):
  syscalls: clean up syscall stub naming convention
  syscalls: clean up compat syscall stub naming convention
  syscalls: rename struct pt_regs-based sys_*() to __x64_sys_*()

 Documentation/process/adding-syscalls.rst |   4 +-
 arch/x86/entry/syscalls/syscall_32.tbl    | 720 +++++++++++-----------
 arch/x86/entry/syscalls/syscall_64.tbl    | 710 ++++++++++-----------
 arch/x86/entry/syscalls/syscalltbl.sh     |  14 +-
 arch/x86/entry/vsyscall/vsyscall_64.c     |   6 +-
 arch/x86/include/asm/syscall_wrapper.h    | 142 +++--
 arch/x86/include/asm/syscalls.h           |   2 +-
 include/linux/compat.h                    |  24 +-
 include/linux/syscalls.h                  |  12 +-
 scripts/bloat-o-meter                     |   4 +-
 10 files changed, 841 insertions(+), 797 deletions(-)

-- 
2.17.0

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> In short (0xffffffff prefix removed, re-ordered):
> 
> 810f0af0 t            kernel_waitid	# common (32/64) kernel helper
> 
> <inline>            __in_sys_waitid	# inlined helper doing actual work
> 810f0be0 t          __do_sys_waitid	# C func calling inlined helper
> 
> <inline>     __in_compat_sys_waitid	# inlined helper doing actual work
> 810f0d80 t   __do_compat_sys_waitid	# compat C func calling inlined helper
> 
> 810f2080 T         __x64_sys_waitid	# x64 64-bit-ptregs -> C stub
> 810f20b0 T        __ia32_sys_waitid	# ia32 32-bit-ptregs -> C stub [unused]
> 810f2470 T __ia32_compat_sys_waitid	# ia32 32-bit-ptregs -> compat C stub
> 810f2490 T  __x32_compat_sys_waitid	# x32 64-bit-ptregs -> compat C stub

Ok, looks pretty clean and nice to me all around, and looking at the highest level 
syscall tables the actual calling convention and address encoding is now a _lot_ 
more obvious at first sight as well.

The "in" part is a tiny bit confusing because it reads like a preposition:
"are we in sys_waitid?".

But I have no better idea, other than we could perhaps use more underscores to 
signal the inline helper, instead of the 'in_' prefix:

> 810f0af0 t            kernel_waitid	# common (32/64) kernel helper
> 
> <inline>            _____sys_waitid	# inlined helper doing actual work
> 810f0be0 t          __do_sys_waitid	# C func calling inlined helper
> 
> <inline>     _____compat_sys_waitid	# inlined helper doing actual work
> 810f0d80 t   __do_compat_sys_waitid	# compat C func calling inlined helper
> 
> 810f2080 T         __x64_sys_waitid	# x64 64-bit-ptregs -> C stub
> 810f20b0 T        __ia32_sys_waitid	# ia32 32-bit-ptregs -> C stub [unused]
> 810f2470 T __ia32_compat_sys_waitid	# ia32 32-bit-ptregs -> compat C stub
> 810f2490 T  __x32_compat_sys_waitid	# x32 64-bit-ptregs -> compat C stub

?

There are some other variants as well, here's the list of all the options I could 
think of:

  - _____sys_waitid()      # ridiculous number of underscores?
  - __sys_waitid()         # too generic sounding?
  - __inline_sys_waitid()  # too long?
  - __il_sys_waitid()      # reminds me of the IL country code ;-)
  - __in_sys_waitid()      # easy to read as 'are we in syscall?'

None is super convinging - but maybe __inline_sys_waitid is the most natural one.

[ Note, whichever we pick (if we pick a new one), there no need to resend, I can 
  edit the patches in place if you agree. ]

One more fundamental question: why do we have the __do_sys_waitid() and 
__inline_sys_waitid() distinction - aren't the function call signatures the same 
with no conversion done?

I.e. couldn't we just do a single, static __do_sys_waitid(), where the compiler 
would decide to what extent inlining is justified? This would allow the compiler 
to inline all the intermediate code into the stubs themselves.

Or is this a side effect of the error injection feature, which needs to add extra 
logic at this intermediate level? That too should be able to use the 
__do_sys_waitid() variant though.

> The kbuild test robot barked at an alleged +20038 bytes kernel size regression
> for i386-tinyconfig due to the first patch of this series. That seems to be a
> false positive, as it likely doesn't take into account the change to
> scripts/bloat-o-meter. Moreover, I could not reproduce such a size regression
> on local i386 builds.

Ok, I'll ignore that.

Is UML unaffected by these renames?

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> In short (0xffffffff prefix removed, re-ordered):
> 
> 810f0af0 t            kernel_waitid	# common (32/64) kernel helper
> 
> <inline>            __in_sys_waitid	# inlined helper doing actual work
> 810f0be0 t          __do_sys_waitid	# C func calling inlined helper
> 
> <inline>     __in_compat_sys_waitid	# inlined helper doing actual work
> 810f0d80 t   __do_compat_sys_waitid	# compat C func calling inlined helper
> 
> 810f2080 T         __x64_sys_waitid	# x64 64-bit-ptregs -> C stub
> 810f20b0 T        __ia32_sys_waitid	# ia32 32-bit-ptregs -> C stub [unused]
> 810f2470 T __ia32_compat_sys_waitid	# ia32 32-bit-ptregs -> compat C stub
> 810f2490 T  __x32_compat_sys_waitid	# x32 64-bit-ptregs -> compat C stub

Ok, looks pretty clean and nice to me all around, and looking at the highest level 
syscall tables the actual calling convention and address encoding is now a _lot_ 
more obvious at first sight as well.

The "in" part is a tiny bit confusing because it reads like a preposition:
"are we in sys_waitid?".

But I have no better idea, other than we could perhaps use more underscores to 
signal the inline helper, instead of the 'in_' prefix:

> 810f0af0 t            kernel_waitid	# common (32/64) kernel helper
> 
> <inline>            _____sys_waitid	# inlined helper doing actual work
> 810f0be0 t          __do_sys_waitid	# C func calling inlined helper
> 
> <inline>     _____compat_sys_waitid	# inlined helper doing actual work
> 810f0d80 t   __do_compat_sys_waitid	# compat C func calling inlined helper
> 
> 810f2080 T         __x64_sys_waitid	# x64 64-bit-ptregs -> C stub
> 810f20b0 T        __ia32_sys_waitid	# ia32 32-bit-ptregs -> C stub [unused]
> 810f2470 T __ia32_compat_sys_waitid	# ia32 32-bit-ptregs -> compat C stub
> 810f2490 T  __x32_compat_sys_waitid	# x32 64-bit-ptregs -> compat C stub

?

There are some other variants as well, here's the list of all the options I could 
think of:

  - _____sys_waitid()      # ridiculous number of underscores?
  - __sys_waitid()         # too generic sounding?
  - __inline_sys_waitid()  # too long?
  - __il_sys_waitid()      # reminds me of the IL country code ;-)
  - __in_sys_waitid()      # easy to read as 'are we in syscall?'

None is super convinging - but maybe __inline_sys_waitid is the most natural one.

[ Note, whichever we pick (if we pick a new one), there no need to resend, I can 
  edit the patches in place if you agree. ]

One more fundamental question: why do we have the __do_sys_waitid() and 
__inline_sys_waitid() distinction - aren't the function call signatures the same 
with no conversion done?

I.e. couldn't we just do a single, static __do_sys_waitid(), where the compiler 
would decide to what extent inlining is justified? This would allow the compiler 
to inline all the intermediate code into the stubs themselves.

Or is this a side effect of the error injection feature, which needs to add extra 
logic at this intermediate level? That too should be able to use the 
__do_sys_waitid() variant though.

> The kbuild test robot barked at an alleged +20038 bytes kernel size regression
> for i386-tinyconfig due to the first patch of this series. That seems to be a
> false positive, as it likely doesn't take into account the change to
> scripts/bloat-o-meter. Moreover, I could not reproduce such a size regression
> on local i386 builds.

Ok, I'll ignore that.

Is UML unaffected by these renames?

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
>   - _____sys_waitid()      # ridiculous number of underscores?
>   - __sys_waitid()         # too generic sounding?

... and we'd need to rename internal helpers in net/

>   - __inline_sys_waitid()  # too long?

sounds acceptable, though a bit long (especially for the compat case, though
it doesn't really matter in the case of 
__inline_compat_sys_sched_rr_get_interval)

> One more fundamental question: why do we have the __do_sys_waitid() and 
> __inline_sys_waitid() distinction - aren't the function call signatures the same 
> with no conversion done?
> 
> I.e. couldn't we just do a single, static __do_sys_waitid(), where the compiler 
> would decide to what extent inlining is justified? This would allow the compiler 
> to inline all the intermediate code into the stubs themselves.
> 
> Or is this a side effect of the error injection feature, which needs to add extra 
> logic at this intermediate level? That too should be able to use the 
> __do_sys_waitid() variant though.

Error injection is unrelated. It seems to be for three reasons, if I read
the code (include/linux/syscalls.h) correctly:

	asmlinkage long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))

1)  This takes arguments of type long (to protect against CVE-2009-0029);
    see https://lwn.net/Articles/604287/ : "Digging into the history of
    this, it turns out that the long version ensures that 32-bit values
    are correctly sign-extended for some 64-bit kernel platforms,
    preventing a historical vulnerability."

	{
		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));
		__MAP(x,__SC_TEST,__VA_ARGS__);

2) We can add testing whether one of the arguments is longer than long.

		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));

3) This adds asmlinkage_protect() on m68k, but seems to be a no-op on other
architectures.


While reasons 1 and 3 seem irrelevant on x86, I'd like to keep the code
close to the generic one -- and reason 2 is valid in and by itself. So I'd
recommend keeping the __inline_sys / __do_sys indirection.


> Is UML unaffected by these renames?

UML is only affected by patch 3/3, but kept happy by the patch to
entry/syscalls/syscalltbl.sh.


On a somewhat related note: I'll try to prepare a patch this evening which
lets us build just the __ia32_sys and __x32_compat_sys stubs we actually
need. We have that information already in entry/syscalls/syscall_{32,64}.tbl,
it just needs to be extracted into another header file (in the form of
	#define NEED_IA32_sys_xyzzz 1
) and then tested within the stubs. After some randconfig testing, this
might be worthwile to add on top of the patches already in tip-asm and the
three renaming patches currently under discussion.


Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
>   - _____sys_waitid()      # ridiculous number of underscores?
>   - __sys_waitid()         # too generic sounding?

... and we'd need to rename internal helpers in net/

>   - __inline_sys_waitid()  # too long?

sounds acceptable, though a bit long (especially for the compat case, though
it doesn't really matter in the case of 
__inline_compat_sys_sched_rr_get_interval)

> One more fundamental question: why do we have the __do_sys_waitid() and 
> __inline_sys_waitid() distinction - aren't the function call signatures the same 
> with no conversion done?
> 
> I.e. couldn't we just do a single, static __do_sys_waitid(), where the compiler 
> would decide to what extent inlining is justified? This would allow the compiler 
> to inline all the intermediate code into the stubs themselves.
> 
> Or is this a side effect of the error injection feature, which needs to add extra 
> logic at this intermediate level? That too should be able to use the 
> __do_sys_waitid() variant though.

Error injection is unrelated. It seems to be for three reasons, if I read
the code (include/linux/syscalls.h) correctly:

	asmlinkage long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))

1)  This takes arguments of type long (to protect against CVE-2009-0029);
    see https://lwn.net/Articles/604287/ : "Digging into the history of
    this, it turns out that the long version ensures that 32-bit values
    are correctly sign-extended for some 64-bit kernel platforms,
    preventing a historical vulnerability."

	{
		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));
		__MAP(x,__SC_TEST,__VA_ARGS__);

2) We can add testing whether one of the arguments is longer than long.

		__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__));

3) This adds asmlinkage_protect() on m68k, but seems to be a no-op on other
architectures.


While reasons 1 and 3 seem irrelevant on x86, I'd like to keep the code
close to the generic one -- and reason 2 is valid in and by itself. So I'd
recommend keeping the __inline_sys / __do_sys indirection.


> Is UML unaffected by these renames?

UML is only affected by patch 3/3, but kept happy by the patch to
entry/syscalls/syscalltbl.sh.


On a somewhat related note: I'll try to prepare a patch this evening which
lets us build just the __ia32_sys and __x32_compat_sys stubs we actually
need. We have that information already in entry/syscalls/syscall_{32,64}.tbl,
it just needs to be extracted into another header file (in the form of
	#define NEED_IA32_sys_xyzzz 1
) and then tested within the stubs. After some randconfig testing, this
might be worthwile to add on top of the patches already in tip-asm and the
three renaming patches currently under discussion.


Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Sun, Apr 08, 2018 at 11:15:36AM +0200, Dominik Brodowski wrote:
> On a somewhat related note: I'll try to prepare a patch this evening which
> lets us build just the __ia32_sys and __x32_compat_sys stubs we actually
> need. We have that information already in entry/syscalls/syscall_{32,64}.tbl,
> it just needs to be extracted into another header file (in the form of
> 	#define NEED_IA32_sys_xyzzz 1
> ) and then tested within the stubs. After some randconfig testing, this
> might be worthwile to add on top of the patches already in tip-asm and the
> three renaming patches currently under discussion.

So this got a bit more complicated than I though, for a -5k text size
decrease. I have my doubts whether the increased code complexity is really
worth that minor size decrease. I'll send another patch shortly, though,
which fixes up the naming of a few macros in <asm/syscall_wrapper.h>.

--------------------------------------------------------------------------
From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Sun, 8 Apr 2018 21:38:54 +0200
Subject: [PATCH] syscalls/x86: only build stubs which are actually needed

A new script arch/x86/entry/syscalls/syscallstubs.sh generates
defines in <asm/syscall_stubs.h>

	for all __ia32_sys_ stubs which are actually needed for
	IA32_EMULATION,

and

	for all __x32_compat_sys_ stubs which are actually needed
	for X32.

By omitting to build the remaining stubs (which are defined as
__SYSCALL_STUBx_UNUSED), there is a measurable decrease in kernel
size (-5k):

    text    data      bss      dec     hex filename
12892057 7094202 13570252 33556511 200081f vmlinux-orig
12886397 7087514 13570252 33544163 1ffd7e3 vmlinux

Further size cuttings could be achieved by not building stubs for
syscalls and compat_syscalls which are not referenced in the syscall
tables, such as (at least)

	sys_gethostname
	sys_sync_file_range2
	sys_send
	sys_recv
	compat_sys_mbind
	compat_sys_set_mempolicy
	compat_sys_migrate_pages
	compat_sys_sendtimedop
	compat_sys_msgrcv
	compat_sys_semctl
	compat_sys_send
	compat_sys_recv

Not-yet-signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/arch/x86/entry/syscalls/Makefile b/arch/x86/entry/syscalls/Makefile
index 6fb9b57ed5ba..036199136513 100644
--- a/arch/x86/entry/syscalls/Makefile
+++ b/arch/x86/entry/syscalls/Makefile
@@ -11,6 +11,7 @@ syscall64 := $(srctree)/$(src)/syscall_64.tbl
 
 syshdr := $(srctree)/$(src)/syscallhdr.sh
 systbl := $(srctree)/$(src)/syscalltbl.sh
+sysstubs := $(srctree)/$(src)/syscallstubs.sh
 
 quiet_cmd_syshdr = SYSHDR  $@
       cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \
@@ -20,6 +21,11 @@ quiet_cmd_syshdr = SYSHDR  $@
 quiet_cmd_systbl = SYSTBL  $@
       cmd_systbl = $(CONFIG_SHELL) '$(systbl)' $< $@
 
+quiet_cmd_sysstubs = SYSSTUBS  $@
+      cmd_sysstubs = $(CONFIG_SHELL) '$(sysstubs)' \
+		   '$(syscall32)' '$(syscall64)' \
+		    $@
+
 quiet_cmd_hypercalls = HYPERCALLS $@
       cmd_hypercalls = $(CONFIG_SHELL) '$<' $@ $(filter-out $<,$^)
 
@@ -51,6 +57,9 @@ $(out)/syscalls_32.h: $(syscall32) $(systbl)
 $(out)/syscalls_64.h: $(syscall64) $(systbl)
 	$(call if_changed,systbl)
 
+$(out)/syscall_stubs.h:  $(syscall32) $(syscall64) $(sysstubs)
+	$(call if_changed,sysstubs)
+
 $(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
 	$(call if_changed,hypercalls)
 
@@ -60,6 +69,7 @@ uapisyshdr-y			+= unistd_32.h unistd_64.h unistd_x32.h
 syshdr-y			+= syscalls_32.h
 syshdr-$(CONFIG_X86_64)		+= unistd_32_ia32.h unistd_64_x32.h
 syshdr-$(CONFIG_X86_64)		+= syscalls_64.h
+syshdr-$(CONFIG_X86_64)		+= syscall_stubs.h
 syshdr-$(CONFIG_XEN)		+= xen-hypercalls.h
 
 targets	+= $(uapisyshdr-y) $(syshdr-y)
diff --git a/arch/x86/entry/syscalls/syscallstubs.sh b/arch/x86/entry/syscalls/syscallstubs.sh
new file mode 100644
index 000000000000..4db64d4db75a
--- /dev/null
+++ b/arch/x86/entry/syscalls/syscallstubs.sh
@@ -0,0 +1,110 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+in32="$1"
+in64="$2"
+out="$3"
+
+emit_stub() {
+    entry="$1"
+    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
+	# We need a stub named __ia32_sys which is common to 64-bit
+	# except for a different pt_regs layout.
+	stubname=${entry#__ia32_sys_}
+	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
+	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
+    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
+	# We need a stub named __x32_compat_sys_ which decodes a
+	# 64-bit pt_regs and then calls the real syscall function 
+	stubname="${entry%%/*}" # handle qualifier
+	stubname=${stubname#__x32_compat_sys_} # handle prefix
+	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
+	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
+    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
+	# The compat entry starts with __ia32_compat_sys_x86, so it
+	# is a specific x86 compat syscall; no need for __ia32_sys_*()
+	stubname=${entry#__ia32_compat_sys_x86_}
+	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
+    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
+	# The compat entry starts with __ia32_compat_sys, so it is
+	# is a generic x86 compat syscall; no need for __ia32_sys_*()
+	stubname=${entry#__ia32_compat_sys_}
+	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
+    fi;
+}
+
+# First, we need to check which stubs we *need*. While at it, we can determine
+# quite many ia32 stubs we do *not* need as the syscall is handled by a compat
+# syscall
+grep '^[0-9]' "$in32" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "I386" -a -n "$compat" ]; then
+	    emit_stub "$compat"
+	fi
+    done
+) > "$out"
+
+grep '^[0-9]' "$in64" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "X32" -a -n "$entry" ]; then
+	    emit_stub "$entry"
+	fi
+    done
+) >> "$out"
+
+# Then, we need to determine all (remaining) stubs we *do not* need.
+grep '^[0-9]' "$in64" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "COMMON" -o "$abi" = "64" ]; then
+	    if [ -n "$entry" -a "$entry" != "${entry#__x64_sys_}" ]; then
+		# what's the actual stubname?
+		stubname="${entry%%/*}" # handle qualifier
+		stubname="${stubname#__x64_sys_}" # handle prefix
+
+		# syscalls only referenced for 64-bit do not need a stub for
+		# IA32_EMULATION
+		echo "#ifndef ASM_X86_HAS__ia32_sys_${stubname}"
+		echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+		echo "#endif"
+
+		# A number of compat syscalls are built in even though they are
+		# completely unused -- e.g. mbind set_mempolicy migrate_pages
+		# sendtimedop msgrcv semctl. We probably define more compat
+		# syscalls than exist, but better be safe than sorry...
+		echo "#ifndef ASM_X86_HAS__x32_compat_sys_${stubname}"
+		echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+		echo "#endif"
+	    fi
+	fi
+    done
+) >> "$out"
+
+grep '^[0-9]' "$in32" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "I386" -a -n "$compat" ]; then
+	    if [ "$compat" != "${compat#__ia32_compat_sys}" ]; then
+		stubname="${compat#__ia32_compat_sys_}"
+		# If a compat syscall is not needed for x32 (see above),
+		# we need to assert that the stub is defined as unused
+		echo "#ifndef ASM_X86_HAS__x32_compat_sys_${stubname}"
+		echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+		echo "#endif"
+	    fi
+	fi
+    done
+) >> "$out"
+
+# FIXME: These syscalls get build even though they are completely unused on x86
+( for unused_syscall in gethostname sync_file_range2 send recv; do
+    echo "#define __IA32_SYS_STUBx_${unused_syscall} __SYSCALL_STUBx_UNUSED"
+done ) >> "$out"
+( for unused_compat_syscall in send recv; do
+    echo "#define __X32_COMPAT_SYS_STUBx_${unused_compat_syscall} __SYSCALL_STUBx_UNUSED"
+done ) >> "$out"
+
diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild
index de690c2d2e33..3aeb3a794da4 100644
--- a/arch/x86/include/asm/Kbuild
+++ b/arch/x86/include/asm/Kbuild
@@ -5,6 +5,7 @@ generated-y += syscalls_64.h
 generated-y += unistd_32_ia32.h
 generated-y += unistd_64_x32.h
 generated-y += xen-hypercalls.h
+generated-y += syscall_stubs.h
 
 generic-y += dma-contiguous.h
 generic-y += early_ioremap.h
diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
index bad0295739bf..0b847f422bf6 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -4,7 +4,10 @@
  */
 
 #ifndef _ASM_X86_SYSCALL_WRAPPER_H
-#define _ASM_X86_SYSCALL_WRAPPER_H
+#define _ASM_X86_SYSCALL_WRAPPER_H 1
+
+#define __SYSCALL_STUBx_UNUSED(x, name, ...)
+#include <asm/syscall_stubs.h>
 
 /* Mapping of registers to parameters for syscalls on x86-64 and x32 */
 #define SC_X86_64_REGS_TO_ARGS(x, ...)					\
@@ -28,15 +31,15 @@
  * kernel/sys_ni.c and SYS_NI in kernel/time/posix-stubs.c to cover this
  * case as well.
  */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)				\
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__ia32_compat_sys##name, ERRNO);		\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs)\
 	{								\
 		return __do_compat_sys##name(SC_IA32_REGS_TO_ARGS(x,__VA_ARGS__));\
-	}								\
+	}
 
-#define SC_IA32_WRAPPERx(x, name, ...)					\
+#define __IA32_SYS_STUBx(x, name, ...)					\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs);	\
 	ALLOW_ERROR_INJECTION(__ia32_sys##name, ERRNO);			\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs)	\
@@ -64,8 +67,8 @@
 	SYSCALL_ALIAS(__ia32_sys_##name, sys_ni_posix_timers)
 
 #else /* CONFIG_IA32_EMULATION */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)
-#define SC_IA32_WRAPPERx(x, fullname, name, ...)
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)
+#define __IA32_SYS_STUBx(x, name, ...)
 #endif /* CONFIG_IA32_EMULATION */
 
 
@@ -75,7 +78,7 @@
  * of the x86-64-style parameter ordering of x32 syscalls. The syscalls common
  * with x86_64 obviously do not need such care.
  */
-#define COMPAT_SC_X32_STUBx(x, name, ...)				\
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__x32_compat_sys##name, ERRNO);		\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs)\
@@ -84,7 +87,7 @@
 	}								\
 
 #else /* CONFIG_X86_X32 */
-#define COMPAT_SC_X32_STUBx(x, name, ...)
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)
 #endif /* CONFIG_X86_X32 */
 
 
@@ -97,8 +100,8 @@
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)					\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
 	static inline long __in_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
-	COMPAT_SC_IA32_STUBx(x, name, __VA_ARGS__)				\
-	COMPAT_SC_X32_STUBx(x, name, __VA_ARGS__)				\
+	__IA32_COMPAT_SYS_STUBx(x, name, __VA_ARGS__)				\
+	__X32_COMPAT_SYS_STUBx##name(x, name, __VA_ARGS__)			\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{									\
 		return __in_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
@@ -120,7 +123,6 @@
 
 #endif /* CONFIG_COMPAT */
 
-
 /*
  * Instead of the generic __SYSCALL_DEFINEx() definition, this macro takes
  * struct pt_regs *regs as the only argument of the syscall stub named
@@ -163,7 +165,7 @@
 	{								\
 		return __do_sys##name(SC_X86_64_REGS_TO_ARGS(x,__VA_ARGS__));\
 	}								\
-	SC_IA32_WRAPPERx(x, name, __VA_ARGS__)				\
+	__IA32_SYS_STUBx##name(x, name, __VA_ARGS__)			\
 	static long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{								\
 		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Sun, Apr 08, 2018 at 11:15:36AM +0200, Dominik Brodowski wrote:
> On a somewhat related note: I'll try to prepare a patch this evening which
> lets us build just the __ia32_sys and __x32_compat_sys stubs we actually
> need. We have that information already in entry/syscalls/syscall_{32,64}.tbl,
> it just needs to be extracted into another header file (in the form of
> 	#define NEED_IA32_sys_xyzzz 1
> ) and then tested within the stubs. After some randconfig testing, this
> might be worthwile to add on top of the patches already in tip-asm and the
> three renaming patches currently under discussion.

So this got a bit more complicated than I though, for a -5k text size
decrease. I have my doubts whether the increased code complexity is really
worth that minor size decrease. I'll send another patch shortly, though,
which fixes up the naming of a few macros in <asm/syscall_wrapper.h>.

--------------------------------------------------------------------------
From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Sun, 8 Apr 2018 21:38:54 +0200
Subject: [PATCH] syscalls/x86: only build stubs which are actually needed

A new script arch/x86/entry/syscalls/syscallstubs.sh generates
defines in <asm/syscall_stubs.h>

	for all __ia32_sys_ stubs which are actually needed for
	IA32_EMULATION,

and

	for all __x32_compat_sys_ stubs which are actually needed
	for X32.

By omitting to build the remaining stubs (which are defined as
__SYSCALL_STUBx_UNUSED), there is a measurable decrease in kernel
size (-5k):

    text    data      bss      dec     hex filename
12892057 7094202 13570252 33556511 200081f vmlinux-orig
12886397 7087514 13570252 33544163 1ffd7e3 vmlinux

Further size cuttings could be achieved by not building stubs for
syscalls and compat_syscalls which are not referenced in the syscall
tables, such as (at least)

	sys_gethostname
	sys_sync_file_range2
	sys_send
	sys_recv
	compat_sys_mbind
	compat_sys_set_mempolicy
	compat_sys_migrate_pages
	compat_sys_sendtimedop
	compat_sys_msgrcv
	compat_sys_semctl
	compat_sys_send
	compat_sys_recv

Not-yet-signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/arch/x86/entry/syscalls/Makefile b/arch/x86/entry/syscalls/Makefile
index 6fb9b57ed5ba..036199136513 100644
--- a/arch/x86/entry/syscalls/Makefile
+++ b/arch/x86/entry/syscalls/Makefile
@@ -11,6 +11,7 @@ syscall64 := $(srctree)/$(src)/syscall_64.tbl
 
 syshdr := $(srctree)/$(src)/syscallhdr.sh
 systbl := $(srctree)/$(src)/syscalltbl.sh
+sysstubs := $(srctree)/$(src)/syscallstubs.sh
 
 quiet_cmd_syshdr = SYSHDR  $@
       cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \
@@ -20,6 +21,11 @@ quiet_cmd_syshdr = SYSHDR  $@
 quiet_cmd_systbl = SYSTBL  $@
       cmd_systbl = $(CONFIG_SHELL) '$(systbl)' $< $@
 
+quiet_cmd_sysstubs = SYSSTUBS  $@
+      cmd_sysstubs = $(CONFIG_SHELL) '$(sysstubs)' \
+		   '$(syscall32)' '$(syscall64)' \
+		    $@
+
 quiet_cmd_hypercalls = HYPERCALLS $@
       cmd_hypercalls = $(CONFIG_SHELL) '$<' $@ $(filter-out $<,$^)
 
@@ -51,6 +57,9 @@ $(out)/syscalls_32.h: $(syscall32) $(systbl)
 $(out)/syscalls_64.h: $(syscall64) $(systbl)
 	$(call if_changed,systbl)
 
+$(out)/syscall_stubs.h:  $(syscall32) $(syscall64) $(sysstubs)
+	$(call if_changed,sysstubs)
+
 $(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
 	$(call if_changed,hypercalls)
 
@@ -60,6 +69,7 @@ uapisyshdr-y			+= unistd_32.h unistd_64.h unistd_x32.h
 syshdr-y			+= syscalls_32.h
 syshdr-$(CONFIG_X86_64)		+= unistd_32_ia32.h unistd_64_x32.h
 syshdr-$(CONFIG_X86_64)		+= syscalls_64.h
+syshdr-$(CONFIG_X86_64)		+= syscall_stubs.h
 syshdr-$(CONFIG_XEN)		+= xen-hypercalls.h
 
 targets	+= $(uapisyshdr-y) $(syshdr-y)
diff --git a/arch/x86/entry/syscalls/syscallstubs.sh b/arch/x86/entry/syscalls/syscallstubs.sh
new file mode 100644
index 000000000000..4db64d4db75a
--- /dev/null
+++ b/arch/x86/entry/syscalls/syscallstubs.sh
@@ -0,0 +1,110 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+in32="$1"
+in64="$2"
+out="$3"
+
+emit_stub() {
+    entry="$1"
+    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
+	# We need a stub named __ia32_sys which is common to 64-bit
+	# except for a different pt_regs layout.
+	stubname=${entry#__ia32_sys_}
+	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
+	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
+    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
+	# We need a stub named __x32_compat_sys_ which decodes a
+	# 64-bit pt_regs and then calls the real syscall function 
+	stubname="${entry%%/*}" # handle qualifier
+	stubname=${stubname#__x32_compat_sys_} # handle prefix
+	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
+	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
+    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
+	# The compat entry starts with __ia32_compat_sys_x86, so it
+	# is a specific x86 compat syscall; no need for __ia32_sys_*()
+	stubname=${entry#__ia32_compat_sys_x86_}
+	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
+    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
+	# The compat entry starts with __ia32_compat_sys, so it is
+	# is a generic x86 compat syscall; no need for __ia32_sys_*()
+	stubname=${entry#__ia32_compat_sys_}
+	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
+    fi;
+}
+
+# First, we need to check which stubs we *need*. While at it, we can determine
+# quite many ia32 stubs we do *not* need as the syscall is handled by a compat
+# syscall
+grep '^[0-9]' "$in32" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "I386" -a -n "$compat" ]; then
+	    emit_stub "$compat"
+	fi
+    done
+) > "$out"
+
+grep '^[0-9]' "$in64" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "X32" -a -n "$entry" ]; then
+	    emit_stub "$entry"
+	fi
+    done
+) >> "$out"
+
+# Then, we need to determine all (remaining) stubs we *do not* need.
+grep '^[0-9]' "$in64" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "COMMON" -o "$abi" = "64" ]; then
+	    if [ -n "$entry" -a "$entry" != "${entry#__x64_sys_}" ]; then
+		# what's the actual stubname?
+		stubname="${entry%%/*}" # handle qualifier
+		stubname="${stubname#__x64_sys_}" # handle prefix
+
+		# syscalls only referenced for 64-bit do not need a stub for
+		# IA32_EMULATION
+		echo "#ifndef ASM_X86_HAS__ia32_sys_${stubname}"
+		echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+		echo "#endif"
+
+		# A number of compat syscalls are built in even though they are
+		# completely unused -- e.g. mbind set_mempolicy migrate_pages
+		# sendtimedop msgrcv semctl. We probably define more compat
+		# syscalls than exist, but better be safe than sorry...
+		echo "#ifndef ASM_X86_HAS__x32_compat_sys_${stubname}"
+		echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+		echo "#endif"
+	    fi
+	fi
+    done
+) >> "$out"
+
+grep '^[0-9]' "$in32" | sort -n | (
+    while read nr abi name entry compat; do
+	abi=`echo "$abi" | tr '[a-z]' '[A-Z]'`
+	if [ "$abi" = "I386" -a -n "$compat" ]; then
+	    if [ "$compat" != "${compat#__ia32_compat_sys}" ]; then
+		stubname="${compat#__ia32_compat_sys_}"
+		# If a compat syscall is not needed for x32 (see above),
+		# we need to assert that the stub is defined as unused
+		echo "#ifndef ASM_X86_HAS__x32_compat_sys_${stubname}"
+		echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
+		echo "#endif"
+	    fi
+	fi
+    done
+) >> "$out"
+
+# FIXME: These syscalls get build even though they are completely unused on x86
+( for unused_syscall in gethostname sync_file_range2 send recv; do
+    echo "#define __IA32_SYS_STUBx_${unused_syscall} __SYSCALL_STUBx_UNUSED"
+done ) >> "$out"
+( for unused_compat_syscall in send recv; do
+    echo "#define __X32_COMPAT_SYS_STUBx_${unused_compat_syscall} __SYSCALL_STUBx_UNUSED"
+done ) >> "$out"
+
diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild
index de690c2d2e33..3aeb3a794da4 100644
--- a/arch/x86/include/asm/Kbuild
+++ b/arch/x86/include/asm/Kbuild
@@ -5,6 +5,7 @@ generated-y += syscalls_64.h
 generated-y += unistd_32_ia32.h
 generated-y += unistd_64_x32.h
 generated-y += xen-hypercalls.h
+generated-y += syscall_stubs.h
 
 generic-y += dma-contiguous.h
 generic-y += early_ioremap.h
diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
index bad0295739bf..0b847f422bf6 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -4,7 +4,10 @@
  */
 
 #ifndef _ASM_X86_SYSCALL_WRAPPER_H
-#define _ASM_X86_SYSCALL_WRAPPER_H
+#define _ASM_X86_SYSCALL_WRAPPER_H 1
+
+#define __SYSCALL_STUBx_UNUSED(x, name, ...)
+#include <asm/syscall_stubs.h>
 
 /* Mapping of registers to parameters for syscalls on x86-64 and x32 */
 #define SC_X86_64_REGS_TO_ARGS(x, ...)					\
@@ -28,15 +31,15 @@
  * kernel/sys_ni.c and SYS_NI in kernel/time/posix-stubs.c to cover this
  * case as well.
  */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)				\
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__ia32_compat_sys##name, ERRNO);		\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs)\
 	{								\
 		return __do_compat_sys##name(SC_IA32_REGS_TO_ARGS(x,__VA_ARGS__));\
-	}								\
+	}
 
-#define SC_IA32_WRAPPERx(x, name, ...)					\
+#define __IA32_SYS_STUBx(x, name, ...)					\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs);	\
 	ALLOW_ERROR_INJECTION(__ia32_sys##name, ERRNO);			\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs)	\
@@ -64,8 +67,8 @@
 	SYSCALL_ALIAS(__ia32_sys_##name, sys_ni_posix_timers)
 
 #else /* CONFIG_IA32_EMULATION */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)
-#define SC_IA32_WRAPPERx(x, fullname, name, ...)
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)
+#define __IA32_SYS_STUBx(x, name, ...)
 #endif /* CONFIG_IA32_EMULATION */
 
 
@@ -75,7 +78,7 @@
  * of the x86-64-style parameter ordering of x32 syscalls. The syscalls common
  * with x86_64 obviously do not need such care.
  */
-#define COMPAT_SC_X32_STUBx(x, name, ...)				\
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__x32_compat_sys##name, ERRNO);		\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs)\
@@ -84,7 +87,7 @@
 	}								\
 
 #else /* CONFIG_X86_X32 */
-#define COMPAT_SC_X32_STUBx(x, name, ...)
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)
 #endif /* CONFIG_X86_X32 */
 
 
@@ -97,8 +100,8 @@
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)					\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
 	static inline long __in_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
-	COMPAT_SC_IA32_STUBx(x, name, __VA_ARGS__)				\
-	COMPAT_SC_X32_STUBx(x, name, __VA_ARGS__)				\
+	__IA32_COMPAT_SYS_STUBx(x, name, __VA_ARGS__)				\
+	__X32_COMPAT_SYS_STUBx##name(x, name, __VA_ARGS__)			\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{									\
 		return __in_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
@@ -120,7 +123,6 @@
 
 #endif /* CONFIG_COMPAT */
 
-
 /*
  * Instead of the generic __SYSCALL_DEFINEx() definition, this macro takes
  * struct pt_regs *regs as the only argument of the syscall stub named
@@ -163,7 +165,7 @@
 	{								\
 		return __do_sys##name(SC_X86_64_REGS_TO_ARGS(x,__VA_ARGS__));\
 	}								\
-	SC_IA32_WRAPPERx(x, name, __VA_ARGS__)				\
+	__IA32_SYS_STUBx##name(x, name, __VA_ARGS__)			\
 	static long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{								\
 		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\

[PATCH 4/3] syscalls/x86: adapt syscall_wrapper.h to the new syscall stub naming convention

[Dominik Brodowski]

Make the code in syscall_wrapper.h more readable by naming the stub macros
similar to the stub they provide. While at it, fix a stray newline at the
end of the __IA32_COMPAT_SYS_STUBx macro.

Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
index bad0295739bf..37ee1985d156 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -28,15 +28,15 @@
  * kernel/sys_ni.c and SYS_NI in kernel/time/posix-stubs.c to cover this
  * case as well.
  */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)				\
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__ia32_compat_sys##name, ERRNO);		\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs)\
 	{								\
 		return __do_compat_sys##name(SC_IA32_REGS_TO_ARGS(x,__VA_ARGS__));\
-	}								\
+	}
 
-#define SC_IA32_WRAPPERx(x, name, ...)					\
+#define __IA32_SYS_STUBx(x, name, ...)					\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs);	\
 	ALLOW_ERROR_INJECTION(__ia32_sys##name, ERRNO);			\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs)	\
@@ -64,8 +64,8 @@
 	SYSCALL_ALIAS(__ia32_sys_##name, sys_ni_posix_timers)
 
 #else /* CONFIG_IA32_EMULATION */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)
-#define SC_IA32_WRAPPERx(x, fullname, name, ...)
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)
+#define __IA32_SYS_STUBx(x, fullname, name, ...)
 #endif /* CONFIG_IA32_EMULATION */
 
 
@@ -75,7 +75,7 @@
  * of the x86-64-style parameter ordering of x32 syscalls. The syscalls common
  * with x86_64 obviously do not need such care.
  */
-#define COMPAT_SC_X32_STUBx(x, name, ...)				\
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__x32_compat_sys##name, ERRNO);		\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs)\
@@ -84,7 +84,7 @@
 	}								\
 
 #else /* CONFIG_X86_X32 */
-#define COMPAT_SC_X32_STUBx(x, name, ...)
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)
 #endif /* CONFIG_X86_X32 */
 
 
@@ -97,8 +97,8 @@
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)					\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
 	static inline long __in_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
-	COMPAT_SC_IA32_STUBx(x, name, __VA_ARGS__)				\
-	COMPAT_SC_X32_STUBx(x, name, __VA_ARGS__)				\
+	__IA32_COMPAT_SYS_STUBx(x, name, __VA_ARGS__)				\
+	__X32_COMPAT_SYS_STUBx(x, name, __VA_ARGS__)				\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{									\
 		return __in_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
@@ -163,7 +163,7 @@
 	{								\
 		return __do_sys##name(SC_X86_64_REGS_TO_ARGS(x,__VA_ARGS__));\
 	}								\
-	SC_IA32_WRAPPERx(x, name, __VA_ARGS__)				\
+	__IA32_SYS_STUBx(x, name, __VA_ARGS__)				\
 	static long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{								\
 		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\

[PATCH 4/3] syscalls/x86: adapt syscall_wrapper.h to the new syscall stub naming convention

[Dominik Brodowski]

Make the code in syscall_wrapper.h more readable by naming the stub macros
similar to the stub they provide. While at it, fix a stray newline at the
end of the __IA32_COMPAT_SYS_STUBx macro.

Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
index bad0295739bf..37ee1985d156 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -28,15 +28,15 @@
  * kernel/sys_ni.c and SYS_NI in kernel/time/posix-stubs.c to cover this
  * case as well.
  */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)				\
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__ia32_compat_sys##name, ERRNO);		\
 	asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs)\
 	{								\
 		return __do_compat_sys##name(SC_IA32_REGS_TO_ARGS(x,__VA_ARGS__));\
-	}								\
+	}
 
-#define SC_IA32_WRAPPERx(x, name, ...)					\
+#define __IA32_SYS_STUBx(x, name, ...)					\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs);	\
 	ALLOW_ERROR_INJECTION(__ia32_sys##name, ERRNO);			\
 	asmlinkage long __ia32_sys##name(const struct pt_regs *regs)	\
@@ -64,8 +64,8 @@
 	SYSCALL_ALIAS(__ia32_sys_##name, sys_ni_posix_timers)
 
 #else /* CONFIG_IA32_EMULATION */
-#define COMPAT_SC_IA32_STUBx(x, name, ...)
-#define SC_IA32_WRAPPERx(x, fullname, name, ...)
+#define __IA32_COMPAT_SYS_STUBx(x, name, ...)
+#define __IA32_SYS_STUBx(x, fullname, name, ...)
 #endif /* CONFIG_IA32_EMULATION */
 
 
@@ -75,7 +75,7 @@
  * of the x86-64-style parameter ordering of x32 syscalls. The syscalls common
  * with x86_64 obviously do not need such care.
  */
-#define COMPAT_SC_X32_STUBx(x, name, ...)				\
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)				\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs);\
 	ALLOW_ERROR_INJECTION(__x32_compat_sys##name, ERRNO);		\
 	asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs)\
@@ -84,7 +84,7 @@
 	}								\
 
 #else /* CONFIG_X86_X32 */
-#define COMPAT_SC_X32_STUBx(x, name, ...)
+#define __X32_COMPAT_SYS_STUBx(x, name, ...)
 #endif /* CONFIG_X86_X32 */
 
 
@@ -97,8 +97,8 @@
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)					\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
 	static inline long __in_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
-	COMPAT_SC_IA32_STUBx(x, name, __VA_ARGS__)				\
-	COMPAT_SC_X32_STUBx(x, name, __VA_ARGS__)				\
+	__IA32_COMPAT_SYS_STUBx(x, name, __VA_ARGS__)				\
+	__X32_COMPAT_SYS_STUBx(x, name, __VA_ARGS__)				\
 	static long __do_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{									\
 		return __in_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
@@ -163,7 +163,7 @@
 	{								\
 		return __do_sys##name(SC_X86_64_REGS_TO_ARGS(x,__VA_ARGS__));\
 	}								\
-	SC_IA32_WRAPPERx(x, name, __VA_ARGS__)				\
+	__IA32_SYS_STUBx(x, name, __VA_ARGS__)				\
 	static long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{								\
 		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> > One more fundamental question: why do we have the __do_sys_waitid() and 
> > __inline_sys_waitid() distinction - aren't the function call signatures the same 
> > with no conversion done?
> > 
> > I.e. couldn't we just do a single, static __do_sys_waitid(), where the compiler 
> > would decide to what extent inlining is justified? This would allow the compiler 
> > to inline all the intermediate code into the stubs themselves.
> > 
> > Or is this a side effect of the error injection feature, which needs to add extra 
> > logic at this intermediate level? That too should be able to use the 
> > __do_sys_waitid() variant though.
> 
> Error injection is unrelated. It seems to be for three reasons, if I read
> the code (include/linux/syscalls.h) correctly:
> 
> 	asmlinkage long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))
> 
> 1)  This takes arguments of type long (to protect against CVE-2009-0029);
>     see https://lwn.net/Articles/604287/ : "Digging into the history of
>     this, it turns out that the long version ensures that 32-bit values
>     are correctly sign-extended for some 64-bit kernel platforms,
>     preventing a historical vulnerability."
> 
> 	{
> 		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));
> 		__MAP(x,__SC_TEST,__VA_ARGS__);

I see - so it's _not_ the same function call signature, but a wrapper with a 
sign-extended version, which is fair and useful. So on architectures where this 
matters there's type conversion and active code generated.

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> > One more fundamental question: why do we have the __do_sys_waitid() and 
> > __inline_sys_waitid() distinction - aren't the function call signatures the same 
> > with no conversion done?
> > 
> > I.e. couldn't we just do a single, static __do_sys_waitid(), where the compiler 
> > would decide to what extent inlining is justified? This would allow the compiler 
> > to inline all the intermediate code into the stubs themselves.
> > 
> > Or is this a side effect of the error injection feature, which needs to add extra 
> > logic at this intermediate level? That too should be able to use the 
> > __do_sys_waitid() variant though.
> 
> Error injection is unrelated. It seems to be for three reasons, if I read
> the code (include/linux/syscalls.h) correctly:
> 
> 	asmlinkage long __do_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))
> 
> 1)  This takes arguments of type long (to protect against CVE-2009-0029);
>     see https://lwn.net/Articles/604287/ : "Digging into the history of
>     this, it turns out that the long version ensures that 32-bit values
>     are correctly sign-extended for some 64-bit kernel platforms,
>     preventing a historical vulnerability."
> 
> 	{
> 		long ret = __in_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));
> 		__MAP(x,__SC_TEST,__VA_ARGS__);

I see - so it's _not_ the same function call signature, but a wrapper with a 
sign-extended version, which is fair and useful. So on architectures where this 
matters there's type conversion and active code generated.

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> >   - _____sys_waitid()      # ridiculous number of underscores?
> >   - __sys_waitid()         # too generic sounding?
> 
> ... and we'd need to rename internal helpers in net/
> 
> >   - __inline_sys_waitid()  # too long?
> 
> sounds acceptable, though a bit long (especially for the compat case, though
> it doesn't really matter in the case of 
> __inline_compat_sys_sched_rr_get_interval)

So as per the previous mail this is not just an inline function, but an active 
type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
on some 64-bit architectures.

And that's a really non-obvious property IMO, and the name should probably reflect 
_that_ non-obvious property, not the inlining property which is really just a 
small detail.

I.e. how about:

	__se_sys_waitid()

... where 'se' stands for sign-extended, with a comment in the macro that explains 
the prefix? (The historical abbreviation for sign extension is 'sext', which I 
think wouldn't really be suitable these days.)

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> >   - _____sys_waitid()      # ridiculous number of underscores?
> >   - __sys_waitid()         # too generic sounding?
> 
> ... and we'd need to rename internal helpers in net/
> 
> >   - __inline_sys_waitid()  # too long?
> 
> sounds acceptable, though a bit long (especially for the compat case, though
> it doesn't really matter in the case of 
> __inline_compat_sys_sched_rr_get_interval)

So as per the previous mail this is not just an inline function, but an active 
type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
on some 64-bit architectures.

And that's a really non-obvious property IMO, and the name should probably reflect 
_that_ non-obvious property, not the inlining property which is really just a 
small detail.

I.e. how about:

	__se_sys_waitid()

... where 'se' stands for sign-extended, with a comment in the macro that explains 
the prefix? (The historical abbreviation for sign extension is 'sext', which I 
think wouldn't really be suitable these days.)

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> +emit_stub() {
> +    entry="$1"
> +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> +	# We need a stub named __ia32_sys which is common to 64-bit
> +	# except for a different pt_regs layout.
> +	stubname=${entry#__ia32_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> +	# We need a stub named __x32_compat_sys_ which decodes a
> +	# 64-bit pt_regs and then calls the real syscall function 
> +	stubname="${entry%%/*}" # handle qualifier
> +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> +	# The compat entry starts with __ia32_compat_sys_x86, so it
> +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> +	stubname=${entry#__ia32_compat_sys_x86_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> +	# The compat entry starts with __ia32_compat_sys, so it is
> +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> +	stubname=${entry#__ia32_compat_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    fi;
> +}

I only have a bikeshed painting comment, even in shell scripts please try to 
follow the kernel comment style visually, i.e. something like:

> +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> +
> +	#
> +	# We need a stub named __ia32_sys which is common to 64-bit
> +	# except for a different pt_regs layout.
> +	#
> +	stubname=${entry#__ia32_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> +
> +	#
> +	# We need a stub named __x32_compat_sys_ which decodes a
> +	# 64-bit pt_regs and then calls the real syscall function 
> +     #
> +	stubname="${entry%%/*}" # handle qualifier
> +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> +
> +     #
> +	# The compat entry starts with __ia32_compat_sys_x86, so it
> +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> +     #
> +	stubname=${entry#__ia32_compat_sys_x86_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> +
> +     #
> +	# The compat entry starts with __ia32_compat_sys, so it is
> +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> +     #
> +	stubname=${entry#__ia32_compat_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    fi;

( Also note the vertical separation that helps see the overall structure a bit 
  better. )

But more fundamentally, that's an awful lot of complex scripting to save 4K 
(unused) kernel text, not sure it's worth it.

Once we grow proper LTO support I'd guess these unused functions would go away 
naturally? None should have a __visible annotation.

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> +emit_stub() {
> +    entry="$1"
> +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> +	# We need a stub named __ia32_sys which is common to 64-bit
> +	# except for a different pt_regs layout.
> +	stubname=${entry#__ia32_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> +	# We need a stub named __x32_compat_sys_ which decodes a
> +	# 64-bit pt_regs and then calls the real syscall function 
> +	stubname="${entry%%/*}" # handle qualifier
> +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> +	# The compat entry starts with __ia32_compat_sys_x86, so it
> +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> +	stubname=${entry#__ia32_compat_sys_x86_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> +	# The compat entry starts with __ia32_compat_sys, so it is
> +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> +	stubname=${entry#__ia32_compat_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    fi;
> +}

I only have a bikeshed painting comment, even in shell scripts please try to 
follow the kernel comment style visually, i.e. something like:

> +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> +
> +	#
> +	# We need a stub named __ia32_sys which is common to 64-bit
> +	# except for a different pt_regs layout.
> +	#
> +	stubname=${entry#__ia32_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> +
> +	#
> +	# We need a stub named __x32_compat_sys_ which decodes a
> +	# 64-bit pt_regs and then calls the real syscall function 
> +     #
> +	stubname="${entry%%/*}" # handle qualifier
> +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> +
> +     #
> +	# The compat entry starts with __ia32_compat_sys_x86, so it
> +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> +     #
> +	stubname=${entry#__ia32_compat_sys_x86_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> +
> +     #
> +	# The compat entry starts with __ia32_compat_sys, so it is
> +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> +     #
> +	stubname=${entry#__ia32_compat_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    fi;

( Also note the vertical separation that helps see the overall structure a bit 
  better. )

But more fundamentally, that's an awful lot of complex scripting to save 4K 
(unused) kernel text, not sure it's worth it.

Once we grow proper LTO support I'd guess these unused functions would go away 
naturally? None should have a __visible annotation.

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Mon, Apr 09, 2018 at 08:45:11AM +0200, Ingo Molnar wrote:
> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> > >   - _____sys_waitid()      # ridiculous number of underscores?
> > >   - __sys_waitid()         # too generic sounding?
> > 
> > ... and we'd need to rename internal helpers in net/
> > 
> > >   - __inline_sys_waitid()  # too long?
> > 
> > sounds acceptable, though a bit long (especially for the compat case, though
> > it doesn't really matter in the case of 
> > __inline_compat_sys_sched_rr_get_interval)
> 
> So as per the previous mail this is not just an inline function, but an active 
> type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
> on some 64-bit architectures.
> 
> And that's a really non-obvious property IMO, and the name should probably reflect 
> _that_ non-obvious property, not the inlining property which is really just a 
> small detail.
> 
> I.e. how about:
> 
> 	__se_sys_waitid()
> 
> ... where 'se' stands for sign-extended, with a comment in the macro that explains 
> the prefix? (The historical abbreviation for sign extension is 'sext', which I 
> think wouldn't really be suitable these days.)

Agreed. Could you do that when applying my patches, please?

Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Mon, Apr 09, 2018 at 08:45:11AM +0200, Ingo Molnar wrote:
> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> > >   - _____sys_waitid()      # ridiculous number of underscores?
> > >   - __sys_waitid()         # too generic sounding?
> > 
> > ... and we'd need to rename internal helpers in net/
> > 
> > >   - __inline_sys_waitid()  # too long?
> > 
> > sounds acceptable, though a bit long (especially for the compat case, though
> > it doesn't really matter in the case of 
> > __inline_compat_sys_sched_rr_get_interval)
> 
> So as per the previous mail this is not just an inline function, but an active 
> type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
> on some 64-bit architectures.
> 
> And that's a really non-obvious property IMO, and the name should probably reflect 
> _that_ non-obvious property, not the inlining property which is really just a 
> small detail.
> 
> I.e. how about:
> 
> 	__se_sys_waitid()
> 
> ... where 'se' stands for sign-extended, with a comment in the macro that explains 
> the prefix? (The historical abbreviation for sign extension is 'sext', which I 
> think wouldn't really be suitable these days.)

Agreed. Could you do that when applying my patches, please?

Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Mon, Apr 09, 2018 at 08:49:45AM +0200, Ingo Molnar wrote:
> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > +emit_stub() {
> > +    entry="$1"
> > +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> > +	# We need a stub named __ia32_sys which is common to 64-bit
> > +	# except for a different pt_regs layout.
> > +	stubname=${entry#__ia32_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> > +	# We need a stub named __x32_compat_sys_ which decodes a
> > +	# 64-bit pt_regs and then calls the real syscall function 
> > +	stubname="${entry%%/*}" # handle qualifier
> > +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> > +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> > +	# The compat entry starts with __ia32_compat_sys_x86, so it
> > +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> > +	stubname=${entry#__ia32_compat_sys_x86_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> > +	# The compat entry starts with __ia32_compat_sys, so it is
> > +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> > +	stubname=${entry#__ia32_compat_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +    fi;
> > +}
> 
> I only have a bikeshed painting comment, even in shell scripts please try to 
> follow the kernel comment style visually, i.e. something like:
> 
> > +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> > +
> > +	#
> > +	# We need a stub named __ia32_sys which is common to 64-bit
> > +	# except for a different pt_regs layout.
> > +	#
> > +	stubname=${entry#__ia32_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +
> > +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> > +
> > +	#
> > +	# We need a stub named __x32_compat_sys_ which decodes a
> > +	# 64-bit pt_regs and then calls the real syscall function 
> > +     #
> > +	stubname="${entry%%/*}" # handle qualifier
> > +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> > +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> > +
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> > +
> > +     #
> > +	# The compat entry starts with __ia32_compat_sys_x86, so it
> > +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> > +     #
> > +	stubname=${entry#__ia32_compat_sys_x86_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> > +
> > +     #
> > +	# The compat entry starts with __ia32_compat_sys, so it is
> > +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> > +     #
> > +	stubname=${entry#__ia32_compat_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +
> > +    fi;
> 
> ( Also note the vertical separation that helps see the overall structure a bit 
>   better. )
> 
> But more fundamentally, that's an awful lot of complex scripting to save 4K 
> (unused) kernel text, not sure it's worth it.

Yes. I just wanted to see myself whether it's worthwile, and it indeed
doesn't seem so. That's exactly why I didn't sign off on it so far...

Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Mon, Apr 09, 2018 at 08:49:45AM +0200, Ingo Molnar wrote:
> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > +emit_stub() {
> > +    entry="$1"
> > +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> > +	# We need a stub named __ia32_sys which is common to 64-bit
> > +	# except for a different pt_regs layout.
> > +	stubname=${entry#__ia32_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> > +	# We need a stub named __x32_compat_sys_ which decodes a
> > +	# 64-bit pt_regs and then calls the real syscall function 
> > +	stubname="${entry%%/*}" # handle qualifier
> > +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> > +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> > +	# The compat entry starts with __ia32_compat_sys_x86, so it
> > +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> > +	stubname=${entry#__ia32_compat_sys_x86_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> > +	# The compat entry starts with __ia32_compat_sys, so it is
> > +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> > +	stubname=${entry#__ia32_compat_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +    fi;
> > +}
> 
> I only have a bikeshed painting comment, even in shell scripts please try to 
> follow the kernel comment style visually, i.e. something like:
> 
> > +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> > +
> > +	#
> > +	# We need a stub named __ia32_sys which is common to 64-bit
> > +	# except for a different pt_regs layout.
> > +	#
> > +	stubname=${entry#__ia32_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +
> > +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> > +
> > +	#
> > +	# We need a stub named __x32_compat_sys_ which decodes a
> > +	# 64-bit pt_regs and then calls the real syscall function 
> > +     #
> > +	stubname="${entry%%/*}" # handle qualifier
> > +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> > +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> > +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> > +
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> > +
> > +     #
> > +	# The compat entry starts with __ia32_compat_sys_x86, so it
> > +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> > +     #
> > +	stubname=${entry#__ia32_compat_sys_x86_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +
> > +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> > +
> > +     #
> > +	# The compat entry starts with __ia32_compat_sys, so it is
> > +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> > +     #
> > +	stubname=${entry#__ia32_compat_sys_}
> > +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> > +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> > +
> > +    fi;
> 
> ( Also note the vertical separation that helps see the overall structure a bit 
>   better. )
> 
> But more fundamentally, that's an awful lot of complex scripting to save 4K 
> (unused) kernel text, not sure it's worth it.

Yes. I just wanted to see myself whether it's worthwile, and it indeed
doesn't seem so. That's exactly why I didn't sign off on it so far...

Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> > >   - _____sys_waitid()      # ridiculous number of underscores?
> > >   - __sys_waitid()         # too generic sounding?
> > 
> > ... and we'd need to rename internal helpers in net/
> > 
> > >   - __inline_sys_waitid()  # too long?
> > 
> > sounds acceptable, though a bit long (especially for the compat case, though
> > it doesn't really matter in the case of 
> > __inline_compat_sys_sched_rr_get_interval)
> 
> So as per the previous mail this is not just an inline function, but an active 
> type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
> on some 64-bit architectures.
> 
> And that's a really non-obvious property IMO, and the name should probably reflect 
> _that_ non-obvious property, not the inlining property which is really just a 
> small detail.
> 
> I.e. how about:
> 
> 	__se_sys_waitid()
> 
> ... where 'se' stands for sign-extended, with a comment in the macro that explains 
> the prefix? (The historical abbreviation for sign extension is 'sext', which I 
> think wouldn't really be suitable these days.)

Ok, so I got confused there: I think it's the do_sys_waitid() intermediate that
is actually doing the sign-extension - and the inlined helper is what is in the 
syscall definition body.

So it's all still somewhat of a confusing misnomer: the 'do' named function is 
actually the sign-extension function variant - and the '_il' variant actually 
'does' the real work ...

I.e., old naming:

810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)

<inline>     __il_sys_waitid	# inlined helper doing the actual work
				# (takes parameters as declared)

810f1aa0 T   __do_sys_waitid	# C function calling inlined helper
				# (takes parameters of type long; casts
				#  them to the declared type)

810f1aa0 T        sys_waitid	# alias to __do_sys_waitid() (taking
				# parameters as declared), to be included
				# in syscall table


New suggested naming:

810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)

<inline>     __do_sys_waitid	# inlined helper doing the actual work
				# (takes original parameters as declared)

810f1aa0 T   __se_sys_waitid	# sign-extending C function calling inlined
				# helper (takes parameters of type long;
				# casts them to the declared type)

810f1aa0 T        sys_waitid	# alias to __se_sys_waitid() (but taking
				# original parameters as declared), to be
				# included in syscall table

Agreed?

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> > >   - _____sys_waitid()      # ridiculous number of underscores?
> > >   - __sys_waitid()         # too generic sounding?
> > 
> > ... and we'd need to rename internal helpers in net/
> > 
> > >   - __inline_sys_waitid()  # too long?
> > 
> > sounds acceptable, though a bit long (especially for the compat case, though
> > it doesn't really matter in the case of 
> > __inline_compat_sys_sched_rr_get_interval)
> 
> So as per the previous mail this is not just an inline function, but an active 
> type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
> on some 64-bit architectures.
> 
> And that's a really non-obvious property IMO, and the name should probably reflect 
> _that_ non-obvious property, not the inlining property which is really just a 
> small detail.
> 
> I.e. how about:
> 
> 	__se_sys_waitid()
> 
> ... where 'se' stands for sign-extended, with a comment in the macro that explains 
> the prefix? (The historical abbreviation for sign extension is 'sext', which I 
> think wouldn't really be suitable these days.)

Ok, so I got confused there: I think it's the do_sys_waitid() intermediate that
is actually doing the sign-extension - and the inlined helper is what is in the 
syscall definition body.

So it's all still somewhat of a confusing misnomer: the 'do' named function is 
actually the sign-extension function variant - and the '_il' variant actually 
'does' the real work ...

I.e., old naming:

810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)

<inline>     __il_sys_waitid	# inlined helper doing the actual work
				# (takes parameters as declared)

810f1aa0 T   __do_sys_waitid	# C function calling inlined helper
				# (takes parameters of type long; casts
				#  them to the declared type)

810f1aa0 T        sys_waitid	# alias to __do_sys_waitid() (taking
				# parameters as declared), to be included
				# in syscall table


New suggested naming:

810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)

<inline>     __do_sys_waitid	# inlined helper doing the actual work
				# (takes original parameters as declared)

810f1aa0 T   __se_sys_waitid	# sign-extending C function calling inlined
				# helper (takes parameters of type long;
				# casts them to the declared type)

810f1aa0 T        sys_waitid	# alias to __se_sys_waitid() (but taking
				# original parameters as declared), to be
				# included in syscall table

Agreed?

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Mon, Apr 09, 2018 at 09:06:11AM +0200, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> > 
> > * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> > 
> > > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> > > >   - _____sys_waitid()      # ridiculous number of underscores?
> > > >   - __sys_waitid()         # too generic sounding?
> > > 
> > > ... and we'd need to rename internal helpers in net/
> > > 
> > > >   - __inline_sys_waitid()  # too long?
> > > 
> > > sounds acceptable, though a bit long (especially for the compat case, though
> > > it doesn't really matter in the case of 
> > > __inline_compat_sys_sched_rr_get_interval)
> > 
> > So as per the previous mail this is not just an inline function, but an active 
> > type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
> > on some 64-bit architectures.
> > 
> > And that's a really non-obvious property IMO, and the name should probably reflect 
> > _that_ non-obvious property, not the inlining property which is really just a 
> > small detail.
> > 
> > I.e. how about:
> > 
> > 	__se_sys_waitid()
> > 
> > ... where 'se' stands for sign-extended, with a comment in the macro that explains 
> > the prefix? (The historical abbreviation for sign extension is 'sext', which I 
> > think wouldn't really be suitable these days.)
> 
> Ok, so I got confused there: I think it's the do_sys_waitid() intermediate that
> is actually doing the sign-extension - and the inlined helper is what is in the 
> syscall definition body.
> 
> So it's all still somewhat of a confusing misnomer: the 'do' named function is 
> actually the sign-extension function variant - and the '_il' variant actually 
> 'does' the real work ...
> 
> I.e., old naming:
> 
> 810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)
> 
> <inline>     __il_sys_waitid	# inlined helper doing the actual work
> 				# (takes parameters as declared)
> 
> 810f1aa0 T   __do_sys_waitid	# C function calling inlined helper
> 				# (takes parameters of type long; casts
> 				#  them to the declared type)
> 
> 810f1aa0 T        sys_waitid	# alias to __do_sys_waitid() (taking
> 				# parameters as declared), to be included
> 				# in syscall table
> 
> 
> New suggested naming:
> 
> 810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)
> 
> <inline>     __do_sys_waitid	# inlined helper doing the actual work
> 				# (takes original parameters as declared)
> 
> 810f1aa0 T   __se_sys_waitid	# sign-extending C function calling inlined
> 				# helper (takes parameters of type long;
> 				# casts them to the declared type)
> 
> 810f1aa0 T        sys_waitid	# alias to __se_sys_waitid() (but taking
> 				# original parameters as declared), to be
> 				# included in syscall table
> 
> Agreed?

Yes.

Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Dominik Brodowski]

On Mon, Apr 09, 2018 at 09:06:11AM +0200, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> > 
> > * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> > 
> > > On Sun, Apr 08, 2018 at 10:35:50AM +0200, Ingo Molnar wrote:
> > > >   - _____sys_waitid()      # ridiculous number of underscores?
> > > >   - __sys_waitid()         # too generic sounding?
> > > 
> > > ... and we'd need to rename internal helpers in net/
> > > 
> > > >   - __inline_sys_waitid()  # too long?
> > > 
> > > sounds acceptable, though a bit long (especially for the compat case, though
> > > it doesn't really matter in the case of 
> > > __inline_compat_sys_sched_rr_get_interval)
> > 
> > So as per the previous mail this is not just an inline function, but an active 
> > type conversion wrapper that sign-extends 32-bit ints to longs, which is important 
> > on some 64-bit architectures.
> > 
> > And that's a really non-obvious property IMO, and the name should probably reflect 
> > _that_ non-obvious property, not the inlining property which is really just a 
> > small detail.
> > 
> > I.e. how about:
> > 
> > 	__se_sys_waitid()
> > 
> > ... where 'se' stands for sign-extended, with a comment in the macro that explains 
> > the prefix? (The historical abbreviation for sign extension is 'sext', which I 
> > think wouldn't really be suitable these days.)
> 
> Ok, so I got confused there: I think it's the do_sys_waitid() intermediate that
> is actually doing the sign-extension - and the inlined helper is what is in the 
> syscall definition body.
> 
> So it's all still somewhat of a confusing misnomer: the 'do' named function is 
> actually the sign-extension function variant - and the '_il' variant actually 
> 'does' the real work ...
> 
> I.e., old naming:
> 
> 810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)
> 
> <inline>     __il_sys_waitid	# inlined helper doing the actual work
> 				# (takes parameters as declared)
> 
> 810f1aa0 T   __do_sys_waitid	# C function calling inlined helper
> 				# (takes parameters of type long; casts
> 				#  them to the declared type)
> 
> 810f1aa0 T        sys_waitid	# alias to __do_sys_waitid() (taking
> 				# parameters as declared), to be included
> 				# in syscall table
> 
> 
> New suggested naming:
> 
> 810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)
> 
> <inline>     __do_sys_waitid	# inlined helper doing the actual work
> 				# (takes original parameters as declared)
> 
> 810f1aa0 T   __se_sys_waitid	# sign-extending C function calling inlined
> 				# helper (takes parameters of type long;
> 				# casts them to the declared type)
> 
> 810f1aa0 T        sys_waitid	# alias to __se_sys_waitid() (but taking
> 				# original parameters as declared), to be
> 				# included in syscall table
> 
> Agreed?

Yes.

Thanks,
	Dominik

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> > New suggested naming:
> > 
> > 810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)
> > 
> > <inline>     __do_sys_waitid	# inlined helper doing the actual work
> > 				# (takes original parameters as declared)
> > 
> > 810f1aa0 T   __se_sys_waitid	# sign-extending C function calling inlined
> > 				# helper (takes parameters of type long;
> > 				# casts them to the declared type)
> > 
> > 810f1aa0 T        sys_waitid	# alias to __se_sys_waitid() (but taking
> > 				# original parameters as declared), to be
> > 				# included in syscall table
> > 
> > Agreed?
> 
> Yes.

Ok, great. Since these re-renames look complex enough, mind re-sending the series 
with these changes incorporated and the 4/3 patch incorporated as well (and the 
image shrinking script left out)?

The base patches are looking good here so I plan sending these to Linus 
tomorrow-ish. (The renames don't affect functionality so they don't need
as much testing.)

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> > New suggested naming:
> > 
> > 810f08d0 t     kernel_waitid	# common C function (see kernel/exit.c)
> > 
> > <inline>     __do_sys_waitid	# inlined helper doing the actual work
> > 				# (takes original parameters as declared)
> > 
> > 810f1aa0 T   __se_sys_waitid	# sign-extending C function calling inlined
> > 				# helper (takes parameters of type long;
> > 				# casts them to the declared type)
> > 
> > 810f1aa0 T        sys_waitid	# alias to __se_sys_waitid() (but taking
> > 				# original parameters as declared), to be
> > 				# included in syscall table
> > 
> > Agreed?
> 
> Yes.

Ok, great. Since these re-renames look complex enough, mind re-sending the series 
with these changes incorporated and the 4/3 patch incorporated as well (and the 
image shrinking script left out)?

The base patches are looking good here so I plan sending these to Linus 
tomorrow-ish. (The renames don't affect functionality so they don't need
as much testing.)

Thanks,

	Ingo

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Linus Torvalds]

On Sun, Apr 8, 2018 at 11:39 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> I see - so it's _not_ the same function call signature, but a wrapper with a
> sign-extended version, which is fair and useful. So on architectures where this
> matters there's type conversion and active code generated.

Exactly. Some architectures have that as part of their ABI.

On Powerpc, for example, when you pass an "int" argument, the ABI
specifies that you have to sign-extend the register in the caller to
64 bits.

And the generated code actually depends on that behavior, in that
maybe it first tests the 32 bit value, but then _uses_ the full 64
bits, knowing that the caller sign-extended it properly.

This is a problem with the system call interface, since the caller
isn't a trusted entity, and user space could pass an "int" value with
the high bits set to something that _isn't_ the sign-extended thing,
so then the compiler generates unsafe code.

On x86, this never happens, since x86 doesn't have that "hidden higher
bits matter" ABI model. So that part of the wrapper will be a complete
no-op. But some other architectures depend on it.

               Linus

Re: [PATCH 0/3] syscalls: clean up stub naming convention

[Linus Torvalds]

On Sun, Apr 8, 2018 at 11:39 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> I see - so it's _not_ the same function call signature, but a wrapper with a
> sign-extended version, which is fair and useful. So on architectures where this
> matters there's type conversion and active code generated.

Exactly. Some architectures have that as part of their ABI.

On Powerpc, for example, when you pass an "int" argument, the ABI
specifies that you have to sign-extend the register in the caller to
64 bits.

And the generated code actually depends on that behavior, in that
maybe it first tests the 32 bit value, but then _uses_ the full 64
bits, knowing that the caller sign-extended it properly.

This is a problem with the system call interface, since the caller
isn't a trusted entity, and user space could pass an "int" value with
the high bits set to something that _isn't_ the sign-extended thing,
so then the compiler generates unsafe code.

On x86, this never happens, since x86 doesn't have that "hidden higher
bits matter" ABI model. So that part of the wrapper will be a complete
no-op. But some other architectures depend on it.

               Linus