rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Miguel Ojeda <ojeda@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	rust-for-linux <rust-for-linux@vger.kernel.org>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Alex Gaynor <alex.gaynor@gmail.com>, Finn Behrens <me@kloenk.de>,
	Adam Bratschi-Kaye <ark.email@gmail.com>,
	Wedson Almeida Filho <wedsonaf@google.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Sven Van Asbroeck <thesven73@gmail.com>,
	Gary Guo <gary@garyguo.net>,
	Boris-Chengbiao Zhou <bobo1239@web.de>,
	Boqun Feng <boqun.feng@gmail.com>,
	Douglas Su <d0u9.su@outlook.com>,
	Dariusz Sosnowski <dsosnowski@dsosnowski.pl>,
	Antonio Terceiro <antonio.terceiro@linaro.org>,
	Daniel Xu <dxu@dxuuu.xyz>
Subject: Re: [PATCH 15/19] Kbuild: add Rust support
Date: Wed, 8 Dec 2021 23:52:13 +0100	[thread overview]
Message-ID: <CANiq72mYmzwADwJFbOpAPh5YF59RxP-ZoHm-DD=uVdj+yKB_Kw@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOd=7QTUH69+ZbT7e8einvgcosTbDkyohmPaUBv6_y8RfrQ@mail.gmail.com>

Hi Nick,

On Tue, Dec 7, 2021 at 11:45 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Can we update some Kconfig checks to fix that?

Yes, I can definitely add that.

> Consider putting clippy specific flags in their own variable, and/or
> just adding them to KBUILD_RUSTFLAGS only when clippy is actually
> being used.

This was actually the original approach, but I simplified it -- there
is no need to put them on their own.

Unless, of course, what you want is shorter verbose output -- is this
what you are referring to?

> ... here. Should we sink the check for origin CLIPPY? Can we also
> match how we check for LLVM=1 rather than checking the variables
> origin?  Especially since KBUILD_CLIPPY isn't exported, it seems it's
> not used outside of this file, IIUC?

I am mimicking what we do for the checkers etc., since that is the
closest to Clippy.

> Is this used in a different patch in the series? ^

`RUSTC_BOOTSTRAP` is used to be able to use stable releases of the
compiler with unstable features (it should go away, eventually).

> If z and s are distinct opt levels, then rustc should really be using
> s rather than z.  IME, z has created larger images than s due to LLVM
> aggressively emitting libcalls, regardless of the cost of call setup.
> See also commit a75bb4eb9e56 ("Revert "kbuild: use -Oz instead of -Os
> when using clang")

Sounds good. In any case, I will check with upstream Rust if they are
used (or planned to be used) for any optimization purposes even before
LLVM is reached.

> oh boy. I wonder which configs change the above values? ;) Can rustc
> still override these via command line flags?

No, it cannot. This is actually a topic I brought up at LPC. Ideally,
we would get `rustc` to support similar command line flags as GCC and
Clang do to customize the targets.

The good news are that it seems upstream may be open to accept flags
to customize targets. They may use it as a way to stabilize parts of
the custom target spec piece by piece, instead of stabilizing the
custom target spec files.

We also raised the issue at the Rust CTCFT meeting a couple of weeks ago.

> So this is like turning on -fsanitize=signed-integer-overflow and
> -fsanitize=unsigned-integer-overflow by default?

Not exactly -- in the Rust case, it is a (Rust) panic. So it would be
closer to `-fno-sanitize-recover`.

> A rust panic or a kernel panic?

The former -- which currently means `BUG()`. However, there could
perhaps be other alternatives. There was a bit of discussion about
this around Kangrejos time but nothing conclusive.

> This should really be the only option here. I still don't see the
> point of allowing wacky combinations of configs where we differ from C
> code.  The more combinations will just frustrate randconfig builds.

I don't have a strong opinion -- but I have not heard from others either...

Regarding `randconfig`, we could restrict it, no?

> Seeing repeated filter-out rules makes me wonder if we could DRY up
> some of these rules with function definitions?

Yeah, there are also some factoring out that I could do. I grew the
file over time and while I am using target-specific variables to
generalize a bit too, it can still be taken further.

I have some other changes planned to the build process, so I will
probably take the chance to clean up a bit as well.

> Are there links to corresponding feature requests upstream so that one
> day we can drop technical debt here?  In particular, having to create
> a custom sysroot as is done here isn't particularly pretty.
>
> I'd be curious if we could remove the dependency on cargo for rust tests.

Yes, we have brought this up to upstream, for instance in the Rust
CTCFT meeting: https://rust-lang.github.io/ctcft/meetings/2021-11-22.html

In general, I track all feature requests etc. in this "live" issue
(see also the linked ones from there):
https://github.com/Rust-for-Linux/linux/issues/2

> Do we denote the conflict in KCONFIG?

No, but yeah, we could. However, I am not sure if we want to denote
them for something so experimental, though (another question is
whether we warn the user about GCC+Rust builds in general).

Or, if we are going to be serious about GCC already (i.e. before we
have actual codegen through GCC for Rust), then I would say we need a
better solution to the problem (like the "check-if-correct" approach
you suggested a while ago, or a proper GCC backend for `bindgen`).

