All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org,
	joro@8bytes.org, boris.ostrovsky@oracle.com, jgross@suse.com,
	x86@kernel.org, mbenes@suse.com, rostedt@goodmis.org,
	dvyukov@google.com, elver@google.com, sam@ravnborg.org,
	masahiroy@kernel.org, hca@linux.ibm.com, nickhu@andestech.com,
	vgupta@kernel.org
Subject: Re: [PATCH v2 06/24] x86: Always inline context_tracking_guest_enter()
Date: Wed, 25 Aug 2021 14:16:01 +0200	[thread overview]
Message-ID: <YSY0gSLyTL3kIfP8@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20210820233440.jjzdw6qjnytonlmv@treble>

On Fri, Aug 20, 2021 at 04:34:40PM -0700, Josh Poimboeuf wrote:

> WARNING: This patch may shrink your patch set and improve your sanity.

I'll go rebuild me some kernels to see how many __always_inline things I
can drop.

Lets add a few Cc's though.

> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] kbuild: Remove CONFIG_DEBUG_SECTION_MISMATCH
> 
> Modpost's section mismatch detection warns when a non-init function
> references an __init function.  The warning is needed because __init
> memory is freed during boot, so the non-init function might end up
> calling into some random memory.
> 
> CONFIG_DEBUG_SECTION_MISMATCH is intended to root out more of these
> issues by adding the -fno-inline-functions-called-once compiler flag,
> which forces once-called static functions to not be inlined.  As
> described in the option's help description:
> 
>   - Add the option -fno-inline-functions-called-once to gcc commands.
>     When inlining a function annotated with __init in a non-init
>     function, we would lose the section information and thus
>     the analysis would not catch the illegal reference.
>     This option tells gcc to inline less (but it does result in
>     a larger kernel).
> 
> It's true that if an __init function is inlined in a non-init function,
> it's no longer __init.  But that's not actually a problem.
> 
> If an inlined __init function doesn't access any other __init functions
> or data, the inlining is harmless.
> 
> If the inlined __init function does access other __init functions/data,
> __init relocations will be generated for a non-init section, and
> modpost's section mismatch detection will discover it.
> 
> So the option isn't actually useful.  It's not even useful for finding
> issues on other configs, which might have different inlining decisions,
> because with -O2, once-called static functions are always inlined.  And
> anyway, nowadays we have build bots everywhere, which are great at
> finding such edge cases.
> 
> I get the feeling this option has been responsible for dozens of
> "convert inline to __always_inline" patches over the years.  Such
> patches usually complain about the compiler's inlining decisions being
> unpredictable.  Turns out the culprit is this
> CONFIG_DEBUG_SECTION_MISMATCH option, at least for once-called
> functions.
> 
> This option really has no benefits, and only drawbacks, causing needless
> churn.  Remove it.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  Makefile                                      |  5 -----
>  arch/arc/configs/tb10x_defconfig              |  1 -
>  arch/nds32/configs/defconfig                  |  1 -
>  arch/s390/configs/debug_defconfig             |  1 -
>  arch/s390/configs/defconfig                   |  1 -
>  lib/Kconfig.debug                             | 21 -------------------
>  .../selftests/wireguard/qemu/debug.config     |  1 -
>  7 files changed, 31 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c19d1638da25..195d4f4914e3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -913,11 +913,6 @@ KBUILD_CFLAGS	+= $(CC_FLAGS_FTRACE) $(CC_FLAGS_USING)
>  KBUILD_AFLAGS	+= $(CC_FLAGS_USING)
>  endif
>  
> -# We trigger additional mismatches with less inlining
> -ifdef CONFIG_DEBUG_SECTION_MISMATCH
> -KBUILD_CFLAGS += $(call cc-option, -fno-inline-functions-called-once)
> -endif
> -
>  ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>  KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
>  LDFLAGS_vmlinux += --gc-sections
> diff --git a/arch/arc/configs/tb10x_defconfig b/arch/arc/configs/tb10x_defconfig
> index a12656ec0072..5acf8cc3e7b0 100644
> --- a/arch/arc/configs/tb10x_defconfig
> +++ b/arch/arc/configs/tb10x_defconfig
> @@ -96,7 +96,6 @@ CONFIG_STRIP_ASM_SYMS=y
>  CONFIG_DEBUG_FS=y
>  CONFIG_HEADERS_INSTALL=y
>  CONFIG_HEADERS_CHECK=y
> -CONFIG_DEBUG_SECTION_MISMATCH=y
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_MEMORY_INIT=y
>  CONFIG_DEBUG_STACKOVERFLOW=y
> diff --git a/arch/nds32/configs/defconfig b/arch/nds32/configs/defconfig
> index f9a89cf00aa6..61b5b83bd20e 100644
> --- a/arch/nds32/configs/defconfig
> +++ b/arch/nds32/configs/defconfig
> @@ -93,7 +93,6 @@ CONFIG_GDB_SCRIPTS=y
>  CONFIG_READABLE_ASM=y
>  CONFIG_HEADERS_INSTALL=y
>  CONFIG_HEADERS_CHECK=y
> -CONFIG_DEBUG_SECTION_MISMATCH=y
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_KERNEL=y
>  CONFIG_PANIC_ON_OOPS=y
> diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
> index b88184019af9..68104f82cc9d 100644
> --- a/arch/s390/configs/debug_defconfig
> +++ b/arch/s390/configs/debug_defconfig
> @@ -782,7 +782,6 @@ CONFIG_DEBUG_INFO=y
>  CONFIG_DEBUG_INFO_DWARF4=y
>  CONFIG_GDB_SCRIPTS=y
>  CONFIG_HEADERS_INSTALL=y
> -CONFIG_DEBUG_SECTION_MISMATCH=y
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_PAGEALLOC=y
>  CONFIG_PAGE_OWNER=y
> diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
> index 1667a3cdcf0a..a5e7eb0276c6 100644
> --- a/arch/s390/configs/defconfig
> +++ b/arch/s390/configs/defconfig
> @@ -767,7 +767,6 @@ CONFIG_DYNAMIC_DEBUG=y
>  CONFIG_DEBUG_INFO=y
>  CONFIG_DEBUG_INFO_DWARF4=y
>  CONFIG_GDB_SCRIPTS=y
> -CONFIG_DEBUG_SECTION_MISMATCH=y
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_WX=y
>  CONFIG_PTDUMP_DEBUGFS=y
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index e5cdf98f50c2..93f9c00783b2 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -381,27 +381,6 @@ config HEADERS_INSTALL
>  	  user-space program samples. It is also needed by some features such
>  	  as uapi header sanity checks.
>  
> -config DEBUG_SECTION_MISMATCH
> -	bool "Enable full Section mismatch analysis"
> -	help
> -	  The section mismatch analysis checks if there are illegal
> -	  references from one section to another section.
> -	  During linktime or runtime, some sections are dropped;
> -	  any use of code/data previously in these sections would
> -	  most likely result in an oops.
> -	  In the code, functions and variables are annotated with
> -	  __init,, etc. (see the full list in include/linux/init.h),
> -	  which results in the code/data being placed in specific sections.
> -	  The section mismatch analysis is always performed after a full
> -	  kernel build, and enabling this option causes the following
> -	  additional step to occur:
> -	  - Add the option -fno-inline-functions-called-once to gcc commands.
> -	    When inlining a function annotated with __init in a non-init
> -	    function, we would lose the section information and thus
> -	    the analysis would not catch the illegal reference.
> -	    This option tells gcc to inline less (but it does result in
> -	    a larger kernel).
> -
>  config SECTION_MISMATCH_WARN_ONLY
>  	bool "Make section mismatch errors non-fatal"
>  	default y
> diff --git a/tools/testing/selftests/wireguard/qemu/debug.config b/tools/testing/selftests/wireguard/qemu/debug.config
> index fe07d97df9fa..83588bb08fde 100644
> --- a/tools/testing/selftests/wireguard/qemu/debug.config
> +++ b/tools/testing/selftests/wireguard/qemu/debug.config
> @@ -57,7 +57,6 @@ CONFIG_USER_STACKTRACE_SUPPORT=y
>  CONFIG_DEBUG_SG=y
>  CONFIG_DEBUG_NOTIFIERS=y
>  CONFIG_X86_DEBUG_FPU=y
> -CONFIG_DEBUG_SECTION_MISMATCH=y
>  CONFIG_DEBUG_PAGEALLOC=y
>  CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y
>  CONFIG_DEBUG_WW_MUTEX_SLOWPATH=y
> -- 
> 2.31.1
> 

  reply	other threads:[~2021-08-25 12:18 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24  9:40 [PATCH v2 00/24] objtool/x86: noinstr vs PARAVIRT Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 01/24] x86/xen: Mark cpu_bringup_and_idle() as dead_end_function Peter Zijlstra
