[1/6] kbuild: specify output names separately for each emission type from rustc
Commit Message
In Kbuild, two different rules must not write to the same file, but
it happens when compiling rust source files.
For example, set CONFIG_SAMPLE_RUST_MINIMAL=m and run the following:
$ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
[snip]
RUSTC [M] samples/rust/rust_minimal.o
RUSTC [M] samples/rust/rust_minimal.rsi
RUSTC [M] samples/rust/rust_minimal.s
RUSTC [M] samples/rust/rust_minimal.ll
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:334: samples/rust/rust_minimal.ll] Error 1
make[3]: *** Waiting for unfinished jobs....
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:309: samples/rust/rust_minimal.o] Error 1
mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
make[3]: *** [scripts/Makefile.build:326: samples/rust/rust_minimal.s] Error 1
make[2]: *** [scripts/Makefile.build:504: samples/rust] Error 2
make[1]: *** [scripts/Makefile.build:504: samples] Error 2
make: *** [Makefile:2008: .] Error 2
The reason for the error is that 4 threads running in parallel creates
and renames the same file path, samples/rust/rust_minimal.d.
This does not happen when compiling C or assembly files because we
explicitly specify the dependency filename by using the preprocessor
option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target.
Currently, rustc is only given --out-dir and the list of emitted types.
So, all the rust build rules output the dep-info into the default
<CRATE_NAME>.d, causing the conflict.
Fortunately, the --emit option is able to specify the output path
individually, with the form --emit=<type>=<path>.
Add --emit=dep-info=$(depfile) to the common command part. Also, remove
the redundant --out-dir because we specify the output path for each type.
The code gets much cleaner because we do not need to rename *.d files.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
rust/Makefile | 10 ++++------
scripts/Makefile.build | 14 +++++++-------
scripts/Makefile.host | 9 +++------
3 files changed, 14 insertions(+), 19 deletions(-)
Comments
On Sat, 31 Dec 2022 15:41:58 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:
> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index da133780b751..4434cdbf7b8e 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
> $(HOSTCFLAGS_$(target-stem).o)
> _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
> $(HOSTCXXFLAGS_$(target-stem).o)
> -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> - $(HOSTRUSTFLAGS_$(target-stem))
> +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> + $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
>
> # $(objtree)/$(obj) for including generated headers from checkin source files
> ifeq ($(KBUILD_EXTMOD),)
> @@ -97,7 +97,6 @@ endif
>
> hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags)
> hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> -hostrust_flags = $(_hostrust_flags)
Would it be better to have `--emit=dep-info=$(depfile)` added here
instead so that it mimics c/cxx flags?
>
> #####
> # Compile programs on the host
> @@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
> # host-rust -> Executable
> quiet_cmd_host-rust = HOSTRUSTC $@
> cmd_host-rust = \
> - $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \
> - --out-dir=$(obj)/ $<; \
> - mv $(obj)/$(target-stem).d $(depfile); \
> + $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
> sed -i '/^\#/d' $(depfile)
> $(host-rust): $(obj)/%: $(src)/%.rs FORCE
> $(call if_changed_dep,host-rust)
Best,
Gary
On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
> samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
Yeah, we were testing the single targets, but not multiple at once, thanks!
> + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
Perhaps a newline here to avoid the lengthy line?
> hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags)
> hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> -hostrust_flags = $(_hostrust_flags)
This was originally meant to be consistent with C and C++ indeed, but
if you prefer less variables, I guess it is fine, in which case,
should we update the C/C++ side too (in another series)?
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Tested-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
On Sat Dec 31, 2022 at 7:41 AM CET, Masahiro Yamada wrote:
> In Kbuild, two different rules must not write to the same file, but
> it happens when compiling rust source files.
>
> For example, set CONFIG_SAMPLE_RUST_MINIMAL=m and run the following:
>
> $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
> samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
> [snip]
> RUSTC [M] samples/rust/rust_minimal.o
> RUSTC [M] samples/rust/rust_minimal.rsi
> RUSTC [M] samples/rust/rust_minimal.s
> RUSTC [M] samples/rust/rust_minimal.ll
> mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
> make[3]: *** [scripts/Makefile.build:334: samples/rust/rust_minimal.ll] Error 1
> make[3]: *** Waiting for unfinished jobs....
> mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
> make[3]: *** [scripts/Makefile.build:309: samples/rust/rust_minimal.o] Error 1
> mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory
> make[3]: *** [scripts/Makefile.build:326: samples/rust/rust_minimal.s] Error 1
> make[2]: *** [scripts/Makefile.build:504: samples/rust] Error 2
> make[1]: *** [scripts/Makefile.build:504: samples] Error 2
> make: *** [Makefile:2008: .] Error 2
>
> The reason for the error is that 4 threads running in parallel creates
> and renames the same file path, samples/rust/rust_minimal.d.
>
> This does not happen when compiling C or assembly files because we
> explicitly specify the dependency filename by using the preprocessor
> option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target.
>
> Currently, rustc is only given --out-dir and the list of emitted types.
> So, all the rust build rules output the dep-info into the default
> <CRATE_NAME>.d, causing the conflict.
>
> Fortunately, the --emit option is able to specify the output path
> individually, with the form --emit=<type>=<path>.
>
> Add --emit=dep-info=$(depfile) to the common command part. Also, remove
> the redundant --out-dir because we specify the output path for each type.
>
> The code gets much cleaner because we do not need to rename *.d files.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> rust/Makefile | 10 ++++------
> scripts/Makefile.build | 14 +++++++-------
> scripts/Makefile.host | 9 +++------
> 3 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index ff70c4c916f8..0e2a32f4b3e9 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -331,10 +331,9 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
> quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
> cmd_rustc_procmacro = \
> $(RUSTC_OR_CLIPPY) $(rust_common_flags) \
> - --emit=dep-info,link --extern proc_macro \
> - --crate-type proc-macro --out-dir $(objtree)/$(obj) \
> + --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
> + --crate-type proc-macro \
> --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
> - mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \
> sed -i '/^\#/d' $(depfile)
>
> # Procedural macros can only be used with the `rustc` that compiled it.
> @@ -348,10 +347,9 @@ 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,obj,metadata --crate-type rlib \
> - --out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \
> + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
> + --crate-type rlib -L$(objtree)/$(obj) \
> --crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
> - mv $(objtree)/$(obj)/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \
> sed -i '/^\#/d' $(depfile) \
> $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index a0d5c6cca76d..40de20246e50 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -285,11 +285,11 @@ rust_common_cmd = \
> -Zcrate-attr=no_std \
> -Zcrate-attr='feature($(rust_allowed_features))' \
> --extern alloc --extern kernel \
> - --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \
> - --crate-name $(basename $(notdir $@))
> + --crate-type rlib -L $(objtree)/rust/ \
> + --crate-name $(basename $(notdir $@)) \
> + --emit=dep-info=$(depfile)
>
> rust_handle_depfile = \
> - mv $(obj)/$(basename $(notdir $@)).d $(depfile); \
> sed -i '/^\#/d' $(depfile)
>
> # `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit
> @@ -302,7 +302,7 @@ rust_handle_depfile = \
>
> quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> cmd_rustc_o_rs = \
> - $(rust_common_cmd) --emit=dep-info,obj $<; \
> + $(rust_common_cmd) --emit=obj=$@ $<; \
> $(rust_handle_depfile)
>
> $(obj)/%.o: $(src)/%.rs FORCE
> @@ -310,7 +310,7 @@ $(obj)/%.o: $(src)/%.rs FORCE
>
> quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> cmd_rustc_rsi_rs = \
> - $(rust_common_cmd) --emit=dep-info -Zunpretty=expanded $< >$@; \
> + $(rust_common_cmd) -Zunpretty=expanded $< >$@; \
> command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \
> $(rust_handle_depfile)
>
> @@ -319,7 +319,7 @@ $(obj)/%.rsi: $(src)/%.rs FORCE
>
> quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> cmd_rustc_s_rs = \
> - $(rust_common_cmd) --emit=dep-info,asm $<; \
> + $(rust_common_cmd) --emit=asm=$@ $<; \
> $(rust_handle_depfile)
>
> $(obj)/%.s: $(src)/%.rs FORCE
> @@ -327,7 +327,7 @@ $(obj)/%.s: $(src)/%.rs FORCE
>
> quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> cmd_rustc_ll_rs = \
> - $(rust_common_cmd) --emit=dep-info,llvm-ir $<; \
> + $(rust_common_cmd) --emit=llvm-ir=$@ $<; \
> $(rust_handle_depfile)
>
> $(obj)/%.ll: $(src)/%.rs FORCE
> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index da133780b751..4434cdbf7b8e 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
> $(HOSTCFLAGS_$(target-stem).o)
> _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
> $(HOSTCXXFLAGS_$(target-stem).o)
> -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> - $(HOSTRUSTFLAGS_$(target-stem))
> +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> + $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
>
> # $(objtree)/$(obj) for including generated headers from checkin source files
> ifeq ($(KBUILD_EXTMOD),)
> @@ -97,7 +97,6 @@ endif
>
> hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags)
> hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> -hostrust_flags = $(_hostrust_flags)
>
> #####
> # Compile programs on the host
> @@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
> # host-rust -> Executable
> quiet_cmd_host-rust = HOSTRUSTC $@
> cmd_host-rust = \
> - $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \
> - --out-dir=$(obj)/ $<; \
> - mv $(obj)/$(target-stem).d $(depfile); \
> + $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
> sed -i '/^\#/d' $(depfile)
> $(host-rust): $(obj)/%: $(src)/%.rs FORCE
> $(call if_changed_dep,host-rust)
> --
> 2.34.1
On Wed, Jan 4, 2023 at 5:45 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \
> > samples/rust/rust_minimal.s samples/rust/rust_minimal.ll
>
> Yeah, we were testing the single targets, but not multiple at once, thanks!
>
> > + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
>
> Perhaps a newline here to avoid the lengthy line?
OK, I will wrap it in v2.
>
> > hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags)
> > hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
> > -hostrust_flags = $(_hostrust_flags)
>
> This was originally meant to be consistent with C and C++ indeed, but
> if you prefer less variables, I guess it is fine, in which case,
> should we update the C/C++ side too (in another series)?
Yup, we could do this with less variables.
I will send a clean up.
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Tested-by: Miguel Ojeda <ojeda@kernel.org>
>
> Cheers,
> Miguel
@@ -331,10 +331,9 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
cmd_rustc_procmacro = \
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
- --emit=dep-info,link --extern proc_macro \
- --crate-type proc-macro --out-dir $(objtree)/$(obj) \
+ --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
+ --crate-type proc-macro \
--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
- mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)
# Procedural macros can only be used with the `rustc` that compiled it.
@@ -348,10 +347,9 @@ 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,obj,metadata --crate-type rlib \
- --out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \
+ --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
+ --crate-type rlib -L$(objtree)/$(obj) \
--crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
- mv $(objtree)/$(obj)/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile) \
$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
@@ -285,11 +285,11 @@ rust_common_cmd = \
-Zcrate-attr=no_std \
-Zcrate-attr='feature($(rust_allowed_features))' \
--extern alloc --extern kernel \
- --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \
- --crate-name $(basename $(notdir $@))
+ --crate-type rlib -L $(objtree)/rust/ \
+ --crate-name $(basename $(notdir $@)) \
+ --emit=dep-info=$(depfile)
rust_handle_depfile = \
- mv $(obj)/$(basename $(notdir $@)).d $(depfile); \
sed -i '/^\#/d' $(depfile)
# `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit
@@ -302,7 +302,7 @@ rust_handle_depfile = \
quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_o_rs = \
- $(rust_common_cmd) --emit=dep-info,obj $<; \
+ $(rust_common_cmd) --emit=obj=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.o: $(src)/%.rs FORCE
@@ -310,7 +310,7 @@ $(obj)/%.o: $(src)/%.rs FORCE
quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_rsi_rs = \
- $(rust_common_cmd) --emit=dep-info -Zunpretty=expanded $< >$@; \
+ $(rust_common_cmd) -Zunpretty=expanded $< >$@; \
command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \
$(rust_handle_depfile)
@@ -319,7 +319,7 @@ $(obj)/%.rsi: $(src)/%.rs FORCE
quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_s_rs = \
- $(rust_common_cmd) --emit=dep-info,asm $<; \
+ $(rust_common_cmd) --emit=asm=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.s: $(src)/%.rs FORCE
@@ -327,7 +327,7 @@ $(obj)/%.s: $(src)/%.rs FORCE
quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
cmd_rustc_ll_rs = \
- $(rust_common_cmd) --emit=dep-info,llvm-ir $<; \
+ $(rust_common_cmd) --emit=llvm-ir=$@ $<; \
$(rust_handle_depfile)
$(obj)/%.ll: $(src)/%.rs FORCE
@@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \
$(HOSTCFLAGS_$(target-stem).o)
_hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
$(HOSTCXXFLAGS_$(target-stem).o)
-_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
- $(HOSTRUSTFLAGS_$(target-stem))
+hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
+ $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile)
# $(objtree)/$(obj) for including generated headers from checkin source files
ifeq ($(KBUILD_EXTMOD),)
@@ -97,7 +97,6 @@ endif
hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags)
hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags)
-hostrust_flags = $(_hostrust_flags)
#####
# Compile programs on the host
@@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE
# host-rust -> Executable
quiet_cmd_host-rust = HOSTRUSTC $@
cmd_host-rust = \
- $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \
- --out-dir=$(obj)/ $<; \
- mv $(obj)/$(target-stem).d $(depfile); \
+ $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \
sed -i '/^\#/d' $(depfile)
$(host-rust): $(obj)/%: $(src)/%.rs FORCE
$(call if_changed_dep,host-rust)