A question of KVM selftests' makefile

A question of KVM selftests' makefile

[Yu Zhang]

Hi all,

  Currently, unlike the build system of Linux kernel, KVM selftests will
have to run "make clean && make" to rebuild the entire test suite, once
a header file is modified.

  Is it designed like this on purpose, or does anyone wanna change it?

  I hacked the makefile by using "-MD" as EXTRA_CFLAGS, so that dependency
rules can be generated for each target object, whose prerequisites contains
the source file and the included header files as well.

  However, this change has its own costs. E.g., new ".o" and ".d" files will
occupy more storage. And performance-wise, the benifit could be limited,
because for now, most header files are needed by almost every ".c" files.
But with the evolution of KVM selftests, more ".h" files may be added. Some
of which may be of special usage. E.g., file "include/x86_64/mce.h" is only
used by "x86_64/ucna_injection_test". Having to rebuild the whole test suite
just because one specific header is touched would be annoying.

  I am not sure if this change is worthy, or if there's a better solution.
So does anyone have any suggestion? 

  Below are my current hack in KVM selftest's makefile in case someone is
interested. It can not be optimal and any comment is welcome. Thanks!

B.R.
Yu

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 1750f91dd936..b329e0d1a460 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -180,6 +180,8 @@ TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR))
 TEST_GEN_PROGS_EXTENDED += $(TEST_GEN_PROGS_EXTENDED_$(ARCH_DIR))
 LIBKVM += $(LIBKVM_$(ARCH_DIR))

+OVERRIDE_TARGETS = 1
+
 # lib.mak defines $(OUTPUT), prepends $(OUTPUT)/ to $(TEST_GEN_PROGS), and most
 # importantly defines, i.e. overwrites, $(CC) (unless `make -e` or `make CC=`,
 # which causes the environment variable to override the makefile).
@@ -198,9 +200,11 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
        -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \
        -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \
        -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
-       -I$(<D) -Iinclude/$(ARCH_DIR) -I ../rseq -I.. $(EXTRA_CFLAGS) \
+       -I$(<D) -Iinclude/$(ARCH_DIR) -I ../rseq -I.. \
        $(KHDR_INCLUDES)

+EXTRA_CFLAGS += -MD
+
 no-pie-option := $(call try-run, echo 'int main(void) { return 0; }' | \
         $(CC) -Werror $(CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)

@@ -218,11 +222,22 @@ LIBKVM_S_OBJ := $(patsubst %.S, $(OUTPUT)/%.o, $(LIBKVM_S))
 LIBKVM_STRING_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_STRING))
 LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(LIBKVM_STRING_OBJ)

-EXTRA_CLEAN += $(LIBKVM_OBJS) cscope.*
+TEST_GEN_OBJ = $(patsubst %, %.o, $(TEST_GEN_PROGS))
+TEST_GEN_OBJ += $(patsubst %, %.o, $(TEST_GEN_PROGS_EXTENDED))
+TEST_DEP_FILES = $(patsubst %.o, %.d, $(TEST_GEN_OBJ))
+TEST_DEP_FILES += $(patsubst %.o, %.d, $(LIBKVM_OBJS))
+-include $(TEST_DEP_FILES)
+
+$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): %: %.o
+       $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $< $(LIBKVM_OBJS) $(LDLIBS) -o $@
+$(TEST_GEN_OBJ): %.o: %.c
+       $(CC) $(CFLAGS) $(CPPFLAGS) $(EXTRA_CFLAGS) $(TARGET_ARCH) -c $< -o $@
+
+EXTRA_CLEAN += $(LIBKVM_OBJS) $(TEST_DEP_FILES) $(TEST_GEN_OBJ) cscope.*

 x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))
 $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c
-       $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
+       $(CC) $(CFLAGS) $(CPPFLAGS) $(EXTRA_CFLAGS) $(TARGET_ARCH) -c $< -o $@

 $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S
        $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@

Re: A question of KVM selftests' makefile

[Sean Christopherson]

On Wed, Jan 25, 2023, Yu Zhang wrote:
> Hi all,
> 
>   Currently, unlike the build system of Linux kernel, KVM selftests will
> have to run "make clean && make" to rebuild the entire test suite, once
> a header file is modified.
> 
>   Is it designed like this on purpose,

My #1 rule: never assume anything weird in KVM was an explicit design choice :-)

>   or does anyone wanna change it?

Yes!

>   I hacked the makefile by using "-MD" as EXTRA_CFLAGS, so that dependency
> rules can be generated for each target object, whose prerequisites contains
> the source file and the included header files as well.
> 
>   However, this change has its own costs. E.g., new ".o" and ".d" files will
> occupy more storage. And performance-wise, the benifit could be limited,
> because for now, most header files are needed by almost every ".c" files.
> But with the evolution of KVM selftests, more ".h" files may be added. Some
> of which may be of special usage. E.g., file "include/x86_64/mce.h" is only
> used by "x86_64/ucna_injection_test". Having to rebuild the whole test suite
> just because one specific header is touched would be annoying.
> 
>   I am not sure if this change is worthy,

Absolutely worthy, I've run afoul of the lack of dependency tracking far too many
times.

> or if there's a better solution.

This part I don't know.  I would have cobbled together something long ago if I
wasn't so clueless about Makefiles.

Re: A question of KVM selftests' makefile

[Paolo Bonzini]

On 1/25/23 09:53, Yu Zhang wrote:
>   x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))
>   $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c
> -       $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> +       $(CC) $(CFLAGS) $(CPPFLAGS) $(EXTRA_CFLAGS) $(TARGET_ARCH) -c $< -o $@

Yes, this all looks very good.  Feel free to post it as a patch!

Paolo

Re: A question of KVM selftests' makefile

[Yu Zhang]

On Wed, Jan 25, 2023 at 07:46:47PM +0100, Paolo Bonzini wrote:
> On 1/25/23 09:53, Yu Zhang wrote:
> >   x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))
> >   $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c
> > -       $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> > +       $(CC) $(CFLAGS) $(CPPFLAGS) $(EXTRA_CFLAGS) $(TARGET_ARCH) -c $< -o $@
> 
> Yes, this all looks very good.  Feel free to post it as a patch!
> 
> Paolo
> 

Thank you so much, Paolo & Sean! Just posted the patch!

B.R.
Yu