diff mbox series

[v2,1/1] Makefile: Add kernel modules related make targets

Message ID 20250801100935.974351-1-pvorel@suse.cz
State Changes Requested
Headers show
Series [v2,1/1] Makefile: Add kernel modules related make targets | expand

Checks

Context Check Description
ltpci/debian_stable_s390x-linux-gnu-gcc_s390x success success
ltpci/debian_stable_powerpc64le-linux-gnu-gcc_ppc64el success success
ltpci/debian_stable_aarch64-linux-gnu-gcc_arm64 success success
ltpci/debian_stable_gcc success success
ltpci/ubuntu_bionic_gcc success success
ltpci/quay-io-centos-centos_stream9_gcc success success
ltpci/opensuse-leap_latest_gcc success success
ltpci/debian_stable_gcc success success
ltpci/opensuse-archive_42-2_gcc success success
ltpci/debian_oldstable_gcc success success
ltpci/debian_testing_gcc success success
ltpci/alpine_latest_gcc success success
ltpci/ubuntu_jammy_gcc success success
ltpci/fedora_latest_clang success success
ltpci/debian_oldstable_clang success success
ltpci/debian_testing_clang success success

Commit Message

Petr Vorel Aug. 1, 2025, 10:09 a.m. UTC
LTP contains few kernel modules and tests which are using them.  These
require to be built with the same kernel headers as the running kernel
(SUT). Sometimes the best way to achieve this is to compile them on the
SUT.

Add 'modules', 'modules-clean' and 'modules-install' make targets to
make it easier to build them.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

I'm sorry for the noise.

Most of the people will need just modules-install, but let's be
consistent with other LTP make targets.

Kind regards,
Petr

Changes v1->v2:
* Add also modules-clean and modules-install
This is needed as 'make modules clean' or 'make modules install'
would be running 2 separate targets.

v1:
https://lore.kernel.org/ltp/20250801094205.965645-1-pvorel@suse.cz/
https://patchwork.ozlabs.org/project/ltp/patch/20250801094205.965645-1-pvorel@suse.cz/

 INSTALL  | 10 ++++++++++
 Makefile | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Petr Vorel Aug. 1, 2025, 10:20 a.m. UTC | #1
Hi,

> Changes v1->v2:
> * Add also modules-clean and modules-install
> This is needed as 'make modules clean' or 'make modules install'
> would be running 2 separate targets.

Also, I wondered about fail when kernel modules aren't build:

include/mk/module.mk contains:

ifeq ($(WITH_MODULES),no)
SKIP := 1
else
ifeq ($(LINUX_VERSION_MAJOR)$(LINUX_VERSION_PATCH),)
SKIP := 1
else
SKIP ?= $(shell \
	[ "$(LINUX_VERSION_MAJOR)" -gt "$(REQ_VERSION_MAJOR)" ] || \
	[ "$(LINUX_VERSION_MAJOR)" -eq "$(REQ_VERSION_MAJOR)" -a \
	  "$(LINUX_VERSION_PATCH)" -ge "$(REQ_VERSION_PATCH)" ]; echo $$?)
endif
endif

ifneq ($(SKIP),0)
MAKE_TARGETS := $(filter-out %.ko, $(MAKE_TARGETS))
endif

# Ignoring the exit status of commands is done to be forward compatible with
# kernel internal API changes. The user-space test will return TCONF, if it
# doesn't find the module (i.e. it wasn't built either due to kernel-devel
# missing or module build failure).
%.ko: %.c .dep_modules ;
-----

We could allow to fail build with:
$ make modules FORCE=1

with these changes to include/mk/module.mk:

ifneq ($(SKIP),0)
MAKE_TARGETS := $(filter-out %.ko, $(MAKE_TARGETS))
ifeq ($(FORCE),1)
$(error Modules not built)
endif
endif

ifneq ($(FORCE),1)
%.ko: %.c .dep_modules ;
endif

Kind regards,
Petr
Li Wang Aug. 2, 2025, 3:25 a.m. UTC | #2
On Fri, Aug 1, 2025 at 6:09 PM Petr Vorel <pvorel@suse.cz> wrote:

