From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH 0/3] syscalls: clean up stub naming convention Date: Mon, 9 Apr 2018 08:49:45 +0200 Message-ID: <20180409064945.wpj4knmalpb4fm2t@gmail.com> References: <20180407074651.29014-1-linux@dominikbrodowski.net> <20180408083550.32d65b6ra6yca5p7@gmail.com> <20180408091536.GA10120@light.dominikbrodowski.net> <20180408195952.GA27462@light.dominikbrodowski.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20180408195952.GA27462@light.dominikbrodowski.net> Sender: linux-kernel-owner@vger.kernel.org To: Dominik Brodowski Cc: linux-kernel@vger.kernel.org, Al Viro , Andi Kleen , Andrew Morton , Andy Lutomirski , Brian Gerst , Denys Vlasenko , "H. Peter Anvin" , Ingo Molnar , Linus Torvalds , Peter Zijlstra , Thomas Gleixner , x86@kernel.org, Maninder Singh , Arnd Bergmann , linux-arch List-Id: linux-arch.vger.kernel.org * Dominik Brodowski 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com ([209.85.128.194]:39290 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750877AbeDIGtt (ORCPT ); Mon, 9 Apr 2018 02:49:49 -0400 Date: Mon, 9 Apr 2018 08:49:45 +0200 From: Ingo Molnar Subject: Re: [PATCH 0/3] syscalls: clean up stub naming convention Message-ID: <20180409064945.wpj4knmalpb4fm2t@gmail.com> References: <20180407074651.29014-1-linux@dominikbrodowski.net> <20180408083550.32d65b6ra6yca5p7@gmail.com> <20180408091536.GA10120@light.dominikbrodowski.net> <20180408195952.GA27462@light.dominikbrodowski.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180408195952.GA27462@light.dominikbrodowski.net> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Dominik Brodowski Cc: linux-kernel@vger.kernel.org, Al Viro , Andi Kleen , Andrew Morton , Andy Lutomirski , Brian Gerst , Denys Vlasenko , "H. Peter Anvin" , Ingo Molnar , Linus Torvalds , Peter Zijlstra , Thomas Gleixner , x86@kernel.org, Maninder Singh , Arnd Bergmann , linux-arch Message-ID: <20180409064945.arwSpK7yzstQnwhqZMBbqTnr7YGo3dtxgsvoH86VyFk@z> * Dominik Brodowski 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