Re: [PATCH v3 2/2] kbuild: rust: provide an option to inline C helpers into Rust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 19, 2025 at 4:53 PM Gary Guo <gary@xxxxxxxxxxx> wrote:
>
> A new Kconfig option, `RUST_INLINE_HELPERS` is added to allow C helpers
> (which were 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 global LTO.
>
> If the option is enabled, the following is performed:
> * For helpers, instead of compiling them to an object file to be linked
>   into vmlinux, they're compiled to LLVM IR.
> * The LLVM IR is patched to add `linkonce_odr` linkage. This linkage
>   means that functions are inlineable (effect of `_odr`), and the
>   symbols generated will have weak linkage if emitted into object file

either "object files" or "an 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

the subject of this clause is "the symbols generated", which is
plural, but here you use "it", which is singular.

"it will may" is also not correct; the word "will" looks misplaced.

>   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).

s/have/having/

> * When a Rust crate is compiled, instead of generating an 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 LTO, but this is much faster since it
>   only needs to inline the helpers.

nit: s/but this is much/but is much/

> * clang is invoked to turn the combined bitcode into a final object file.
>
> Some caveats with the option:
> * clang and Rust doesn't have the exact target string. Clang generates

s/doesn't/don't/ (because "clang and Rust" is plural)

>   `+cmov,+cx8,+fxsr` but Rust doesn't enable them (in fact, Rust will
>   complain if `-Ctarget-feature=+cmov,+cx8,+fxsr` is used). x86-64 always

is the fact that rustc complains a bug?

>   enable these features, so they are in fact the same target string, but

s/enable/enables/

>   LLVM doesn't understand this and so inlining is inhibited. This is bypassed
>   with `--ignore-tti-inline-compatible`.
> * LLVM doesn't want to inline functions compiled with
>   `-fno-delete-null-pointer-checks` with code compiled without. So we
>   remove this flag when compiling helpers. This is okay since this is one of
>   the hardening features that does not change the ABI, and we shouldn't have
>   null pointer dereferences in these helpers.
>
> The checks can also be bypassed with force inlining (`__always_inline`),
> but doing so would also bypass inlining cost analysis.
>
> Reviewed-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
> Tested-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
> Co-developed-by: Boqun Feng <boqun.feng@xxxxxxxxx>
> Signed-off-by: Boqun Feng <boqun.feng@xxxxxxxxx>
> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>
> ---
>  Makefile                     |  4 +++-
>  lib/Kconfig.debug            | 14 ++++++++++++
>  rust/Makefile                | 34 ++++++++++++++++++++++++++----
>  rust/exports.c               |  5 ++++-
>  rust/helpers/blk.c           |  5 +++--
>  rust/helpers/bug.c           |  3 ++-
>  rust/helpers/build_bug.c     |  3 ++-
>  rust/helpers/cred.c          |  5 +++--
>  rust/helpers/device.c        |  6 +++---
>  rust/helpers/err.c           |  7 +++---
>  rust/helpers/fs.c            |  3 ++-
>  rust/helpers/helpers.h       | 13 ++++++++++++
>  rust/helpers/io.c            | 41 ++++++++++++++++++++----------------
>  rust/helpers/jump_label.c    |  3 ++-
>  rust/helpers/kunit.c         |  3 ++-
>  rust/helpers/mutex.c         | 10 +++++----
>  rust/helpers/page.c          |  8 ++++---
>  rust/helpers/pci.c           |  8 ++++---
>  rust/helpers/pid_namespace.c |  9 +++++---
>  rust/helpers/platform.c      |  7 ++++--
>  rust/helpers/rbtree.c        |  6 ++++--
>  rust/helpers/rcu.c           |  5 +++--
>  rust/helpers/refcount.c      |  7 +++---
>  rust/helpers/security.c      |  9 +++++---
>  rust/helpers/signal.c        |  3 ++-
>  rust/helpers/slab.c          |  9 ++++----
>  rust/helpers/spinlock.c      | 14 ++++++------
>  rust/helpers/task.c          | 23 ++++++++++----------
>  rust/helpers/uaccess.c       |  9 ++++----
>  rust/helpers/vmalloc.c       |  5 +++--
>  rust/helpers/wait.c          |  3 ++-
>  rust/helpers/workqueue.c     |  9 +++++---
>  scripts/Makefile.build       |  5 ++++-
>  33 files changed, 201 insertions(+), 97 deletions(-)
>  create mode 100644 rust/helpers/helpers.h
>
> diff --git a/Makefile b/Makefile
> index 70bdbf2218fc..97756ecd5a56 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -510,6 +510,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
> @@ -617,7 +619,7 @@ export RUSTC_BOOTSTRAP := 1
>  export CLIPPY_CONF_DIR := $(srctree)
>
>  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
> +export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN 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 1af972a92d06..42e26f3377d1 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -3289,6 +3289,20 @@ config RUST_KERNEL_DOCTESTS
>
>           If unsure, say N.
>
> +config RUST_INLINE_HELPERS
> +    bool "Inline C helpers into Rust crates"
> +    depends on RUST && RUSTC_CLANG_LLVM_COMPATIBLE
> +    help
> +        Links C helpers into Rust crates through LLVM IR.
> +
> +        If this option is enabled, instead of generating object files directly,
> +        rustc is asked to produce LLVM IR, which is then linked together with
> +        the LLVM IR of C helpers, before object file is generated.

