diff mbox series

[2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling

Message ID 20240215-lp-selftests-fixes-v1-2-89f4a6f5cddc@suse.com (mailing list archive)
State New
Headers show
Series Improvements to livepatch kselftests on top of kselftest-next | expand

Commit Message

Marcos Paulo de Souza Feb. 15, 2024, 6:12 p.m. UTC 
We don't need new targets only to run two make modules and make clean.
We can test if TEST_GEN_MODS_DIR is specified, and then run the
commands.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/lib.mk | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

kernel test robot Feb. 19, 2024, 7:16 a.m. UTC | #1 
Hi Marcos,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 345e8abe4c355bc24bab3f4a5634122e55be8665]

url:    https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/selftests-lib-mk-Do-not-process-TEST_GEN_MODS_DIR/20240216-021601
base:   345e8abe4c355bc24bab3f4a5634122e55be8665
patch link:    https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-2-89f4a6f5cddc%40suse.com
patch subject: [PATCH 2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240219/202402191502.dALlSRz0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402191502.dALlSRz0-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Makefile:11: warning: overriding recipe for target 'all'
>> ../lib.mk:62: warning: ignoring old recipe for target 'all'
   make[1]: *** [../lib.mk:62: all] Error 2
   Makefile:65: warning: overriding recipe for target 'emit_tests'
   ../lib.mk:120: warning: ignoring old recipe for target 'emit_tests'
   make[1]: *** No targets.  Stop.
   make[1]: *** No targets.  Stop.
   make[1]: *** No targets.  Stop.


vim +/all +11 Makefile

^1da177e4c3f41 Linus Torvalds 2005-04-16   7  
^1da177e4c3f41 Linus Torvalds 2005-04-16   8  # *DOCUMENTATION*
^1da177e4c3f41 Linus Torvalds 2005-04-16   9  # To see a list of typical targets execute "make help"
^1da177e4c3f41 Linus Torvalds 2005-04-16  10  # More info can be located in ./README
^1da177e4c3f41 Linus Torvalds 2005-04-16 @11  # Comments in this file are targeted only to the developer, do not
^1da177e4c3f41 Linus Torvalds 2005-04-16  12  # expect to learn how to build the kernel reading this file.
^1da177e4c3f41 Linus Torvalds 2005-04-16  13
Marcos Paulo de Souza Feb. 21, 2024, 12:29 p.m. UTC | #2 
On Mon, 19 Feb 2024 15:16:51 +0800 kernel test robot <lkp@intel.com> wrote:

> Hi Marcos,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on 345e8abe4c355bc24bab3f4a5634122e55be8665]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/selftests-lib-mk-Do-not-process-TEST_GEN_MODS_DIR/20240216-021601
> base:   345e8abe4c355bc24bab3f4a5634122e55be8665
> patch link:    https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-2-89f4a6f5cddc%40suse.com
> patch subject: [PATCH 2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240219/202402191502.dALlSRz0-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202402191502.dALlSRz0-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> Makefile:11: warning: overriding recipe for target 'all'
> >> ../lib.mk:62: warning: ignoring old recipe for target 'all'
>    make[1]: *** [../lib.mk:62: all] Error 2
>    Makefile:65: warning: overriding recipe for target 'emit_tests'
>    ../lib.mk:120: warning: ignoring old recipe for target 'emit_tests'
>    make[1]: *** No targets.  Stop.
>    make[1]: *** No targets.  Stop.
>    make[1]: *** No targets.  Stop.

I couldn't reproduce this one locally. Shuah, have you seen this issue in your
setup? I followed the steps to reproduce this issue (it's the same sequence
already reported in earlier patches...)

> 
> 
> vim +/all +11 Makefile
> 
> ^1da177e4c3f41 Linus Torvalds 2005-04-16   7  
> ^1da177e4c3f41 Linus Torvalds 2005-04-16   8  # *DOCUMENTATION*
> ^1da177e4c3f41 Linus Torvalds 2005-04-16   9  # To see a list of typical targets execute "make help"
> ^1da177e4c3f41 Linus Torvalds 2005-04-16  10  # More info can be located in ./README
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 @11  # Comments in this file are targeted only to the developer, do not
> ^1da177e4c3f41 Linus Torvalds 2005-04-16  12  # expect to learn how to build the kernel reading this file.
> ^1da177e4c3f41 Linus Torvalds 2005-04-16  13  
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Yujie Liu Feb. 23, 2024, 3:40 a.m. UTC | #3 
On Wed, Feb 21, 2024 at 09:29:47AM -0300, Marcos Paulo de Souza wrote:
> On Mon, 19 Feb 2024 15:16:51 +0800 kernel test robot <lkp@intel.com> wrote:
> 
> > Hi Marcos,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on 345e8abe4c355bc24bab3f4a5634122e55be8665]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/selftests-lib-mk-Do-not-process-TEST_GEN_MODS_DIR/20240216-021601
> > base:   345e8abe4c355bc24bab3f4a5634122e55be8665
> > patch link:    https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-2-89f4a6f5cddc%40suse.com
> > patch subject: [PATCH 2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling
> > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240219/202402191502.dALlSRz0-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202402191502.dALlSRz0-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > >> Makefile:11: warning: overriding recipe for target 'all'
> > >> ../lib.mk:62: warning: ignoring old recipe for target 'all'
> 
> I couldn't reproduce this one locally. Shuah, have you seen this issue in your
> setup? I followed the steps to reproduce this issue (it's the same sequence
> already reported in earlier patches...)

Hi Marcos,

This seems to be a warning when compiling futex selftest.

linux/tools/testing/selftests/futex$ make
Makefile:11: warning: overriding recipe for target 'all'
../lib.mk:62: warning: ignoring old recipe for target 'all'

There is no such warning if this patch is not applied. Looks like it is
due to the "all" target is defined in lib.mk but overridden in futex
Makefile? Could you please help take a look?

Thanks,
Yujie
Marcos Paulo de Souza Feb. 23, 2024, 4:12 p.m. UTC | #4 
On Fri, 23 Feb 2024 11:40:00 +0800 Yujie Liu <yujie.liu@intel.com> wrote:

> On Wed, Feb 21, 2024 at 09:29:47AM -0300, Marcos Paulo de Souza wrote:
> > On Mon, 19 Feb 2024 15:16:51 +0800 kernel test robot <lkp@intel.com> wrote:
> > 
> > > Hi Marcos,
> > > 
> > > kernel test robot noticed the following build warnings:
> > > 
> > > [auto build test WARNING on 345e8abe4c355bc24bab3f4a5634122e55be8665]
> > > 
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/selftests-lib-mk-Do-not-process-TEST_GEN_MODS_DIR/20240216-021601
> > > base:   345e8abe4c355bc24bab3f4a5634122e55be8665
> > > patch link:    https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-2-89f4a6f5cddc%40suse.com
> > > patch subject: [PATCH 2/3] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling
> > > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240219/202402191502.dALlSRz0-lkp@intel.com/reproduce)
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202402191502.dALlSRz0-lkp@intel.com/
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> Makefile:11: warning: overriding recipe for target 'all'
> > > >> ../lib.mk:62: warning: ignoring old recipe for target 'all'
> > 
> > I couldn't reproduce this one locally. Shuah, have you seen this issue in your
> > setup? I followed the steps to reproduce this issue (it's the same sequence
> > already reported in earlier patches...)
> 
> Hi Marcos,
> 
> This seems to be a warning when compiling futex selftest.
> 
> linux/tools/testing/selftests/futex$ make
> Makefile:11: warning: overriding recipe for target 'all'
> ../lib.mk:62: warning: ignoring old recipe for target 'all'
> 
> There is no such warning if this patch is not applied. Looks like it is
> due to the "all" target is defined in lib.mk but overridden in futex
> Makefile? Could you please help take a look?

