Re: [PATCH kernel] RFC: zstd: Fixing mixed module-builtin objects

[Masahiro Yamada]

On Thu, Apr 28, 2022 at 1:39 PM Alexey Kardashevskiy <aik@xxxxxxxxx> wrote:
>
> With CONFIG_ZSTD_COMPRESS=m and CONFIG_ZSTD_DECOMPRESS=y we end up in
> a situation when files from lib/zstd/common/ are compiled once for
> ZSTD_DECOMPRESS (build-in) and ZSTD_COMPRESS (module) even though
> CFLAGS are different for builtins and modules. So far somehow
> this was not a problem until enabling LLVM LTO where it fails:

I did not notice this potential issue, but you are right.

When an object is shared between vmlinux and a module,
it is compiled with module flags (module wins).


I did not come up with a clean solution in Kbuild.
To avoid this situation, Kbuild must compile different
objects for each, but doing that would complicate Kbuild.



I'd suggest to stop object sharing among modules.

One possible solution might be to move common objects
to a separate zstd_common.ko

  - zstd_compress.ko
  - zstd_decompress.ko
  - zstd_common.ko

You may need to add some EXPORT_SYMBOL
to common functions.




> ld.lld: error: linking module flags 'Code Model': IDs have conflicting values in 'lib/built-in.a(zstd_common.o at 5868)' and 'ld-temp.o'
>
> This particular conflict is caused by KBUILD_CFLAGS=-mcmodel=medium vs.
> KBUILD_CFLAGS_MODULE=-mcmodel=large , modules use the large model on
> POWERPC as explained at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/Makefile?h=v5.18-rc4#n127
> but the current use of common files is wrong anyway.
>
> This works around the issue by inlining common files and fixing few
> conflicts.
>
> Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> ---
>
> I tried fixing it by hacking Makefile to produce a separate .o for "y" and
> "m", like this:
>
> +obj-common-y =  common/debug-y.o \
> +               common/entropy_common-y.o \
> +               common/error_private-y.o \
> +               common/fse_decompress-y.o \
> +               common/zstd_common-y.o
> +obj-common-m =  common/debug-m.o \
> +               common/entropy_common-m.o \
> +               common/error_private-m.o \
> +               common/fse_decompress-m.o \
> +               common/zstd_common-m.o
> +obj-common-m := $(addprefix $(obj)/, $(obj-common-m))
> +obj-common-y := $(addprefix $(obj)/, $(obj-common-y))
> +
> +$(obj-common-m): $(obj)/%-m.o: %.c FORCE
> +        $(call if_changed_dep,cc_o_c)
> +$(obj-common-y): $(obj)/%-y.o: %.c FORCE
> +        $(call if_changed_dep,cc_o_c)
> +


I did not look into this closely, but you need to
use if_changed_rule instead of if_changed_dep at least.





-- 
Best Regards
Masahiro Yamada