> LTP contains few kernel modules and tests which are using them.  These
> require to be built with the same kernel headers as the running kernel
> (SUT). Sometimes the best way to achieve this is to compile them on the
> SUT.
>
> Add 'modules', 'modules-clean' and 'modules-install' make targets to
> make it easier to build them.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi,
>
> I'm sorry for the noise.
>
> Most of the people will need just modules-install, but let's be
> consistent with other LTP make targets.
>
> Kind regards,
> Petr
>
> Changes v1->v2:
> * Add also modules-clean and modules-install
> This is needed as 'make modules clean' or 'make modules install'
> would be running 2 separate targets.
>
> v1:
> https://lore.kernel.org/ltp/20250801094205.965645-1-pvorel@suse.cz/
>
> https://patchwork.ozlabs.org/project/ltp/patch/20250801094205.965645-1-pvorel@suse.cz/
>
>  INSTALL  | 10 ++++++++++
>  Makefile | 24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/INSTALL b/INSTALL
> index cbe27f32ea..10c19d4105 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -165,6 +165,16 @@ PKG_CONFIG_LIBDIR=/usr/lib/i386-linux-gnu/pkgconfig
> CFLAGS=-m32 LDFLAGS=-m32 ./c
>  * Arch Linux
>  PKG_CONFIG_LIBDIR=/usr/lib32/pkgconfig CFLAGS=-m32 LDFLAGS=-m32
> ./configure
>
> +Kernel modules
> +--------------
> +
> +LTP contains few kernel modules and tests which are using them.
> +These require to be built with the same kernel headers as the running
> kernel (SUT).
> +Sometimes the best way to achieve this is to compile them on the SUT.
> +
> +'modules', 'modules-clean' and 'modules-install' make targets are
> shortcuts
> +to build just these modules and tests.
> +
>  Android Users
>  -------------
>
> diff --git a/Makefile b/Makefile
> index eab40da8a6..933e33ca75 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -212,6 +212,30 @@ endif
>  test-metadata: metadata-all
>         $(MAKE) -C $(abs_srcdir)/metadata test
>
> +MODULE_DIRS := commands/insmod \
>

Here should be:

-MODULE_DIRS := commands/insmod \
+MODULE_DIRS := testcases/commands/insmod \



> +       testcases/kernel/firmware \
> +       testcases/kernel/device-drivers \
> +       testcases/kernel/syscalls/delete_module \
> +       testcases/kernel/syscalls/finit_module \
> +       testcases/kernel/syscalls/init_module
> +
> +.PHONY: modules modules-clean modules-install
> +modules:
> +       @$(foreach dir,$(MODULE_DIRS),\
> +               echo "Build $(dir)";\
> +               $(MAKE) -C $(abs_srcdir)/$(dir); \
> +)
> +modules-clean:
> +       @$(foreach dir,$(MODULE_DIRS),\
> +               echo "Build $(dir)";\
> +               $(MAKE) -C $(abs_srcdir)/$(dir) clean; \
> +)
> +modules-install: modules
> +       @$(foreach dir,$(MODULE_DIRS),\
> +               echo "Build $(dir)";\
> +               $(MAKE) -C $(abs_srcdir)/$(dir) install; \
> +)
> +
>  ## Help
>  .PHONY: help
>  help:
> --
> 2.50.1
>
>
Li Wang Aug. 2, 2025, 3:59 a.m. UTC | #3
On Fri, Aug 1, 2025 at 6:21 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi,
>
> > Changes v1->v2:
> > * Add also modules-clean and modules-install
> > This is needed as 'make modules clean' or 'make modules install'
> > would be running 2 separate targets.
>
> Also, I wondered about fail when kernel modules aren't build:
>
> include/mk/module.mk contains:
>
> ifeq ($(WITH_MODULES),no)
> SKIP := 1
> else
> ifeq ($(LINUX_VERSION_MAJOR)$(LINUX_VERSION_PATCH),)
> SKIP := 1
> else
> SKIP ?= $(shell \
>         [ "$(LINUX_VERSION_MAJOR)" -gt "$(REQ_VERSION_MAJOR)" ] || \
>         [ "$(LINUX_VERSION_MAJOR)" -eq "$(REQ_VERSION_MAJOR)" -a \
>           "$(LINUX_VERSION_PATCH)" -ge "$(REQ_VERSION_PATCH)" ]; echo $$?)
> endif
> endif
>
> ifneq ($(SKIP),0)
> MAKE_TARGETS := $(filter-out %.ko, $(MAKE_TARGETS))
> endif
>
> # Ignoring the exit status of commands is done to be forward compatible
> with
> # kernel internal API changes. The user-space test will return TCONF, if it
> # doesn't find the module (i.e. it wasn't built either due to kernel-devel
> # missing or module build failure).
> %.ko: %.c .dep_modules ;
> -----
>
> We could allow to fail build with:
> $ make modules FORCE=1
>
> with these changes to include/mk/module.mk:
>
> ifneq ($(SKIP),0)
> MAKE_TARGETS := $(filter-out %.ko, $(MAKE_TARGETS))
>


