From: Miguel Ojeda <email@example.com> To: Nick Desaulniers <firstname.lastname@example.org> Cc: Miguel Ojeda <email@example.com>, Masahiro Yamada <firstname.lastname@example.org>, Linux Kbuild mailing list <email@example.com>, Linus Torvalds <firstname.lastname@example.org>, Greg Kroah-Hartman <email@example.com>, rust-for-linux <firstname.lastname@example.org>, Linux Doc Mailing List <email@example.com>, linux-kernel <firstname.lastname@example.org>, Alex Gaynor <email@example.com>, Finn Behrens <firstname.lastname@example.org>, Adam Bratschi-Kaye <email@example.com>, Wedson Almeida Filho <firstname.lastname@example.org>, Michael Ellerman <email@example.com>, Sven Van Asbroeck <firstname.lastname@example.org>, Gary Guo <email@example.com>, Boris-Chengbiao Zhou <firstname.lastname@example.org>, Boqun Feng <email@example.com>, Douglas Su <firstname.lastname@example.org>, Dariusz Sosnowski <email@example.com>, Antonio Terceiro <firstname.lastname@example.org>, Daniel Xu <email@example.com> 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 <firstname.lastname@example.org> 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
next prev 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] ` <email@example.com> 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' \ --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 \ --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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --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).