2024-05-29 20:31:54

by Gary Guo

[permalink] [raw]
Subject: [RFC PATCH 2/2] kbuild: rust: provide an option to inline C helpers into Rust

A new Kconfig option, `RUST_LTO_HELPERS` is added to allow C helpers
(which was created to allow Rust to call into inline/macro C functions
without having to re-implement the logic in Rust) to be inlined into
Rust crates without performing a global LTO.

If the option is enabled, the following is performed:
* For helpers, instead of compiling them to object file to be linked
into vmlinux, we compile them to LLVM IR.
* The LLVM IR is patched to add `linkonce_odr` linkage. This linkage
means that the function is inlineable (effect of `_odr`), and the
symbols generated will have weak linkage if emitted into object file
(important since as later described, we might have multiple copies of
the same symbol) and it will may be discarded if it is not invoked or
all invocations are inlined.
* The LLVM IR is compiled to bitcode (This is step is not necessary, but
is a performance optimisation to prevent LLVM from always have to
reparse the same IR).
* When a Rust crate is compiled, instead of generating object file, we
ask LLVM bitcode to be generated.
* llvm-link is invoked to combine the helper bitcode with the crate
bitcode. This step is similar to what's done in a full LTO, hence the
option name, but this is much faster since it only needs to inline the
helpers.
* clang is invoked to turn the combined bitcode into object file.

Some caveats with the option:
* clang and Rust doesn't have the exact target string. Manual inspection
shows that they should be compatible, but since they are not exactly
the same LLVM seems to prefer not inlining them. This is bypassed with
`--ignore-tti-inline-compatible`.
* LLVM doesn't want to inline functions combined with
`-fno-delete-null-pointer-checks` with code compiled without. So we
remove this command when compiling helpers. I think this should be
okay since this is one of the hardening features and we shouldn't have
null pointer dereferences in these helpers.

The checks can also be bypassed with force inlining (`__always_inline`)
but the behaviour is the same with extra options.

This option requires clang to be used for CC and the LLVM version of
clang and rustc to be matching (at least to the same major version), and
therefore is gated as EXPERT option.

Co-developed-by: Boqun Feng <[email protected]>
Signed-off-by: Boqun Feng <[email protected]>
Signed-off-by: Gary Guo <[email protected]>
---
Makefile | 4 +++-
lib/Kconfig.debug | 10 ++++++++++
rust/Makefile | 34 ++++++++++++++++++++++++++++++----
rust/exports.c | 3 +++
rust/helpers.c | 41 +++++++++++++++++++++--------------------
scripts/Makefile.build | 5 ++++-
6 files changed, 71 insertions(+), 26 deletions(-)

diff --git a/Makefile b/Makefile
index f975b6396328..c6f12093fb1c 100644
--- a/Makefile
+++ b/Makefile
@@ -492,6 +492,8 @@ OBJCOPY = $(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX)
OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
+LLVM_LINK = $(LLVM_PREFIX)llvm-link$(LLVM_SUFFIX)
+LLVM_AS = $(LLVM_PREFIX)llvm-as$(LLVM_SUFFIX)
else
CC = $(CROSS_COMPILE)gcc
LD = $(CROSS_COMPILE)ld
@@ -601,7 +603,7 @@ endif
export RUSTC_BOOTSTRAP := 1

export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG
-export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN CARGO
+export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN CARGO LLVM_LINK LLVM_AS
export HOSTRUSTC KBUILD_HOSTRUSTFLAGS
export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 59b6765d86b8..8b9a1bd8ce71 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3049,6 +3049,16 @@ config RUST_KERNEL_DOCTESTS

If unsure, say N.

+config RUST_LTO_HELPERS
+ bool "LTO C helpers with Rust crates"
+ depends on RUST && CC_IS_CLANG && EXPERT
+ help
+ Links C helpers together with Rust crates using LTO.
+
+ This is achieved by compiling both helpers and Rust crates into
+ LLVM bitcode and use llvm-link to combine them.
+
+ This requires a matching LLVM version for Clang and rustc.
+
+ If unsure, say N.
+
endmenu # "Rust"