2021-06-24 10:49   ` Juergen Gross
2021-06-30 11:47   ` Miroslav Benes
2021-08-20 19:22   ` Josh Poimboeuf
2021-08-20 19:31     ` Josh Poimboeuf
2021-08-23  8:40       ` Juergen Gross
2021-08-30  5:55         ` Juergen Gross
2021-08-30  7:48           ` Peter Zijlstra
2021-08-30  7:56             ` Juergen Gross
2021-09-15 15:49       ` [tip: objtool/core] x86/xen: Move hypercall_page to top of the file tip-bot2 for Josh Poimboeuf
2021-09-15 15:49   ` [tip: objtool/core] x86/xen: Mark cpu_bringup_and_idle() as dead_end_function tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 02/24] objtool: Introduce CFI hash Peter Zijlstra
2021-06-30 12:45   ` Miroslav Benes
2021-07-01  8:35     ` Peter Zijlstra
2021-07-01  8:53       ` Miroslav Benes
2021-08-20 22:27   ` Josh Poimboeuf
2021-08-25 10:13     ` Peter Zijlstra
2021-08-25 12:49       ` Peter Zijlstra
2021-08-25 10:15     ` Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 03/24] objtool: Handle __sanitize_cov*() tail calls Peter Zijlstra
2021-06-25 11:05   ` Marco Elver
2021-06-28  9:07     ` Peter Zijlstra
2021-06-25 13:38   ` Steven Rostedt
2021-06-26  8:18     ` Peter Zijlstra
2021-06-29  1:07       ` Steven Rostedt
2021-08-20 23:17   ` Josh Poimboeuf
2021-08-25 10:19     ` Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 04/24] x86/kvm: Always inline sev_*guest() Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 05/24] x86/kvm: Always inline vmload() / vmsave() Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 06/24] x86: Always inline context_tracking_guest_enter() Peter Zijlstra
2021-08-20 23:22   ` Josh Poimboeuf
2021-08-20 23:34     ` Josh Poimboeuf
2021-08-25 12:16       ` Peter Zijlstra [this message]
     [not found]         ` <YSZxtBHNJzoUbqdE@ravnborg.org>
2021-08-26  4:10           ` Josh Poimboeuf
2021-08-25 13:38       ` Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 07/24] x86/kvm: Always inline to_svm() Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 08/24] x86/kvm: Always inline evmcs_write64() Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 09/24] x86: Always inline ip_within_syscall_gap() Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 10/24] x86/sev: Fix noinstr for vc_ghcb_invalidate() Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 11/24] locking/lockdep: Avoid RCU-induced noinstr fail Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 12/24] x86/paravirt: Mark arch_local_irq_*() __always_inline Peter Zijlstra
2021-06-24 10:51   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 13/24] x86/paravirt: Use PVOP_* for paravirt calls Peter Zijlstra
2021-06-24 10:52   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 14/24] x86/xen: Make read_cr2() noinstr Peter Zijlstra
2021-06-24 10:55   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 15/24] x86/xen: Make write_cr2() noinstr Peter Zijlstra
2021-06-24 10:56   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 16/24] x86/xen: Make get_debugreg() noinstr Peter Zijlstra
2021-06-24 10:57   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 17/24] x86/xen: Make set_debugreg() noinstr Peter Zijlstra
2021-06-24 10:59   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 18/24] x86/xen: Make save_fl() noinstr Peter Zijlstra
2021-06-24 11:07   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 19/24] x86/xen: Make hypercall_page noinstr Peter Zijlstra
2021-06-24 11:08   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 20/24] x86/xen: Make irq_enable() noinstr Peter Zijlstra
2021-06-24 11:09   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 21/24] x86/xen: Make irq_disable() noinstr Peter Zijlstra
2021-06-24 11:10   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 22/24] x86/xen: Mark xen_force_evtchn_callback() noinstr Peter Zijlstra
2021-06-24 11:11   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 23/24] x86/xen: Rework the xen_{cpu,irq,mmu}_ops[] arrays Peter Zijlstra
2021-06-24 12:12   ` Juergen Gross
2021-09-15 15:49   ` [tip: objtool/core] x86/xen: Rework the xen_{cpu,irq,mmu}_opsarrays tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24  9:41 ` [PATCH v2 24/24] objtool: Support pv_opsindirect calls for noinstr Peter Zijlstra
2021-09-15 15:49   ` [tip: objtool/core] " tip-bot2 for Peter Zijlstra
2021-09-17 12:58   ` tip-bot2 for Peter Zijlstra
2021-06-24 13:05 ` [PATCH v2 00/24] objtool/x86: noinstr vs PARAVIRT Steven Rostedt

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=YSY0gSLyTL3kIfP8@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=boris.ostrovsky@oracle.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=hca@linux.ibm.com \
    --cc=jgross@suse.com \
    --cc=joro@8bytes.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=mbenes@suse.com \
    --cc=nickhu@andestech.com \
    --cc=rostedt@goodmis.org \
    --cc=sam@ravnborg.org \
    --cc=tglx@linutronix.de \
    --cc=vgupta@kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.