I believe that I understood what's going on:

Before this patch, the 'all' from lib.mk target didn't have a "recipe" only
dependencies. The TARGET_GEN_MODS_DIR variable was used while resolving the
dependencies from it.

The proposed patch simplified the process by removing the gen_mods_dir target,
and checked TARGET_GEN_MODS_DIR variable as a recipe.

Per my local tests, we can have two targets with the same name on two different
Makefiles (one that includes in the other in case), as long as only one of them
have a recipe (commands to execute). The dependencies of the target
on the included file would will be checked and executed either way.

But, if both targets have commands to execute, make will say the target was
overridden. In both cases, only the target from the file that includes the other
will execute. I believe this matches the current expectation of the futex
selftests, as they jump on into "functional" directory and execute the tests
there. That makefile also includes lib.mk...

It seems that planned to include more directories since they introduced the
selftests, but never did:


  commit 2aa8470f02a9b9e6a410d1264fe6c8fa6c402eff
  Author: Darren Hart <dvhart@linux.intel.com>
  Date:   Tue May 12 21:07:52 2015 -0700

      selftests: Add futex functional tests

      The futextest testsuite [1] provides functional, stress, and
      performance tests for the various futex op codes. Those tests will be of
      more use to futex developers if they are included with the kernel
      source.

      Copy the core infrastructure and the functional tests into selftests,
      but adapt them for inclusion in the kernel:

      - Update the Makefile to include the run_tests target, remove reference
        to the performance and stress tests from the contributed sources.
      - Replace my dead IBM email address with my current Intel email address.
      - Remove the warrantee and write-to paragraphs from the license blurbs.
      - Remove the NAME section as the filename is easily determined. ;-)
      - Make the whitespace usage consistent in a couple of places.
      - Cleanup various CodingStyle violations.

      A future effort will explore moving the performance and stress tests
      into the kernel.

      1. http://git.kernel.org/cgit/linux/kernel/git/dvhart/futextest.git

Either way, if my change adds a new warning, I think that we can drop that patch
and move on. OTOH, I believe that futex selftests could be simplified in order
to remove this awkward setup to run their tests.

> 
> Thanks,
> Yujie
diff mbox series

Patch

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 286ce0ee102b..eddcd4a849dc 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -58,8 +58,9 @@  TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
 TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
 TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
 
-all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \
-	$(if $(TEST_GEN_MODS_DIR),gen_mods_dir)
+all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
+	$(if $(TEST_GEN_MODS_DIR), \
+		$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR))
 
 define RUN_TESTS
 	BASE_DIR="$(selfdir)";			\
@@ -85,11 +86,6 @@  else
 	@$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS))
 endif
 
-gen_mods_dir:
-	$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR)
-
-clean_mods_dir:
-	$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean
 
 define INSTALL_SINGLE_RULE
 	$(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH))
@@ -133,9 +129,11 @@  endif
 
 define CLEAN
 	$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN)
+	$(if $(TEST_GEN_MODS_DIR), \
+		$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean)
 endef
 
-clean: $(if $(TEST_GEN_MODS_DIR),clean_mods_dir)
+clean:
 	$(CLEAN)
 
 # Enables to extend CFLAGS and LDFLAGS from command line, e.g.
@@ -166,4 +164,4 @@  $(OUTPUT)/%:%.S
 	$(LINK.S) $^ $(LDLIBS) -o $@
 endif
 
-.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir
+.PHONY: run_tests all clean install emit_tests