endmenu # Kernel hacking
diff --git a/rust/Makefile b/rust/Makefile
index b4d63ea9209f..ad0797467102 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -6,9 +6,14 @@ rustdoc_output := $(objtree)/Documentation/output/rust/rustdoc
obj-$(CONFIG_RUST) += core.o compiler_builtins.o
always-$(CONFIG_RUST) += exports_core_generated.h

+ifdef CONFIG_RUST_LTO_HELPERS
+always-$(CONFIG_RUST) += helpers.bc
+else
+obj-$(CONFIG_RUST) += helpers.o
+always-$(CONFIG_RUST) += exports_helpers_generated.h
+endif
# Missing prototypes are expected in the helpers since these are exported
# for Rust only, thus there is no header nor prototypes.
-obj-$(CONFIG_RUST) += helpers.o
CFLAGS_REMOVE_helpers.o = -Wmissing-prototypes -Wmissing-declarations

always-$(CONFIG_RUST) += libmacros.so
@@ -17,7 +22,7 @@ no-clean-files += libmacros.so
always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \
- exports_kernel_generated.h exports_helpers_generated.h
+ exports_kernel_generated.h

always-$(CONFIG_RUST) += uapi/uapi_generated.rs
obj-$(CONFIG_RUST) += uapi.o
@@ -350,12 +355,24 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_flags = \
--blocklist-type '.*' --allowlist-var '' \
--allowlist-function 'rust_helper_.*'
$(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_cflags = \
- -I$(objtree)/$(obj) -Wno-missing-prototypes -Wno-missing-declarations
+ -I$(objtree)/$(obj) $(CFLAGS_helpers.o) -Wno-missing-prototypes -Wno-missing-declarations
$(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ; \
sed -Ei 's/pub fn rust_helper_([a-zA-Z0-9_]*)/#[link_name="rust_helper_\1"]\n pub fn \1/g' $@
$(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers.c FORCE
$(call if_changed_dep,bindgen)

+# When compiling helpers, filter `-fno-delete-null-pointer-checks` since LLVM
+# prevents inlining such functions to be inlined into functions compiled
+# without the option (e.g. Rust functions).
+quiet_cmd_rust_helper = HELPER $@
+ cmd_rust_helper = \
+ $(CC) $(filter-out -fno-delete-null-pointer-checks $(CFLAGS_REMOVE_helpers.o), $(c_flags) $(CFLAGS_helpers.o)) -S $< -emit-llvm -o $(patsubst %.bc,%.ll,$@); \
+ sed -i 's/^define dso_local/define linkonce_odr dso_local/g' $(patsubst %.bc,%.ll,$@); \
+ $(LLVM_AS) $(patsubst %.bc,%.ll,$@) -o $@
+
+$(obj)/helpers.bc: $(obj)/helpers.c FORCE
+ +$(call if_changed_dep,rust_helper)
+
quiet_cmd_exports = EXPORTS $@
cmd_exports = \
$(NM) -p --defined-only $< \
@@ -396,11 +413,13 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
OBJTREE=$(abspath $(objtree)) \
$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
- --emit=dep-info=$(depfile) --emit=obj=$@ \
+ --emit=dep-info=$(depfile) --emit=$(if $(link_helper),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) \
--emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
--crate-type rlib -L$(objtree)/$(obj) \
--crate-name $(patsubst %.o,%,$(notdir $@)) $< \
--sysroot=/dev/null \
+ $(if $(link_helper),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(obj)/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
+ $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)

rust-analyzer:
@@ -463,4 +482,11 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
+$(call if_changed_dep,rustc_library)

+ifdef CONFIG_RUST_LTO_HELPERS
+
+$(obj)/kernel.o: private link_helper = 1
+$(obj)/kernel.o: $(obj)/helpers.bc
+
+endif
+
endif # CONFIG_RUST
diff --git a/rust/exports.c b/rust/exports.c
index aa1218b325e5..212c1ec1d38b 100644
--- a/rust/exports.c
+++ b/rust/exports.c
@@ -19,7 +19,10 @@
#include "exports_alloc_generated.h"
#include "exports_bindings_generated.h"
#include "exports_kernel_generated.h"
+
+#ifndef CONFIG_RUST_LTO_HELPERS
#include "exports_helpers_generated.h"
+#endif

// For modules using `rust/build_error.rs`.
#ifdef CONFIG_RUST_BUILD_ASSERT_ALLOW
diff --git a/rust/helpers.c b/rust/helpers.c
index 895f4b696962..3abf96f14148 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -32,18 +32,19 @@
#include <linux/spinlock.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include "helpers.h"

-__noreturn void rust_helper_BUG(void)
+__rust_helper __noreturn void rust_helper_BUG(void)
{
BUG();
}

-void rust_helper_mutex_lock(struct mutex *lock)
+__rust_helper void rust_helper_mutex_lock(struct mutex *lock)
{
mutex_lock(lock);
}

-void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
+__rust_helper void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
struct lock_class_key *key)
{
#ifdef CONFIG_DEBUG_SPINLOCK
@@ -53,82 +54,82 @@ void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
#endif
}

-void rust_helper_spin_lock(spinlock_t *lock)
+__rust_helper void rust_helper_spin_lock(spinlock_t *lock)
{
spin_lock(lock);
}

-void rust_helper_spin_unlock(spinlock_t *lock)
+__rust_helper void rust_helper_spin_unlock(spinlock_t *lock)
{
spin_unlock(lock);
}

-void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
+__rust_helper void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
{
init_wait(wq_entry);
}

-int rust_helper_signal_pending(struct task_struct *t)
+__rust_helper int rust_helper_signal_pending(struct task_struct *t)
{
return signal_pending(t);
}

-refcount_t rust_helper_REFCOUNT_INIT(int n)
+__rust_helper refcount_t rust_helper_REFCOUNT_INIT(int n)
{
return (refcount_t)REFCOUNT_INIT(n);
}

-void rust_helper_refcount_inc(refcount_t *r)
+__rust_helper void rust_helper_refcount_inc(refcount_t *r)
{
refcount_inc(r);
}

-bool rust_helper_refcount_dec_and_test(refcount_t *r)
+__rust_helper bool rust_helper_refcount_dec_and_test(refcount_t *r)
{
return refcount_dec_and_test(r);
}

-__force void *rust_helper_ERR_PTR(long err)
+__rust_helper __force void *rust_helper_ERR_PTR(long err)
{
return ERR_PTR(err);
}

-bool rust_helper_IS_ERR(__force const void *ptr)
+__rust_helper bool rust_helper_IS_ERR(__force const void *ptr)
{
return IS_ERR(ptr);
}

-long rust_helper_PTR_ERR(__force const void *ptr)
+__rust_helper long rust_helper_PTR_ERR(__force const void *ptr)
{
return PTR_ERR(ptr);
}

-const char *rust_helper_errname(int err)
+__rust_helper const char *rust_helper_errname(int err)
{
return errname(err);
}

-struct task_struct *rust_helper_get_current(void)
+__rust_helper struct task_struct *rust_helper_get_current(void)
{
return current;
}

-void rust_helper_get_task_struct(struct task_struct *t)
+__rust_helper void rust_helper_get_task_struct(struct task_struct *t)
{
get_task_struct(t);
}

-void rust_helper_put_task_struct(struct task_struct *t)
+__rust_helper void rust_helper_put_task_struct(struct task_struct *t)
{
put_task_struct(t);
}

-struct kunit *rust_helper_kunit_get_current_test(void)
+__rust_helper struct kunit *rust_helper_kunit_get_current_test(void)
{
return kunit_get_current_test();
}

-void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
+__rust_helper void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
bool onstack, const char *name,
struct lock_class_key *key)
{
@@ -139,7 +140,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
work->func = func;
}

-void * __must_check __realloc_size(2)
+__rust_helper void * __must_check __realloc_size(2)
rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
{
return krealloc(objp, new_size, flags);
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index efacca63c897..201e7dc5ae5d 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -288,7 +288,10 @@ rust_common_cmd = \
# would not match each other.

quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
- cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<
+ cmd_rustc_o_rs = $(rust_common_cmd) --emit=$(if $(CONFIG_RUST_LTO_HELPERS),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) $< \
+ $(if $(CONFIG_RUST_LTO_HELPERS),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(objtree)/rust/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
+ $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@)
+

$(obj)/%.o: $(obj)/%.rs FORCE
+$(call if_changed_dep,rustc_o_rs)
--
2.42.0



2024-05-29 20:46:11

by Gary Guo

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] kbuild: rust: provide an option to inline C helpers into Rust

On Wed, 29 May 2024 21:28:15 +0100
Gary Guo <[email protected]> wrote:

> Makefile | 4 +++-
> lib/Kconfig.debug | 10 ++++++++++
> rust/Makefile | 34 ++++++++++++++++++++++++++++++----
> rust/exports.c | 3 +++
> rust/helpers.c | 41 +++++++++++++++++++++--------------------
> scripts/Makefile.build | 5 ++++-
> 6 files changed, 71 insertions(+), 26 deletions(-)
>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 895f4b696962..3abf96f14148 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -32,18 +32,19 @@
> #include <linux/spinlock.h>
> #include <linux/wait.h>
> #include <linux/workqueue.h>
> +#include "helpers.h"

I made a mistake during rebase and forgot to include rust/helpers.h in
this patch. The content is

```
#ifndef RUST_HELPERS_H
#define RUST_HELPERS_H

#include <linux/compiler_types.h>

#ifdef __BINDGEN__
#define __rust_helper
#else
#define __rust_helper inline
#endif

#endif
```

I'll include this in patch v2 but would like to get a general feedback
before sending another version.





2024-06-07 14:48:08

by Matthew Maurer

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] kbuild: rust: provide an option to inline C helpers into Rust