from previous review: should this say bitcode instead of IR?

"before object file is" should be "before object files are" since you
used plural in the prior clause.

> +
> +        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 e761a8cc3bd5..07fc4f5d0e36 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -6,15 +6,19 @@ rustdoc_output := $(objtree)/Documentation/output/rust/rustdoc
>  obj-$(CONFIG_RUST) += core.o compiler_builtins.o ffi.o
>  always-$(CONFIG_RUST) += exports_core_generated.h
>
> +ifdef CONFIG_RUST_INLINE_HELPERS
> +always-$(CONFIG_RUST) += helpers/helpers.bc
> +else
> +obj-$(CONFIG_RUST) += helpers/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/helpers.o
>  CFLAGS_REMOVE_helpers/helpers.o = -Wmissing-prototypes -Wmissing-declarations
>
>  always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
>  obj-$(CONFIG_RUST) += bindings.o pin_init.o kernel.o
> -always-$(CONFIG_RUST) += exports_helpers_generated.h \
> -    exports_bindings_generated.h exports_kernel_generated.h
> +always-$(CONFIG_RUST) += exports_bindings_generated.h exports_kernel_generated.h
>
>  always-$(CONFIG_RUST) += uapi/uapi_generated.rs
>  obj-$(CONFIG_RUST) += uapi.o
> @@ -360,6 +364,21 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ;
>  $(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers/helpers.c FORCE
>         $(call if_changed_dep,bindgen)
>
> +# When compiling helpers, filter `-fno-delete-null-pointer-checks` since LLVM
> +# prevents inlining such functions into functions compiled without the option
> +# (e.g. Rust functions).
> +#
> +# `linkonce_odr` is added to make functions inlineable and allow LLVM to
> +# omit unreferenced functions.
> +quiet_cmd_rust_helper = HELPER  $@
> +      cmd_rust_helper = \
> +       $(CC) $(filter-out -fno-delete-null-pointer-checks $(CFLAGS_REMOVE_helpers/helpers.o), $(c_flags)) -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/helpers.bc: $(obj)/helpers/helpers.c FORCE
> +       +$(call if_changed_dep,rust_helper)

from prior review: should all these rules be defined iff
CONFIG_RUST_INLINE_HELPERS? Always defining them seems like it could
lead to subtle bugs, but perhaps there's Makefile precedent I'm not
aware of.

> +
>  rust_exports = $(NM) -p --defined-only $(1) | awk '$$2~/(T|R|D|B)/ && $$3!~/__cfi/ && $$3!~/__odr_asan/ { printf $(2),$$3 }'
>
>  quiet_cmd_exports = EXPORTS $@
> @@ -409,11 +428,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/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> +               $(CC) $(CLANG_FLAGS) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
>         $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@) \
>         $(cmd_objtool)
>
> @@ -522,4 +543,9 @@ ifdef CONFIG_JUMP_LABEL
>  $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs
>  endif
>
> +ifdef CONFIG_RUST_INLINE_HELPERS
> +$(obj)/kernel.o: private link_helper = 1
> +$(obj)/kernel.o: $(obj)/helpers/helpers.bc
> +endif


[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux