Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false

[Sami Tolvanen]

Hi Gary,

On Fri, Oct 11, 2024 at 9:12 AM Gary Guo <gary@xxxxxxxxxxx> wrote:
>
> On Fri, 11 Oct 2024 08:23:18 -0700
> Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:
>
> > On Fri, Oct 11, 2024 at 5:13 AM Gary Guo <gary@xxxxxxxxxxx> wrote:
> > >
> > > On Fri, 11 Oct 2024 10:13:34 +0000
> > > Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> > >
> > > > +#ifndef CONFIG_JUMP_LABEL
> > > > +int rust_helper_static_key_count(struct static_key *key)
> > > > +{
> > > > +     return static_key_count(key);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(rust_helper_static_key_count);
> > >
> > > ^ Explicit export should be removed. This only works because we didn't
> > > remove export.h from all helpers.c yet, but there's a patch to do
> > > that and this will stop working.
> >
> > What's the benefit of removing explicit exports from the Rust helper C
> > code? It requires special casing things like modversions for these
> > files, so I assume there's a reason for this. I asked about it here,
> > but never got a response:
> >
> > https://lore.kernel.org/rust-for-linux/CABCJKudqAEvLcdqTqyfE2+iW+jeqBpnTGgYJvrZ0by6hGdfevQ@xxxxxxxxxxxxxx/
> >
> > Sami
>
> Ah, I didn't saw that email, probably because I archived the mails after
> the patch is applied.

Sometimes you might get pings about patches that are already applied too. :)

> We're working towards having an option that enables inlining these
> helpers into Rust; when that option is enabled, the helpers must not be
> exported. See
> https://lore.kernel.org/rust-for-linux/20240529202817.3641974-1-gary@xxxxxxxxxxx/
> and https://lwn.net/Articles/993163/.

Interesting, thanks for the links. It would have been helpful to
explain the motivation for the change also in the patch that was
applied.

Did you consider using the preprocessor to simply skip exporting the
helpers when cross-language LTO inlining is used? This would allow us
to use the existing C build rules for the code instead of adding a
separate rule to handle Rust-style exports, like I'm doing here:

https://github.com/samitolvanen/linux/commit/545277e4d0432dafc530b1618f0152aed82af2f5

> It's also quite tedious for every helper to carry this export.

It's just one line per helper, but sure, I do see your point.

Sami