The LLVM `.ll` textual bitcode representation may change from version
to version, which makes using `sed` to edit it potentially fragile.
Would it be possible to attach similar attributes without this? (Or
other attributes that can accomplish something similar, see the second
part.)

Would it be possible to use a Rust-only-LTO (actual LTO) flag rather
than a `llvm-link`? It seems likely that someone that wants to use a
synced `clang` and `rustc` to allow helpers to be inlined would be
unlikely to object to thin LTO of Rust. This would avoid introducing a
new tool.

As one possible route around editing the `.ll` file, we could consider
*not* hiding these symbols, just marking them as "good idea to inline
if possible", and then masking any symbol that ends up in the final
`.o` via `objcopy --localize-symbol=blah`. If we're doing full-kernel
LTO rather than rust-only LTO, we'd need to use `llvm-objcopy`
instead, but the same process would follow - a postprocessing step on
the final crate object that localizes the symbol, with LTO giving the
compiler the option (but not requirement) to inline these functions if
it seems like a good idea.


On Wed, May 29, 2024 at 1:30 PM Gary Guo <[email protected]> wrote:
>
> A new Kconfig option, `RUST_LTO_HELPERS` is added to allow C helpers
> (which was created to allow Rust to call into inline/macro C functions
> without having to re-implement the logic in Rust) to be inlined into
> Rust crates without performing a global LTO.
>
> If the option is enabled, the following is performed:
> * For helpers, instead of compiling them to object file to be linked
> into vmlinux, we compile them to LLVM IR.
> * The LLVM IR is patched to add `linkonce_odr` linkage. This linkage
> means that the function is inlineable (effect of `_odr`), and the
> symbols generated will have weak linkage if emitted into object file
> (important since as later described, we might have multiple copies of
> the same symbol) and it will may be discarded if it is not invoked or
> all invocations are inlined.
> * The LLVM IR is compiled to bitcode (This is step is not necessary, but
> is a performance optimisation to prevent LLVM from always have to
> reparse the same IR).
> * When a Rust crate is compiled, instead of generating object file, we
> ask LLVM bitcode to be generated.
> * llvm-link is invoked to combine the helper bitcode with the crate
> bitcode. This step is similar to what's done in a full LTO, hence the
> option name, but this is much faster since it only needs to inline the
> helpers.
> * clang is invoked to turn the combined bitcode into object file.
>
> Some caveats with the option:
> * clang and Rust doesn't have the exact target string. Manual inspection
> shows that they should be compatible, but since they are not exactly
> the same LLVM seems to prefer not inlining them. This is bypassed with
> `--ignore-tti-inline-compatible`.
> * LLVM doesn't want to inline functions combined with
> `-fno-delete-null-pointer-checks` with code compiled without. So we
> remove this command when compiling helpers. I think this should be
> okay since this is one of the hardening features and we shouldn't have
> null pointer dereferences in these helpers.
>
> The checks can also be bypassed with force inlining (`__always_inline`)
> but the behaviour is the same with extra options.
>
> This option requires clang to be used for CC and the LLVM version of
> clang and rustc to be matching (at least to the same major version), and
> therefore is gated as EXPERT option.
>
> Co-developed-by: Boqun Feng <[email protected]>
> Signed-off-by: Boqun Feng <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
> ---
> Makefile | 4 +++-
> lib/Kconfig.debug | 10 ++++++++++
> rust/Makefile | 34 ++++++++++++++++++++++++++++++----
> rust/exports.c | 3 +++
> rust/helpers.c | 41 +++++++++++++++++++++--------------------
> scripts/Makefile.build | 5 ++++-
> 6 files changed, 71 insertions(+), 26 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f975b6396328..c6f12093fb1c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -492,6 +492,8 @@ OBJCOPY = $(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX)
> OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
> READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
> STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
> +LLVM_LINK = $(LLVM_PREFIX)llvm-link$(LLVM_SUFFIX)
> +LLVM_AS = $(LLVM_PREFIX)llvm-as$(LLVM_SUFFIX)
> else
> CC = $(CROSS_COMPILE)gcc
> LD = $(CROSS_COMPILE)ld
> @@ -601,7 +603,7 @@ endif
> export RUSTC_BOOTSTRAP := 1
>
> export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG
> -export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN CARGO
> +export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN CARGO LLVM_LINK LLVM_AS
> export HOSTRUSTC KBUILD_HOSTRUSTFLAGS
> export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
> export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 59b6765d86b8..8b9a1bd8ce71 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -3049,6 +3049,16 @@ config RUST_KERNEL_DOCTESTS
>
> If unsure, say N.
>
> +config RUST_LTO_HELPERS
> + bool "LTO C helpers with Rust crates"
> + depends on RUST && CC_IS_CLANG && EXPERT
> + help
> + Links C helpers together with Rust crates using LTO.
> +
> + This is achieved by compiling both helpers and Rust crates into
> + LLVM bitcode and use llvm-link to combine them.
> +
> + This requires a matching LLVM version for Clang and rustc.
> +
> + If unsure, say N.
> +
> endmenu # "Rust"
>
> endmenu # Kernel hacking
> diff --git a/rust/Makefile b/rust/Makefile
> index b4d63ea9209f..ad0797467102 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -6,9 +6,14 @@ rustdoc_output := $(objtree)/Documentation/output/rust/rustdoc
> obj-$(CONFIG_RUST) += core.o compiler_builtins.o
> always-$(CONFIG_RUST) += exports_core_generated.h
>
> +ifdef CONFIG_RUST_LTO_HELPERS
> +always-$(CONFIG_RUST) += helpers.bc
> +else
> +obj-$(CONFIG_RUST) += helpers.o
> +always-$(CONFIG_RUST) += exports_helpers_generated.h
> +endif
> # Missing prototypes are expected in the helpers since these are exported
> # for Rust only, thus there is no header nor prototypes.
> -obj-$(CONFIG_RUST) += helpers.o
> CFLAGS_REMOVE_helpers.o = -Wmissing-prototypes -Wmissing-declarations
>
> always-$(CONFIG_RUST) += libmacros.so
> @@ -17,7 +22,7 @@ no-clean-files += libmacros.so
> always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
> obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
> always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \
> - exports_kernel_generated.h exports_helpers_generated.h
> + exports_kernel_generated.h
>
> always-$(CONFIG_RUST) += uapi/uapi_generated.rs
> obj-$(CONFIG_RUST) += uapi.o
> @@ -350,12 +355,24 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_flags = \
> --blocklist-type '.*' --allowlist-var '' \
> --allowlist-function 'rust_helper_.*'
> $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_cflags = \
> - -I$(objtree)/$(obj) -Wno-missing-prototypes -Wno-missing-declarations
> + -I$(objtree)/$(obj) $(CFLAGS_helpers.o) -Wno-missing-prototypes -Wno-missing-declarations
> $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ; \
> sed -Ei 's/pub fn rust_helper_([a-zA-Z0-9_]*)/#[link_name="rust_helper_\1"]\n pub fn \1/g' $@
> $(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers.c FORCE
> $(call if_changed_dep,bindgen)
>
> +# When compiling helpers, filter `-fno-delete-null-pointer-checks` since LLVM
> +# prevents inlining such functions to be inlined into functions compiled
> +# without the option (e.g. Rust functions).
> +quiet_cmd_rust_helper = HELPER $@
> + cmd_rust_helper = \
> + $(CC) $(filter-out -fno-delete-null-pointer-checks $(CFLAGS_REMOVE_helpers.o), $(c_flags) $(CFLAGS_helpers.o)) -S $< -emit-llvm -o $(patsubst %.bc,%.ll,$@); \
> + sed -i 's/^define dso_local/define linkonce_odr dso_local/g' $(patsubst %.bc,%.ll,$@); \
> + $(LLVM_AS) $(patsubst %.bc,%.ll,$@) -o $@
> +
> +$(obj)/helpers.bc: $(obj)/helpers.c FORCE
> + +$(call if_changed_dep,rust_helper)
> +
> quiet_cmd_exports = EXPORTS $@
> cmd_exports = \
> $(NM) -p --defined-only $< \
> @@ -396,11 +413,13 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
> OBJTREE=$(abspath $(objtree)) \
> $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
> $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
> - --emit=dep-info=$(depfile) --emit=obj=$@ \
> + --emit=dep-info=$(depfile) --emit=$(if $(link_helper),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) \
> --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
> --crate-type rlib -L$(objtree)/$(obj) \
> --crate-name $(patsubst %.o,%,$(notdir $@)) $< \
> --sysroot=/dev/null \
> + $(if $(link_helper),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(obj)/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> + $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
> $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
>
> rust-analyzer:
> @@ -463,4 +482,11 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
> $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
> +$(call if_changed_dep,rustc_library)
>
> +ifdef CONFIG_RUST_LTO_HELPERS
> +
> +$(obj)/kernel.o: private link_helper = 1
> +$(obj)/kernel.o: $(obj)/helpers.bc
> +
> +endif
> +
> endif # CONFIG_RUST
> diff --git a/rust/exports.c b/rust/exports.c
> index aa1218b325e5..212c1ec1d38b 100644
> --- a/rust/exports.c
> +++ b/rust/exports.c
> @@ -19,7 +19,10 @@
> #include "exports_alloc_generated.h"
> #include "exports_bindings_generated.h"
> #include "exports_kernel_generated.h"
> +
> +#ifndef CONFIG_RUST_LTO_HELPERS
> #include "exports_helpers_generated.h"
> +#endif
>
> // For modules using `rust/build_error.rs`.
> #ifdef CONFIG_RUST_BUILD_ASSERT_ALLOW
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 895f4b696962..3abf96f14148 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -32,18 +32,19 @@
> #include <linux/spinlock.h>
> #include <linux/wait.h>
> #include <linux/workqueue.h>
> +#include "helpers.h"
>
> -__noreturn void rust_helper_BUG(void)
> +__rust_helper __noreturn void rust_helper_BUG(void)
> {
> BUG();
> }
>
> -void rust_helper_mutex_lock(struct mutex *lock)
> +__rust_helper void rust_helper_mutex_lock(struct mutex *lock)
> {
> mutex_lock(lock);
> }
>
> -void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
> +__rust_helper void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
> struct lock_class_key *key)
> {
> #ifdef CONFIG_DEBUG_SPINLOCK
> @@ -53,82 +54,82 @@ void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
> #endif
> }
>
> -void rust_helper_spin_lock(spinlock_t *lock)
> +__rust_helper void rust_helper_spin_lock(spinlock_t *lock)
> {
> spin_lock(lock);
> }
>
> -void rust_helper_spin_unlock(spinlock_t *lock)
> +__rust_helper void rust_helper_spin_unlock(spinlock_t *lock)
> {
> spin_unlock(lock);
> }
>
> -void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
> +__rust_helper void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
> {
> init_wait(wq_entry);
> }
>
> -int rust_helper_signal_pending(struct task_struct *t)
> +__rust_helper int rust_helper_signal_pending(struct task_struct *t)
> {
> return signal_pending(t);
> }
>
> -refcount_t rust_helper_REFCOUNT_INIT(int n)
> +__rust_helper refcount_t rust_helper_REFCOUNT_INIT(int n)
> {
> return (refcount_t)REFCOUNT_INIT(n);
> }
>
> -void rust_helper_refcount_inc(refcount_t *r)
> +__rust_helper void rust_helper_refcount_inc(refcount_t *r)
> {
> refcount_inc(r);
> }
>
> -bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +__rust_helper bool rust_helper_refcount_dec_and_test(refcount_t *r)
> {
> return refcount_dec_and_test(r);
> }
>
> -__force void *rust_helper_ERR_PTR(long err)
> +__rust_helper __force void *rust_helper_ERR_PTR(long err)
> {
> return ERR_PTR(err);
> }
>
> -bool rust_helper_IS_ERR(__force const void *ptr)
> +__rust_helper bool rust_helper_IS_ERR(__force const void *ptr)
> {
> return IS_ERR(ptr);
> }
>
> -long rust_helper_PTR_ERR(__force const void *ptr)
> +__rust_helper long rust_helper_PTR_ERR(__force const void *ptr)
> {
> return PTR_ERR(ptr);
> }
>
> -const char *rust_helper_errname(int err)
> +__rust_helper const char *rust_helper_errname(int err)
> {
> return errname(err);
> }
>
> -struct task_struct *rust_helper_get_current(void)
> +__rust_helper struct task_struct *rust_helper_get_current(void)
> {
> return current;
> }
>
> -void rust_helper_get_task_struct(struct task_struct *t)
> +__rust_helper void rust_helper_get_task_struct(struct task_struct *t)
> {
> get_task_struct(t);
> }
>
> -void rust_helper_put_task_struct(struct task_struct *t)
> +__rust_helper void rust_helper_put_task_struct(struct task_struct *t)
> {
> put_task_struct(t);
> }
>
> -struct kunit *rust_helper_kunit_get_current_test(void)
> +__rust_helper struct kunit *rust_helper_kunit_get_current_test(void)
> {
> return kunit_get_current_test();
> }
>
> -void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
> +__rust_helper void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
> bool onstack, const char *name,
> struct lock_class_key *key)
> {
> @@ -139,7 +140,7 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
> work->func = func;
> }
>
> -void * __must_check __realloc_size(2)
> +__rust_helper void * __must_check __realloc_size(2)
> rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
> {
> return krealloc(objp, new_size, flags);
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index efacca63c897..201e7dc5ae5d 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -288,7 +288,10 @@ rust_common_cmd = \
> # would not match each other.
>
> quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> - cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<
> + cmd_rustc_o_rs = $(rust_common_cmd) --emit=$(if $(CONFIG_RUST_LTO_HELPERS),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) $< \
> + $(if $(CONFIG_RUST_LTO_HELPERS),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(objtree)/rust/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> + $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@)
> +
>
> $(obj)/%.o: $(obj)/%.rs FORCE
> +$(call if_changed_dep,rustc_o_rs)
> --
> 2.42.0
>
>

2024-06-12 16:51:35

by Gary Guo

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] kbuild: rust: provide an option to inline C helpers into Rust