> ifeq ($(FORCE),1)
> $(error Modules not built)
>

This suggestion looks good, maybe we can also add a helpful error
message showing how to fix it:

ifeq ($(FORCE),1)
$(error Kernel modules not built! \
Check that kernel-devel/kernel-headers for
$(LINUX_VERSION_MAJOR).$(LINUX_VERSION_PATCH) are installed, \
and try again. You can build anyway by omitting FORCE=1.)


endif
> endif
>
> ifneq ($(FORCE),1)
> %.ko: %.c .dep_modules ;
> endif
>
> Kind regards,
> Petr
>
>
Li Wang Aug. 2, 2025, 4:11 a.m. UTC | #4
On Sat, Aug 2, 2025 at 11:25 AM Li Wang <liwang@redhat.com> wrote:

>
>
> On Fri, Aug 1, 2025 at 6:09 PM Petr Vorel <pvorel@suse.cz> wrote:
>
>> LTP contains few kernel modules and tests which are using them.  These
>> require to be built with the same kernel headers as the running kernel
>> (SUT). Sometimes the best way to achieve this is to compile them on the
>> SUT.
>>
>> Add 'modules', 'modules-clean' and 'modules-install' make targets to
>> make it easier to build them.
>>
>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>> ---
>> Hi,
>>
>> I'm sorry for the noise.
>>
>> Most of the people will need just modules-install, but let's be
>> consistent with other LTP make targets.
>>
>> Kind regards,
>> Petr
>>
>> Changes v1->v2:
>> * Add also modules-clean and modules-install
>> This is needed as 'make modules clean' or 'make modules install'
>> would be running 2 separate targets.
>>
>> v1:
>> https://lore.kernel.org/ltp/20250801094205.965645-1-pvorel@suse.cz/
>>
>> https://patchwork.ozlabs.org/project/ltp/patch/20250801094205.965645-1-pvorel@suse.cz/
>>
>>  INSTALL  | 10 ++++++++++
>>  Makefile | 24 ++++++++++++++++++++++++
>>  2 files changed, 34 insertions(+)
>>
>> diff --git a/INSTALL b/INSTALL
>> index cbe27f32ea..10c19d4105 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -165,6 +165,16 @@ PKG_CONFIG_LIBDIR=/usr/lib/i386-linux-gnu/pkgconfig
>> CFLAGS=-m32 LDFLAGS=-m32 ./c
>>  * Arch Linux
>>  PKG_CONFIG_LIBDIR=/usr/lib32/pkgconfig CFLAGS=-m32 LDFLAGS=-m32
>> ./configure
>>
>> +Kernel modules
>> +--------------
>> +
>> +LTP contains few kernel modules and tests which are using them.
>> +These require to be built with the same kernel headers as the running
>> kernel (SUT).
>> +Sometimes the best way to achieve this is to compile them on the SUT.
>> +
>> +'modules', 'modules-clean' and 'modules-install' make targets are
>> shortcuts
>> +to build just these modules and tests.
>> +
>>  Android Users
>>  -------------
>>
>> diff --git a/Makefile b/Makefile
>> index eab40da8a6..933e33ca75 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -212,6 +212,30 @@ endif
>>  test-metadata: metadata-all
>>         $(MAKE) -C $(abs_srcdir)/metadata test
>>
>> +MODULE_DIRS := commands/insmod \
>>
>
> Here should be:
>
> -MODULE_DIRS := commands/insmod \
> +MODULE_DIRS := testcases/commands/insmod \
>

