From: Peter Zijlstra <email@example.com> To: Peter Oberparleiter <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Subject: Re: [PATCH] gcov,x86: Mark GCOV broken for x86 Date: Fri, 18 Jun 2021 13:13:57 +0200 [thread overview] Message-ID: <YMx/9Xv8BF7ghAO6@hirez.programming.kicks-ass.net> (raw) In-Reply-To: <email@example.com> On Mon, Jun 14, 2021 at 04:43:27PM +0200, Peter Oberparleiter wrote: > On 14.06.2021 12:17, Peter Zijlstra wrote: > > As recently discovered, there is no function attribute to disable the > > -fprofile-generate instrumentation. As such, GCOV is fundamentally > > incompatible with architectures that rely on 'noinstr' for correctness. > > Does this problem affect all code or just those pieces that use > 'noinstr'? Doing a quick grep over kernel source shows me ~40 source > files containing 'noinstr' vs. ~30000 that don't. I count 82, but yeah. > It seems to me like an extreme measure to disable gcov-based profiling > for all files on an architecture when only a small fraction of code > would actually be affected. > > I'll gladly admit that I haven't followed the full discussion that lead > to your patch, so maybe some of the following suggestions may already > have been proposed. > > What about marking source files that contain 'noinstr' using the > > GCOV_PROFILE_<filename.o> := n > > directive that gcov-kernel profiling provides to exclude those files > from being compiled with the corresponding profiling flags? If that's > too much effort there's also a directive for excluding all files in a > directory. It's just not scalable and super fragile. Forget one and you have a potentially dead kernel. At the same time, we'll end up excluding significant chunks of the core kernel that way, also limit the use of GCOV. > If there was a way to automatically identify 'noinstr'-afflicted source > files (e.g. by grepping the pre-processed source files), one could also > automate this process by adjusting the kbuild-code that adds profiling > flags to automatically exclude such files. Or we just wait for the compilers to implement the required function attribute and then make the whole thing depend on having a recent enough compiler, which is what I'm hoping for. Developers should use recent compilers anyway... > > Until such time as that compilers have added a function attribute to > > disable this instrumentation, mark GCOV as broken. > > > > Signed-off-by: Peter Zijlstra (Intel) <firstname.lastname@example.org> > > --- > > arch/x86/Kconfig | 2 +- > > kernel/gcov/Kconfig | 4 ++++ > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 86dae426798b..c0f8c9d4c31a 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -75,7 +75,7 @@ config X86 > > select ARCH_HAS_FAST_MULTIPLIER > > select ARCH_HAS_FILTER_PGPROT > > select ARCH_HAS_FORTIFY_SOURCE > > - select ARCH_HAS_GCOV_PROFILE_ALL > > + select ARCH_HAS_GCOV_BROKEN > > Assuming none of the above mentioned alternatives are viable, removing > ARCH_HAS_GCOV_PROFILE_ALL should be enough for your purpose. This way > you are already excluding all source files from automatic profiling on x86. But you can still select one manually, which is not safe. > Users that are absolutely sure that their code can work with > gcov-profiling can manually edit their sub-Makefiles to list those files > that should be instrumented. In my opinion your introduction of > ARCH_HAS_GCOV_BROKEN unnecessarily takes away this capability. Are there any users? Who uses this GCOV stuff, and should we migrate them to KCOV? The thing is, I got dead kernel reports from KCOV users really quickly after all this landed, I've never even heard of a GCOV user, let alone had a problem report from one. Given all this seems mostly unused, I suppose we can wait for the compilers to implement the attribute and simply ignore any and all problems stemming from the use of GCOV -- telling them to go use KCOV instead. At the same time; since there are no users (that I know of), I don't see the problem with killing the entire thing for x86 either.
next prev parent reply other threads:[~2021-06-18 11:14 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-14 10:17 Peter Zijlstra 2021-06-14 10:31 ` Marco Elver 2021-06-14 14:43 ` Peter Oberparleiter 2021-06-18 11:13 ` Peter Zijlstra [this message] 2021-06-21 13:53 ` Peter Oberparleiter 2021-06-14 16:05 ` Nick Desaulniers 2021-06-14 16:20 ` Peter Zijlstra 2021-06-14 18:05 ` Nick Desaulniers 2021-06-14 18:20 ` Borislav Petkov 2021-06-14 19:03 ` Nick Desaulniers 2021-06-14 19:28 ` Borislav Petkov 2021-06-14 18:31 ` Fangrui Song 2021-06-14 19:07 ` Peter Zijlstra
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=YMx/9Xv8BF7ghAO6@hirez.programming.kicks-ass.net \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] gcov,x86: Mark GCOV broken for x86' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).