On Fri, 7 Jun 2024 07:40:45 -0700
Matthew Maurer <[email protected]> wrote:

> The LLVM `.ll` textual bitcode representation may change from version
> to version, which makes using `sed` to edit it potentially fragile.
> Would it be possible to attach similar attributes without this? (Or
> other attributes that can accomplish something similar, see the second
> part.)

I agree that text editing is fragile and should be avoided, but I'm not
aware of a way to achieve this without text editing.

> Would it be possible to use a Rust-only-LTO (actual LTO) flag rather
> than a `llvm-link`? It seems likely that someone that wants to use a
> synced `clang` and `rustc` to allow helpers to be inlined would be
> unlikely to object to thin LTO of Rust. This would avoid introducing a
> new tool.

There's no way for rustc to actually perform the LTO, since the crate
type that we use is rlib rather than staticlibs or cdylibs, Rust does
not take care of the linking.

> As one possible route around editing the `.ll` file, we could consider
> *not* hiding these symbols, just marking them as "good idea to inline
> if possible", and then masking any symbol that ends up in the final
> `.o` via `objcopy --localize-symbol=blah`. If we're doing full-kernel
> LTO rather than rust-only LTO, we'd need to use `llvm-objcopy`
> instead, but the same process would follow - a postprocessing step on
> the final crate object that localizes the symbol, with LTO giving the
> compiler the option (but not requirement) to inline these functions if
> it seems like a good idea.