Or, if we avoid hard-coding the module dirs, then we don't need
to amend it when adding new tests later.

Something maybe like:

-MODULE_DIRS := commands/insmod \
-       testcases/kernel/firmware \
-       testcases/kernel/device-drivers \
-       testcases/kernel/syscalls/delete_module \
-       testcases/kernel/syscalls/finit_module \
-       testcases/kernel/syscalls/init_module
+MODULE_DIRS := $(shell \
+  find testcases/ -type f -name 'Makefile' | \
+  xargs grep -l 'include.*module\.mk' | \
+  xargs -n1 dirname | \
+  sort -u \
+)


>
>
>> +       testcases/kernel/firmware \
>> +       testcases/kernel/device-drivers \
>> +       testcases/kernel/syscalls/delete_module \
>> +       testcases/kernel/syscalls/finit_module \
>> +       testcases/kernel/syscalls/init_module
>> +
>> +.PHONY: modules modules-clean modules-install
>> +modules:
>> +       @$(foreach dir,$(MODULE_DIRS),\
>> +               echo "Build $(dir)";\
>> +               $(MAKE) -C $(abs_srcdir)/$(dir); \
>> +)
>> +modules-clean:
>> +       @$(foreach dir,$(MODULE_DIRS),\
>> +               echo "Build $(dir)";\
>> +               $(MAKE) -C $(abs_srcdir)/$(dir) clean; \
>> +)
>> +modules-install: modules
>> +       @$(foreach dir,$(MODULE_DIRS),\
>> +               echo "Build $(dir)";\
>> +               $(MAKE) -C $(abs_srcdir)/$(dir) install; \
>> +)
>> +
>>  ## Help
>>  .PHONY: help
>>  help:
>> --
>> 2.50.1
>>
>>
>
> --
> Regards,
> Li Wang
>
Petr Vorel Aug. 6, 2025, 11:47 a.m. UTC | #5
Hi Li, all,

..
> > We could allow to fail build with:
> > $ make modules FORCE=1

> > with these changes to include/mk/module.mk:

> > ifneq ($(SKIP),0)
> > MAKE_TARGETS := $(filter-out %.ko, $(MAKE_TARGETS))

> > ifeq ($(FORCE),1)
> > $(error Modules not built)


> This suggestion looks good, maybe we can also add a helpful error
> message showing how to fix it:

> ifeq ($(FORCE),1)
> $(error Kernel modules not built! \
> Check that kernel-devel/kernel-headers for
> $(LINUX_VERSION_MAJOR).$(LINUX_VERSION_PATCH) are installed, \
> and try again. You can build anyway by omitting FORCE=1.)

+1. I'll probably do this as a separate effort (or at least a separate commit).

Kind regards,
Petr
Petr Vorel Aug. 6, 2025, 11:52 a.m. UTC | #6
Hi Li, all,

> >> +MODULE_DIRS := commands/insmod \

> > Here should be:

> > -MODULE_DIRS := commands/insmod \
> > +MODULE_DIRS := testcases/commands/insmod \


> Or, if we avoid hard-coding the module dirs, then we don't need
> to amend it when adding new tests later.

> Something maybe like:

> -MODULE_DIRS := commands/insmod \
> -       testcases/kernel/firmware \
> -       testcases/kernel/device-drivers \
> -       testcases/kernel/syscalls/delete_module \
> -       testcases/kernel/syscalls/finit_module \
> -       testcases/kernel/syscalls/init_module
> +MODULE_DIRS := $(shell \
> +  find testcases/ -type f -name 'Makefile' | \
> +  xargs grep -l 'include.*module\.mk' | \
> +  xargs -n1 dirname | \
> +  sort -u \
> +)

nit: at least xargs could be avoided.

MODULE_DIRS := $(shell \
	dirname $(grep -l 'include.*module\.mk' $(find testcases/ -type f -name 'Makefile')) | sort -u
)

Although this works, I wonder if we want to depend on a shell detection like
this (I'd appreciate others' opinion).

Kind regards,
Petr

