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

From: Ingo Molnar
Date: Mon Apr 09 2018 - 02:49:57 EST



* Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx> 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