> Why do we need to strip out all these warning flags when running
> bindgen?  Should be just be using -w to silence all warnings from
> clang when invoking bindgen?

Hmm... is there any possibility that we silence something that we care
actually about? I guess we would get the same diagnostics in normal
Clang/LLVM builds, so maybe it is fine. And even if not, perhaps we
should just simplify anyway, given it is GCC+Rust builds we are
talking about.

> I don't understand why the PowerPC specific flags are pulled out, but
> the x86 specific ones (like most of the -m flags in the initial
> defiition of bindgen_skip_c_flags) are not.

No particular reason. When the list of archs started to grow, I
decided it would be better to start splitting them out. I will clean
that out.

> This doesn't accidentally export non-GPL symbols as GPL does it?

It does, but that is fine -- it is the other way around the one that
would "open" the kernel unexpectedly.

Thanks a lot for the throughout review!

Cheers,
Miguel

  parent reply	other threads:[~2021-12-08 22:52 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-06 14:02 [PATCH 00/19] " Miguel Ojeda
2021-12-06 14:02 ` [PATCH 01/19] kallsyms: support "big" kernel symbols Miguel Ojeda
2021-12-06 14:18   ` Matthew Wilcox
2021-12-06 17:00     ` Miguel Ojeda
2021-12-06 14:02 ` [PATCH 02/19] kallsyms: increase maximum kernel symbol length to 512 Miguel Ojeda
2021-12-06 14:02 ` [PATCH 03/19] kallsyms: use the correct buffer size for symbols Miguel Ojeda
2021-12-06 14:02 ` [PATCH 04/19] rust: add C helpers Miguel Ojeda
2021-12-06 14:02 ` [PATCH 05/19] rust: add `compiler_builtins` crate Miguel Ojeda
2021-12-07 23:01   ` Nick Desaulniers
2021-12-09 19:34     ` Miguel Ojeda
2021-12-06 14:03 ` [PATCH 07/19] rust: add `build_error` crate Miguel Ojeda
2021-12-06 14:03 ` [PATCH 08/19] rust: add `macros` crate Miguel Ojeda
2021-12-06 14:03 ` [PATCH 10/19] rust: export generated symbols Miguel Ojeda
2021-12-06 14:03 ` [PATCH 11/19] vsprintf: add new `%pA` format specifier Miguel Ojeda
2021-12-06 15:02   ` Greg Kroah-Hartman
2021-12-06 15:56     ` Miguel Ojeda
2021-12-06 16:02       ` Greg Kroah-Hartman
2021-12-06 19:52         ` Nick Desaulniers
2021-12-06 19:55           ` Matthew Wilcox
2021-12-06 20:02             ` Nick Desaulniers
2021-12-06 14:03 ` [PATCH 12/19] scripts: add `generate_rust_analyzer.py` Miguel Ojeda
2021-12-06 14:03 ` [PATCH 13/19] scripts: decode_stacktrace: demangle Rust symbols Miguel Ojeda
2021-12-06 14:03 ` [PATCH 14/19] docs: add Rust documentation Miguel Ojeda
2021-12-08  1:29   ` Nick Desaulniers
2021-12-08 23:07     ` Miguel Ojeda
2021-12-06 14:03 ` [PATCH 15/19] Kbuild: add Rust support Miguel Ojeda
2021-12-07 22:45   ` Nick Desaulniers
2021-12-07 23:21     ` Nick Desaulniers
2021-12-07 23:25       ` Wedson Almeida Filho
2021-12-08  0:05         ` Nick Desaulniers
2021-12-08 22:52     ` Miguel Ojeda [this message]
2021-12-11 15:53   ` Masahiro Yamada
2022-01-17  4:45     ` Miguel Ojeda
2021-12-06 14:03 ` [PATCH 16/19] samples: add Rust examples Miguel Ojeda
2021-12-06 14:03 ` [PATCH 17/19] MAINTAINERS: Rust Miguel Ojeda
2021-12-06 14:03 ` [RFC PATCH 18/19] drivers: gpio: PrimeCell PL061 in Rust Miguel Ojeda
     [not found] ` <20211206140313.5653-20-ojeda@kernel.org>
2021-12-06 15:01   ` [RFC PATCH 19/19] drivers: android: Binder IPC " Greg Kroah-Hartman
2021-12-06 15:59     ` Wedson Almeida Filho
2021-12-06 16:02       ` Greg Kroah-Hartman

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='CANiq72mYmzwADwJFbOpAPh5YF59RxP-ZoHm-DD=uVdj+yKB_Kw@mail.gmail.com' \
    --to=miguel.ojeda.sandonis@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=antonio.terceiro@linaro.org \
    --cc=ark.email@gmail.com \
    --cc=bobo1239@web.de \
    --cc=boqun.feng@gmail.com \
    --cc=d0u9.su@outlook.com \
    --cc=dsosnowski@dsosnowski.pl \
    --cc=dxu@dxuuu.xyz \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=me@kloenk.de \
    --cc=mpe@ellerman.id.au \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=thesven73@gmail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=wedsonaf@google.com \
    --subject='Re: [PATCH 15/19] Kbuild: add Rust support' \
    /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).