Message-ID: <CALzav=edxTsa7uO7XxiUSx+DZiX169T4WL39vYsn3_WcUuVKrg@mail.gmail.com>
Date: Mon, 17 Nov 2025 11:27:44 -0800
From: David Matlack <dmatlack@...gle.com>
To: Pasha Tatashin <pasha.tatashin@...een.com>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com, rppt@...nel.org, 
	rientjes@...gle.com, corbet@....net, rdunlap@...radead.org, 
	ilpo.jarvinen@...ux.intel.com, kanie@...ux.alibaba.com, ojeda@...nel.org, 
	aliceryhl@...gle.com, masahiroy@...nel.org, akpm@...ux-foundation.org, 
	tj@...nel.org, yoann.congal@...le.fr, mmaurer@...gle.com, 
	roman.gushchin@...ux.dev, chenridong@...wei.com, axboe@...nel.dk, 
	mark.rutland@....com, jannh@...gle.com, vincent.guittot@...aro.org, 
	hannes@...xchg.org, dan.j.williams@...el.com, david@...hat.com, 
	joel.granados@...nel.org, rostedt@...dmis.org, anna.schumaker@...cle.com, 
	song@...nel.org, linux@...ssschuh.net, linux-kernel@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-mm@...ck.org, gregkh@...uxfoundation.org, 
	tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, rafael@...nel.org, 
	dakr@...nel.org, bartosz.golaszewski@...aro.org, cw00.choi@...sung.com, 
	myungjoo.ham@...sung.com, yesanishhere@...il.com, Jonathan.Cameron@...wei.com, 
	quic_zijuhu@...cinc.com, aleksander.lobakin@...el.com, ira.weiny@...el.com, 
	andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de, 
	bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com, 
	stuart.w.hayes@...il.com, ptyadav@...zon.de, lennart@...ttering.net, 
	brauner@...nel.org, linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	saeedm@...dia.com, ajayachandra@...dia.com, jgg@...dia.com, parav@...dia.com, 
	leonro@...dia.com, witu@...dia.com, hughd@...gle.com, skhawaja@...gle.com, 
	chrisl@...nel.org
Subject: Re: [PATCH v6 18/20] selftests/liveupdate: Add kexec-based selftest
 for session lifecycle

On Sat, Nov 15, 2025 at 3:34 PM Pasha Tatashin
<pasha.tatashin@...een.com> wrote:

> diff --git a/tools/testing/selftests/liveupdate/Makefile b/tools/testing/selftests/liveupdate/Makefile
> index 2a573c36016e..1563ac84006a 100644
> --- a/tools/testing/selftests/liveupdate/Makefile
> +++ b/tools/testing/selftests/liveupdate/Makefile
> @@ -1,7 +1,39 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +
> +KHDR_INCLUDES ?= -I../../../../usr/include

You shouldn't need to set this variable and $(OUTPUT). Both should be
provided by lib.mk. Maybe the include is too far down?

>  CFLAGS += -Wall -O2 -Wno-unused-function
>  CFLAGS += $(KHDR_INCLUDES)
> +LDFLAGS += -static

Is static build really required or just for your setup? If it's
setup-specific, I would recommend letting the user pass in -static via
EXTRA_CFLAGS. That what we do in the KVM and VFIO selftests.

CFLAGS += $(EXTRA_CFLAGS)

Then the user can pass EXTRA_CFLAGS=-static on the command line.

> +OUTPUT ?= .
> +
> +# --- Test Configuration (Edit this section when adding new tests) ---
> +LUO_SHARED_SRCS := luo_test_utils.c
> +LUO_SHARED_HDRS += luo_test_utils.h

I would suggest using the -MD flag and Make's -include directive to
automatically handle headers. That way you don't need to add every
header to Makefile for Make to detect changes. See the end of my email
for how to do this.

> +
> +LUO_MANUAL_TESTS += luo_kexec_simple
> +
> +TEST_FILES += do_kexec.sh
>
>  TEST_GEN_PROGS += liveupdate
>
> +# --- Automatic Rule Generation (Do not edit below) ---
> +
> +TEST_GEN_PROGS_EXTENDED += $(LUO_MANUAL_TESTS)
> +
> +# Define the full list of sources for each manual test.
> +$(foreach test,$(LUO_MANUAL_TESTS), \
> +       $(eval $(test)_SOURCES := $(test).c $(LUO_SHARED_SRCS)))

This does not build with Google's gbuild wrapper around make. I get
these errors (after fixing the semi-colon issue below):

clang: error: no such file or directory: 'luo_kexec_simple.c'
clang: error: no such file or directory: 'luo_test_utils.c'
clang: error: no such file or directory: 'luo_test_utils.h'

> +
> +# This loop automatically generates an explicit build rule for each manual test.
> +# It includes dependencies on the shared headers and makes the output
> +# executable.
> +# Note the use of '$$' to escape automatic variables for the 'eval' command.
> +$(foreach test,$(LUO_MANUAL_TESTS), \
> +       $(eval $(OUTPUT)/$(test): $($(test)_SOURCES) $(LUO_SHARED_HDRS) \
> +               $(call msg,LINK,,$$@) ; \
> +               $(Q)$(LINK.c) $$^ $(LDLIBS) -o $$@ ; \
> +               $(Q)chmod +x $$@ \

These semi-colons swollow any errors. I would recommend against using
a foreach and eval. Make supports pattern-based targets so there's
really no need for loops. See below.

> +       ) \
> +)
> +
>  include ../lib.mk

Putting it all together, here is what I'd recommend for this Makefile
(drop-in replacement for the current Makefile). This will also make it
easier for me to share the library code with VFIO selftests, which
I'll need to do in the VFIO series.

(Sorry in advance for the line wrap. I had to send this through gmail.)

# SPDX-License-Identifier: GPL-2.0-only

LIBLIVEUPDATE_C += luo_test_utils.c

TEST_GEN_PROGS_EXTENDED += luo_kexec_simple
TEST_GEN_PROGS_EXTENDED += luo_multi_session

TEST_FILES += do_kexec.sh

include ../lib.mk

CFLAGS += $(KHDR_INCLUDES)
CFLAGS += -Wall -O2 -Wno-unused-function
CFLAGS += -MD
CFLAGS += $(EXTRA_CFLAGS)

LIBLIVEUPDATE_O := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBLIVEUPDATE_C))
TEST_GEN_PROGS_EXTENDED_O += $(patsubst %, %.o, $(TEST_GEN_PROGS_EXTENDED))

TEST_DEP_FILES += $(patsubst %.o, %.d, $(LIBLIVEUPDATE_O))
TEST_DEP_FILES += $(patsubst %.o, %.d, $(TEST_GEN_PROGS_EXTENDED_O))
-include $(TEST_DEP_FILES)

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

$(TEST_GEN_PROGS_EXTENDED): %: %.o $(LIBLIVEUPDATE_O)
        $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $<
$(LIBLIVEUPDATE_O) $(LDLIBS) -o $@

EXTRA_CLEAN += $(LIBLIVEUPDATE_O) $(TEST_GEN_PROGS_EXTENDED_O) $(TEST_DEP_FILES)

Powered by blists - more mailing lists