> >> +       testcases/kernel/firmware \
> >> +       testcases/kernel/device-drivers \
> >> +       testcases/kernel/syscalls/delete_module \
> >> +       testcases/kernel/syscalls/finit_module \
> >> +       testcases/kernel/syscalls/init_module
> >> +
> >> +.PHONY: modules modules-clean modules-install
> >> +modules:
> >> +       @$(foreach dir,$(MODULE_DIRS),\
> >> +               echo "Build $(dir)";\
> >> +               $(MAKE) -C $(abs_srcdir)/$(dir); \
> >> +)
> >> +modules-clean:
> >> +       @$(foreach dir,$(MODULE_DIRS),\
> >> +               echo "Build $(dir)";\
> >> +               $(MAKE) -C $(abs_srcdir)/$(dir) clean; \
> >> +)
> >> +modules-install: modules
> >> +       @$(foreach dir,$(MODULE_DIRS),\
> >> +               echo "Build $(dir)";\
> >> +               $(MAKE) -C $(abs_srcdir)/$(dir) install; \
> >> +)
Cyril Hrubis Aug. 6, 2025, 1:29 p.m. UTC | #7
Hi!
> MODULE_DIRS := $(shell \
> 	dirname $(grep -l 'include.*module\.mk' $(find testcases/ -type f -name 'Makefile')) | sort -u
> )
> 
> Although this works, I wonder if we want to depend on a shell detection like
> this (I'd appreciate others' opinion).

Looks okay to me. It's much better than hardcoding the paths, we will
likely forget to update any hardcoded info.
Petr Vorel Aug. 6, 2025, 7:49 p.m. UTC | #8
> Hi!
> > MODULE_DIRS := $(shell \
> > 	dirname $(grep -l 'include.*module\.mk' $(find testcases/ -type f -name 'Makefile')) | sort -u
> > )

> > Although this works, I wonder if we want to depend on a shell detection like
> > this (I'd appreciate others' opinion).

> Looks okay to me. It's much better than hardcoding the paths, we will
> likely forget to update any hardcoded info.

Good, thanks, going to send v3.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/INSTALL b/INSTALL
index cbe27f32ea..10c19d4105 100644
--- a/INSTALL
+++ b/INSTALL
@@ -165,6 +165,16 @@  PKG_CONFIG_LIBDIR=/usr/lib/i386-linux-gnu/pkgconfig CFLAGS=-m32 LDFLAGS=-m32 ./c
 * Arch Linux
 PKG_CONFIG_LIBDIR=/usr/lib32/pkgconfig CFLAGS=-m32 LDFLAGS=-m32 ./configure
 
+Kernel modules
+--------------
+
+LTP contains few kernel modules and tests which are using them.
+These require to be built with the same kernel headers as the running kernel (SUT).
+Sometimes the best way to achieve this is to compile them on the SUT.
+
+'modules', 'modules-clean' and 'modules-install' make targets are shortcuts
+to build just these modules and tests.
+
 Android Users
 -------------
 
diff --git a/Makefile b/Makefile
index eab40da8a6..933e33ca75 100644
--- a/Makefile
+++ b/Makefile
@@ -212,6 +212,30 @@  endif
 test-metadata: metadata-all
 	$(MAKE) -C $(abs_srcdir)/metadata test
 
+MODULE_DIRS := commands/insmod \
+	testcases/kernel/firmware \
+	testcases/kernel/device-drivers \
+	testcases/kernel/syscalls/delete_module \
+	testcases/kernel/syscalls/finit_module \
+	testcases/kernel/syscalls/init_module
+
+.PHONY: modules modules-clean modules-install
+modules:
+	@$(foreach dir,$(MODULE_DIRS),\
+		echo "Build $(dir)";\
+		$(MAKE) -C $(abs_srcdir)/$(dir); \
+)
+modules-clean:
+	@$(foreach dir,$(MODULE_DIRS),\
+		echo "Build $(dir)";\
+		$(MAKE) -C $(abs_srcdir)/$(dir) clean; \
+)
+modules-install: modules
+	@$(foreach dir,$(MODULE_DIRS),\
+		echo "Build $(dir)";\
+		$(MAKE) -C $(abs_srcdir)/$(dir) install; \
+)
+
 ## Help
 .PHONY: help
 help: