From: Sami Tolvanen <samitolvanen@google.com> To: Mark Rutland <mark.rutland@arm.com> Cc: Masahiro Yamada <masahiroy@kernel.org>, Will Deacon <will@kernel.org>, Steven Rostedt <rostedt@goodmis.org>, Peter Zijlstra <peterz@infradead.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, "Paul E. McKenney" <paulmck@kernel.org>, Kees Cook <keescook@chromium.org>, Nick Desaulniers <ndesaulniers@google.com>, clang-built-linux <clang-built-linux@googlegroups.com>, Kernel Hardening <kernel-hardening@lists.openwall.com>, linux-arch <linux-arch@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, linux-kbuild <linux-kbuild@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, linux-pci@vger.kernel.org, X86 ML <x86@kernel.org> Subject: Re: [PATCH v4 09/29] arm64: disable recordmcount with DYNAMIC_FTRACE_WITH_REGS Date: Wed, 30 Sep 2020 10:10:47 -0700 [thread overview] Message-ID: <CABCJKuegb4MzniWOk2+R3FngZpdWuSEAZuj=arRm0mE6HQ9anw@mail.gmail.com> (raw) In-Reply-To: <20200930095850.GA68612@C02TD0UTHF1T.local> Hi Mark, On Wed, Sep 30, 2020 at 2:59 AM Mark Rutland <mark.rutland@arm.com> wrote: > > Hi Sami, > > On Tue, Sep 29, 2020 at 02:46:11PM -0700, Sami Tolvanen wrote: > > Select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY to disable > > recordmcount when DYNAMIC_FTRACE_WITH_REGS is selected. > > Could you please add an explanation as to /why/ this is necessary in the > commit message? I couldn't figure this out form the commit message > alone, and reading the cover letter also didn't help. Sorry about that, I'll add a better explanation in the next version. Note that without LTO, this change is not strictly necessary as there's no harm in running recordmcount even if it's not needed. It might slow down the build slightly, but I suspect a few thousand invocations of the program won't take that long. However, with LTO we need to disable recordmcount because it doesn't understand LLVM bitcode. > If the minimum required GCC version supports patchable-function-entry > I'd be happy to make that a requirement for dynamic ftrace on arm64, as > then we'd only need to support one mechanism, and can get rid of some > redundant code. We already default to it when present anyhow. That would be great, but Documentation/process/changes.rst suggests the minimum gcc version is 4.9, and according to Godbolt we would need gcc >= 8 for -fpatchable-function-entry: https://godbolt.org/z/jdzcMW > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index 6d232837cbee..ad522b021f35 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -155,6 +155,8 @@ config ARM64 > > select HAVE_DYNAMIC_FTRACE > > select HAVE_DYNAMIC_FTRACE_WITH_REGS \ > > if $(cc-option,-fpatchable-function-entry=2) > > + select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY \ > > + if DYNAMIC_FTRACE_WITH_REGS > > This doesn't look quite right to me. Presumably we shouldn't allow > DYNAMIC_FTRACE_WITH_REGS to be selected if HAVE_DYNAMIC_FTRACE_WITH_REGS > isn't. This won't allow DYNAMIC_FTRACE_WITH_REGS to be selected without HAVE_DYNAMIC_FTRACE_WITH_REGS. Testing with a compiler that does support -fpatchable-function-entry, I get the following, as expected: $ grep -E '(DYNAMIC_FTRACE|MCOUNT_USE)' .config CONFIG_HAVE_DYNAMIC_FTRACE=y CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS=y CONFIG_DYNAMIC_FTRACE=y CONFIG_DYNAMIC_FTRACE_WITH_REGS=y CONFIG_FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY=y And if the compiler doesn't support -fpatchable-function-entry, we would end up with the following: $ grep -E '(DYNAMIC_FTRACE|MCOUNT_USE)' .config CONFIG_HAVE_DYNAMIC_FTRACE=y CONFIG_DYNAMIC_FTRACE=y CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT=y Sami
WARNING: multiple messages have this Message-ID (diff)
From: Sami Tolvanen <samitolvanen@google.com> To: Mark Rutland <mark.rutland@arm.com> Cc: linux-arch <linux-arch@vger.kernel.org>, X86 ML <x86@kernel.org>, Kees Cook <keescook@chromium.org>, "Paul E. McKenney" <paulmck@kernel.org>, Kernel Hardening <kernel-hardening@lists.openwall.com>, Peter Zijlstra <peterz@infradead.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Masahiro Yamada <masahiroy@kernel.org>, linux-kbuild <linux-kbuild@vger.kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, LKML <linux-kernel@vger.kernel.org>, Steven Rostedt <rostedt@goodmis.org>, clang-built-linux <clang-built-linux@googlegroups.com>, linux-pci@vger.kernel.org, Will Deacon <will@kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v4 09/29] arm64: disable recordmcount with DYNAMIC_FTRACE_WITH_REGS Date: Wed, 30 Sep 2020 10:10:47 -0700 [thread overview] Message-ID: <CABCJKuegb4MzniWOk2+R3FngZpdWuSEAZuj=arRm0mE6HQ9anw@mail.gmail.com> (raw) In-Reply-To: <20200930095850.GA68612@C02TD0UTHF1T.local> Hi Mark, On Wed, Sep 30, 2020 at 2:59 AM Mark Rutland <mark.rutland@arm.com> wrote: > > Hi Sami, > > On Tue, Sep 29, 2020 at 02:46:11PM -0700, Sami Tolvanen wrote: > > Select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY to disable > > recordmcount when DYNAMIC_FTRACE_WITH_REGS is selected. > > Could you please add an explanation as to /why/ this is necessary in the > commit message? I couldn't figure this out form the commit message > alone, and reading the cover letter also didn't help. Sorry about that, I'll add a better explanation in the next version. Note that without LTO, this change is not strictly necessary as there's no harm in running recordmcount even if it's not needed. It might slow down the build slightly, but I suspect a few thousand invocations of the program won't take that long. However, with LTO we need to disable recordmcount because it doesn't understand LLVM bitcode. > If the minimum required GCC version supports patchable-function-entry > I'd be happy to make that a requirement for dynamic ftrace on arm64, as > then we'd only need to support one mechanism, and can get rid of some > redundant code. We already default to it when present anyhow. That would be great, but Documentation/process/changes.rst suggests the minimum gcc version is 4.9, and according to Godbolt we would need gcc >= 8 for -fpatchable-function-entry: https://godbolt.org/z/jdzcMW > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index 6d232837cbee..ad522b021f35 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -155,6 +155,8 @@ config ARM64 > > select HAVE_DYNAMIC_FTRACE > > select HAVE_DYNAMIC_FTRACE_WITH_REGS \ > > if $(cc-option,-fpatchable-function-entry=2) > > + select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY \ > > + if DYNAMIC_FTRACE_WITH_REGS > > This doesn't look quite right to me. Presumably we shouldn't allow > DYNAMIC_FTRACE_WITH_REGS to be selected if HAVE_DYNAMIC_FTRACE_WITH_REGS > isn't. This won't allow DYNAMIC_FTRACE_WITH_REGS to be selected without HAVE_DYNAMIC_FTRACE_WITH_REGS. Testing with a compiler that does support -fpatchable-function-entry, I get the following, as expected: $ grep -E '(DYNAMIC_FTRACE|MCOUNT_USE)' .config CONFIG_HAVE_DYNAMIC_FTRACE=y CONFIG_HAVE_DYNAMIC_FTRACE_WITH_REGS=y CONFIG_DYNAMIC_FTRACE=y CONFIG_DYNAMIC_FTRACE_WITH_REGS=y CONFIG_FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY=y And if the compiler doesn't support -fpatchable-function-entry, we would end up with the following: $ grep -E '(DYNAMIC_FTRACE|MCOUNT_USE)' .config CONFIG_HAVE_DYNAMIC_FTRACE=y CONFIG_DYNAMIC_FTRACE=y CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT=y Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-30 17:11 UTC|newest] Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-29 21:46 [PATCH v4 00/29] Add support for Clang LTO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 01/29] RAS/CEC: Fix cec_init() prototype Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 02/29] x86/asm: Replace __force_order with memory clobber Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 03/29] kbuild: preprocess module linker script Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 04/29] objtool: Add a pass for generating __mcount_loc Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-10-01 13:17 ` Miroslav Benes 2020-10-01 13:17 ` Miroslav Benes 2020-10-01 13:17 ` Miroslav Benes 2020-10-01 13:36 ` Peter Zijlstra 2020-10-01 13:36 ` Peter Zijlstra 2020-10-02 14:13 ` Josh Poimboeuf 2020-10-02 14:13 ` Josh Poimboeuf 2020-10-05 7:10 ` Miroslav Benes 2020-10-05 7:10 ` Miroslav Benes 2020-10-05 7:10 ` Miroslav Benes 2020-09-29 21:46 ` [PATCH v4 05/29] objtool: Don't autodetect vmlinux.o Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 06/29] tracing: move function tracer options to Kconfig Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-30 0:12 ` Steven Rostedt 2020-09-30 0:12 ` Steven Rostedt 2020-09-30 16:05 ` Sami Tolvanen 2020-09-30 16:05 ` Sami Tolvanen 2020-09-30 16:05 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 07/29] tracing: add support for objtool mcount Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 08/29] x86, build: use " Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 09/29] arm64: disable recordmcount with DYNAMIC_FTRACE_WITH_REGS Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-30 9:58 ` Mark Rutland 2020-09-30 9:58 ` Mark Rutland 2020-09-30 17:10 ` Sami Tolvanen [this message] 2020-09-30 17:10 ` Sami Tolvanen 2020-09-30 17:10 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 10/29] treewide: remove DISABLE_LTO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-30 20:45 ` Kees Cook 2020-09-30 20:45 ` Kees Cook 2020-09-29 21:46 ` [PATCH v4 11/29] kbuild: add support for Clang LTO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 12/29] kbuild: lto: fix module versioning Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 13/29] kbuild: lto: postpone objtool Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 14/29] kbuild: lto: limit inlining Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 15/29] kbuild: lto: merge module sections Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 16/29] kbuild: lto: remove duplicate dependencies from .mod files Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 17/29] init: lto: ensure initcall ordering Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 18/29] init: lto: fix PREL32 relocations Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 19/29] PCI: Fix PREL32 relocations for LTO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 20/29] modpost: lto: strip .lto from module names Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 21/29] scripts/mod: disable LTO for empty.c Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 22/29] efi/libstub: disable LTO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 23/29] drivers/misc/lkdtm: disable LTO for rodata.o Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 24/29] arm64: vdso: disable LTO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 25/29] KVM: arm64: disable LTO for the nVHE directory Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 26/29] arm64: allow LTO_CLANG and THINLTO to be selected Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 27/29] x86, vdso: disable LTO only for vDSO Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 28/29] x86, cpu: disable LTO for cpu.c Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` [PATCH v4 29/29] x86, build: allow LTO_CLANG and THINLTO to be selected Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-29 21:46 ` Sami Tolvanen 2020-09-30 21:10 ` [PATCH v4 00/29] Add support for Clang LTO Kees Cook 2020-09-30 21:10 ` Kees Cook 2020-09-30 21:58 ` Nick Desaulniers 2020-09-30 21:58 ` Nick Desaulniers 2020-09-30 21:58 ` Nick Desaulniers 2020-09-30 22:12 ` Sami Tolvanen 2020-09-30 22:12 ` Sami Tolvanen 2020-09-30 22:12 ` Sami Tolvanen
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='CABCJKuegb4MzniWOk2+R3FngZpdWuSEAZuj=arRm0mE6HQ9anw@mail.gmail.com' \ --to=samitolvanen@google.com \ --cc=clang-built-linux@googlegroups.com \ --cc=gregkh@linuxfoundation.org \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-arch@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=masahiroy@kernel.org \ --cc=ndesaulniers@google.com \ --cc=paulmck@kernel.org \ --cc=peterz@infradead.org \ --cc=rostedt@goodmis.org \ --cc=will@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: linkBe 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.