From: Ingo Molnar <mingo@kernel.org>
To: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: linux-kernel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
Andi Kleen <ak@linux.intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
Andy Lutomirski <luto@kernel.org>,
Brian Gerst <brgerst@gmail.com>,
Denys Vlasenko <dvlasenk@redhat.com>,
"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
x86@kernel.org, Maninder Singh <maninder1.s@samsung.com>,
Arnd Bergmann <arnd@arndb.de>,
linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [PATCH 0/3] syscalls: clean up stub naming convention
Date: Mon, 9 Apr 2018 08:49:45 +0200 [thread overview]
Message-ID: <20180409064945.wpj4knmalpb4fm2t@gmail.com> (raw)
In-Reply-To: <20180408195952.GA27462@light.dominikbrodowski.net>
* 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
next prev parent reply other threads:[~2018-04-09 6:49 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-07 7:46 [PATCH 0/3] syscalls: clean up stub naming convention Dominik Brodowski
2018-04-07 7:46 ` Dominik Brodowski
2018-04-08 8:35 ` Ingo Molnar
2018-04-08 8:35 ` Ingo Molnar
2018-04-08 9:15 ` Dominik Brodowski
2018-04-08 9:15 ` Dominik Brodowski
2018-04-08 19:59 ` Dominik Brodowski
2018-04-08 19:59 ` Dominik Brodowski
2018-04-08 20:06 ` [PATCH 4/3] syscalls/x86: adapt syscall_wrapper.h to the new syscall " Dominik Brodowski
2018-04-08 20:06 ` Dominik Brodowski
2018-04-09 6:49 ` Ingo Molnar [this message]
2018-04-09 6:49 ` [PATCH 0/3] syscalls: clean up " Ingo Molnar
2018-04-09 6:54 ` Dominik Brodowski
2018-04-09 6:54 ` Dominik Brodowski
2018-04-09 6:39 ` Ingo Molnar
2018-04-09 6:39 ` Ingo Molnar
2018-04-09 19:13 ` Linus Torvalds
2018-04-09 19:13 ` Linus Torvalds
2018-04-09 6:45 ` Ingo Molnar
2018-04-09 6:45 ` Ingo Molnar
2018-04-09 6:53 ` Dominik Brodowski
2018-04-09 6:53 ` Dominik Brodowski
2018-04-09 7:06 ` Ingo Molnar
2018-04-09 7:06 ` Ingo Molnar
2018-04-09 7:08 ` Dominik Brodowski
2018-04-09 7:08 ` Dominik Brodowski
2018-04-09 7:34 ` Ingo Molnar
2018-04-09 7:34 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180409064945.wpj4knmalpb4fm2t@gmail.com \
--to=mingo@kernel.org \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=brgerst@gmail.com \
--cc=dvlasenk@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=luto@kernel.org \
--cc=maninder1.s@samsung.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox