linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Mark Rutland <mark.rutland@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Bill Wendling <morbo@google.com>,
	Bill Wendling <wcw@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	clang-built-linux@googlegroups.com,
	Fangrui Song <maskray@google.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Jarmo Tiitto <jarmo.tiitto@gmail.com>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Miguel Ojeda <ojeda@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Peter Oberparleiter <oberpar@linux.ibm.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [GIT PULL] Clang feature updates for v5.14-rc1
Date: Tue, 29 Jun 2021 14:05:40 -0700	[thread overview]
Message-ID: <CAKwvOd=BRD8Yrq6QvLiZq-_GL-JdDPvx6ghO4ROCo+AtisTJvw@mail.gmail.com> (raw)
In-Reply-To: <20210629131400.GA24514@C02TD0UTHF1T.local>

On Tue, Jun 29, 2021 at 6:14 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi Kees,
>
> I thought the PGO stuff was on hold given Peter had open concerns, e.g.
>
> https://lore.kernel.org/r/20210614154639.GB68749@worktop.programming.kicks-ass.net
>
> ... and there didn't seem to be a strong conclusion to the contrary.

Hi Mark,
If I could rephrase Peter's concerns in my own words to see if I
understood the intent correctly, I'd summarize the concerns as:
1. How does instrumentation act in regards to noinstr?

https://lore.kernel.org/linux-doc/20210614153545.GA68749@worktop.programming.kicks-ass.net/
https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/

2. How much of this code can be reused with GCC?

https://lore.kernel.org/linux-doc/20210614154639.GB68749@worktop.programming.kicks-ass.net/

3. Can we avoid proliferation of compiler specific code in the kernel?

https://lore.kernel.org/linux-doc/YMca2aa+t+3VrpN9@hirez.programming.kicks-ass.net/

---

Regarding point 1, I believe that was addressed by this series, which
Peter Ack'ed, and is based on work I did in LLVM based on Peter's
feedback, while collaborating with GCC developers on the semantics in
regards to inlining.  I notice you weren't explicitly cc'ed on that
thread, that's my fault and I apologize.  It wasn't intentional; once
a cc list as recommended by get_maintainer.pl gets too long, I start
to forget who was on previous threads and might be interested in
updates.

https://lore.kernel.org/lkml/YNGQV09E9xAvvppO@hirez.programming.kicks-ass.net/
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

---

Regarding point 2, I believe I addressed that in my response.  Similar
to GCOV, we need the runtime hooks which are compiler specific in
order to capture the profiling data. Exporting such data to userspace
via sysfs can be easily shared though, as is done currently for GCOV.

https://lore.kernel.org/linux-doc/CAKwvOd=aAo72j-iE2PNE5Os8BPc0y-Zs7ZoMzd21ck+QNeboBA@mail.gmail.com/

---

Regarding point 3, I agree. There's currently 2 big places in the
kernel where we have very compiler specific code, IMO:
1. frame pointer based unwinding on 32b ARM (especially but not
limited to THUMB).
2. GCOV
This series does ask to add a third.

At the same time, there are differences between compilers that are
unlikely to converge without great need.  Compiler IR is generally not
interchangeable between compilers; the compiler runtimes (ie. symbols
typically provided by libgcc_s or compiler-rt) are (generally) tightly
coupled to their respective compilers.  Since PGO relies on the
respective compiler runtimes, we wind up with compiler specific
runtime support for this feature.  For a semi-freestanding environment
like the Linux kernel, that means duplicating the ABI for these
compiler runtime libraries, with additional code for kernel specific
synchronization, memory management, and data retrieval (sysfs).

Further, asking compiler vendors to break their existing ABIs with
their compiler runtimes to support a shared interface for profiling
data is also a hard sell. That's a major issue regarding frame pointer
based unwinding on 32b ARM as well; existing unwinders must change to
support the latest spec, yet not all code will be recompiled to match
it as the same time the unwinder support is added or updated.  Unless
the compiler runtime was statically linked, then upgrading that shared
object might break binaries when they are run next.  I'm not saying
it's impossible, but is it worth it? Do the compiler vendors agree?
-- 
Thanks,
~Nick Desaulniers

  parent reply	other threads:[~2021-06-29 21:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-28 19:32 [GIT PULL] Clang feature updates for v5.14-rc1 Kees Cook
2021-06-29  2:49 ` Linus Torvalds
2021-06-29 20:44   ` Kees Cook
2021-06-29 21:03     ` Linus Torvalds
2021-06-29 21:27       ` Nick Desaulniers
2021-06-29 21:57         ` Linus Torvalds
2021-07-07  8:10         ` Ingo Molnar
2021-07-02 12:46       ` Bill Wendling
2021-07-02 12:56         ` Peter Zijlstra
2021-07-02 17:26           ` Nick Desaulniers
2021-07-02 18:57             ` Peter Zijlstra
2021-07-02 19:53               ` Nick Desaulniers
2022-10-31 23:55           ` Bill Wendling
2021-06-29 13:14 ` Mark Rutland
2021-06-29 20:11   ` Kees Cook
2021-06-29 21:05   ` Nick Desaulniers [this message]
2021-10-05 13:10 ` ARCH_WANTS_NO_INSTR (Re: [GIT PULL] Clang feature updates for v5.14-rc1) Daniel Axtens
2021-10-05 13:45   ` Daniel Axtens
2021-10-05 14:30   ` Mark Rutland
2021-10-07  6:19     ` Jarmo Tiitto

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='CAKwvOd=BRD8Yrq6QvLiZq-_GL-JdDPvx6ghO4ROCo+AtisTJvw@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=hca@linux.ibm.com \
    --cc=jarmo.tiitto@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=maskray@google.com \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=oberpar@linux.ibm.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=wcw@google.com \
    --cc=will@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;
as well as URLs for NNTP newsgroup(s).