This was one of the approach that I considered, but IIUC objcopy
require you to supply it a list of exact-match symbols rather than
patterns, so we have to get all helper symbols and then feed it to
objcopy. I really want to avoid having a script or host program to do
this step.

Best,
Gary



2024-06-12 21:29:13

by Boqun Feng

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] kbuild: rust: provide an option to inline C helpers into Rust

On Wed, May 29, 2024 at 09:28:15PM +0100, Gary Guo wrote:
[...]
> +$(obj)/helpers.bc: $(obj)/helpers.c FORCE
> + +$(call if_changed_dep,rust_helper)
> +
> quiet_cmd_exports = EXPORTS $@
> cmd_exports = \
> $(NM) -p --defined-only $< \
> @@ -396,11 +413,13 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
> OBJTREE=$(abspath $(objtree)) \
> $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
> $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
> - --emit=dep-info=$(depfile) --emit=obj=$@ \
> + --emit=dep-info=$(depfile) --emit=$(if $(link_helper),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) \
> --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
> --crate-type rlib -L$(objtree)/$(obj) \
> --crate-name $(patsubst %.o,%,$(notdir $@)) $< \
> --sysroot=/dev/null \
> + $(if $(link_helper),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(obj)/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> + $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \

I hit some errors when I tried to cross-compile arm64 on x86. Adding
$(CLANG_FLAGS) here resolves my issue, it's really because the target
selection for clang is done in $(CLANG_FLAGS) and carried into
$(KBUILD_CPPFLAGS), but seems you cannot use $(KBUILD_CPPFLAGS) here
because you only use clang as a linker. Without either, clang will just
use the target of host.

diff --git a/rust/Makefile b/rust/Makefile
index ad0797467102..84d698c09c65 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -419,7 +419,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
--crate-name $(patsubst %.o,%,$(notdir $@)) $< \
--sysroot=/dev/null \
$(if $(link_helper),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(obj)/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
- $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
+ $(CC) $(CLANG_FLAGS) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)

rust-analyzer:

> $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
>
> rust-analyzer:
> @@ -463,4 +482,11 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
> $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
> +$(call if_changed_dep,rustc_library)
>
> +ifdef CONFIG_RUST_LTO_HELPERS
> +
> +$(obj)/kernel.o: private link_helper = 1
> +$(obj)/kernel.o: $(obj)/helpers.bc
> +
> +endif
> +
> endif # CONFIG_RUST
[...]
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index efacca63c897..201e7dc5ae5d 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -288,7 +288,10 @@ rust_common_cmd = \
> # would not match each other.
>
> quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> - cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<
> + cmd_rustc_o_rs = $(rust_common_cmd) --emit=$(if $(CONFIG_RUST_LTO_HELPERS),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) $< \
> + $(if $(CONFIG_RUST_LTO_HELPERS),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(objtree)/rust/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> + $(CC) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@)
> +
>

Ditto.

Regards,
Boqun

> $(obj)/%.o: $(obj)/%.rs FORCE
> +$(call if_changed_dep,rustc_o_rs)
> --
> 2.42.0
>