Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

[Mickaël Salaün]

Hi,

This change breaks the Landlock test build when performed on the 
tools/testing/selftests/landlock directory because the 
(non-system/up-to-date) kernel headers aren't found. Looking at the use 
of top_srcdir and HDR_PATH, it seems that multiple subsystems are using 
this feature. I consider this change a regression.

It also removes the check on up-to-date kernel headers (thanks to the 
Makefile's target timestamp).

I wasn't CCed for this change impacting Landlock [1]. Please keep in 
mind to add at least maintainers and related mailing-lists for changes 
on related subsystems.

The following patch almost revert commit a917dd94b832 
("selftests/landlock: drop deprecated headers dependency") and partially 
fixes commit 49de12ba06ef ("selftests: drop KSFT_KHDR_INSTALL make target"):

--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
  TEST_GEN_PROGS_EXTENDED := true

  OVERRIDE_TARGETS := 1
+top_srcdir = ../../../..
  include ../lib.mk

+khdr_dir = $(top_srcdir)/usr/include
+
  $(OUTPUT)/true: true.c
  	$(LINK.c) $< $(LDLIBS) -o $@ -static

-$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
-	$(LINK.c) $< $(LDLIBS) -o $@ -lcap
+$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h 
../kselftest_harness.h common.h
+	$(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)


This doesn't fix the header timestamp check though.

Regards,
  Mickaël

[1] 
https://lore.kernel.org/all/b79c51ed97219b1c10e2e3f2bcd3269305f0f035.1657694067.git.guillaume.tucker@collabora.com/


On 02/08/2022 00:03, Shuah Khan wrote:
> Hi Linus,
> 
> Please pull the following Kselftest update for Linux 5.20-rc1.
> 
> This Kselftest update for Linux 5.20-rc1 consists of:
> 
> - timers test build fixes and cleanups for new tool chains
> - removing khdr from kselftest framework and main Makefile
> - changes to test output messages to improve reports
> 
> Please not that this update also included main Makefile change
> to kselftest build logic in it.
> 
> diff is attached.
> 
> thanks,
> -- Shuah
> 
> ----------------------------------------------------------------
> The following changes since commit 03c765b0e3b4cb5063276b086c76f7a612856a9a:
> 
>     Linux 5.19-rc4 (2022-06-26 14:22:10 -0700)
> 
> are available in the Git repository at:
> 
>     git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest tags/linux-kselftest-next-5.20-rc1
> 
> for you to fetch changes up to 4062eba9f3d072e72645860fbc5d160428a75c50:
> 
>     Makefile: replace headers_install with headers for kselftest (2022-07-26 18:06:33 -0600)
> 
> ----------------------------------------------------------------
> linux-kselftest-next-5.20-rc1
> 
> This Kselftest update for Linux 5.20-rc1 consists of:
> 
> - timers test build fixes and cleanups for new tool chains
> - removing khdr from kselftest framework and main Makefile
> - changes to test output messages to improve reports
> 
> ----------------------------------------------------------------
> Gautam (2):
>         kselftests: Enable the echo command to print newlines in Makefile
>         kselftests/damon: add support for cases where debugfs cannot be read
> 
> Gautam Menghani (3):
>         selftests: Make the usage formatting consistent in kselftest_deps.sh
>         selftests/drivers/gpu: Add error messages to drm_mm.sh
>         selftests/kcmp: Make the test output consistent and clear
> 
> Guillaume Tucker (6):
>         selftests: drop khdr make target
>         selftests: stop using KSFT_KHDR_INSTALL
>         selftests: drop KSFT_KHDR_INSTALL make target
>         Makefile: add headers_install to kselftest targets
>         selftests/landlock: drop deprecated headers dependency
>         Makefile: replace headers_install with headers for kselftest
> 
> Johannes Holland (1):
>         selftests/tpm2: increase timeout for kselftests
> 
> Soumya Negi (1):
>         selftests: drivers/dma-buf: Improve message in selftest summary
> 
> Wolfram Sang (9):
>         selftests: timers: valid-adjtimex: build fix for newer toolchains
>         selftests: timers: fix declarations of main()
>         selftests: timers: nanosleep: adapt to kselftest framework
>         selftests: timers: inconsistency-check: adapt to kselftest framework
>         selftests: timers: clocksource-switch: fix passing errors from child
>         selftests: timers: clocksource-switch: sort includes
>         selftests: timers: clocksource-switch: add command line switch to skip sanity check
>         selftests: timers: clocksource-switch: add 'runtime' command line parameter
>         selftests: timers: clocksource-switch: adapt to kselftest framework
> 
> Xiang wangx (1):
>         userfaultfd/selftests: Fix typo in comment
> 
> Zan Aziz (1):
>         selftests:timers: globals don't need initialization to 0
> 
>    Makefile                                           |  4 +-
>    tools/testing/selftests/Makefile                   | 30 +--------
>    tools/testing/selftests/arm64/mte/Makefile         |  1 -
>    tools/testing/selftests/arm64/signal/Makefile      |  1 -
>    .../testing/selftests/arm64/signal/test_signals.h  |  4 +-
>    tools/testing/selftests/damon/_chk_dependency.sh   | 10 +++
>    tools/testing/selftests/drivers/dma-buf/udmabuf.c  |  3 +-
>    tools/testing/selftests/drivers/gpu/drm_mm.sh      |  4 +-
>    .../selftests/drivers/s390x/uvdevice/Makefile      |  1 -
>    tools/testing/selftests/futex/functional/Makefile  |  1 -
>    tools/testing/selftests/kcmp/kcmp_test.c           |  6 +-
>    tools/testing/selftests/kselftest_deps.sh          |  2 +-
>    tools/testing/selftests/kvm/Makefile               |  1 -
>    tools/testing/selftests/landlock/Makefile          | 10 +--
>    tools/testing/selftests/lib.mk                     | 38 ------------
>    tools/testing/selftests/net/Makefile               |  1 -
>    tools/testing/selftests/net/mptcp/Makefile         |  1 -
>    tools/testing/selftests/tc-testing/Makefile        |  1 -
>    tools/testing/selftests/timers/adjtick.c           |  2 +-
>    .../testing/selftests/timers/alarmtimer-suspend.c  |  2 +-
>    tools/testing/selftests/timers/change_skew.c       |  2 +-
>    .../testing/selftests/timers/clocksource-switch.c  | 71 ++++++++++++++--------
>    .../testing/selftests/timers/inconsistency-check.c | 32 +++++-----
>    tools/testing/selftests/timers/nanosleep.c         | 18 +++---
>    tools/testing/selftests/timers/raw_skew.c          |  2 +-
>    tools/testing/selftests/timers/skew_consistency.c  |  2 +-
>    tools/testing/selftests/timers/valid-adjtimex.c    |  2 +-
>    tools/testing/selftests/tpm2/settings              |  1 +
>    tools/testing/selftests/vm/Makefile                |  1 -
>    tools/testing/selftests/vm/userfaultfd.c           |  2 +-
>    30 files changed, 111 insertions(+), 145 deletions(-)
>    create mode 100644 tools/testing/selftests/tpm2/settings
> ----------------------------------------------------------------

Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

[Shuah Khan]

On 8/2/22 3:51 AM, Mickaël Salaün wrote:
> Hi,
> 
> This change breaks the Landlock test build when performed on the tools/testing/selftests/landlock directory because the (non-system/up-to-date) kernel headers aren't found. Looking at the use of top_srcdir and HDR_PATH, it seems that multiple subsystems are using this feature. I consider this change a regression.
> 

I did a build test from the top level before sending the pull request
and didn't catch this breakage as a result. This breaks when build is
run from the test directory.

We have had several problems related to khdr_dir and target and decided
to move away from it with this change.

> It also removes the check on up-to-date kernel headers (thanks to the Makefile's target timestamp).
> 
> I wasn't CCed for this change impacting Landlock [1]. Please keep in mind to add at least maintainers and related mailing-lists for changes on related subsystems.
> 

That is the usual practice and if I notice missing maintainers, I add
them. We missed this one.

> The following patch almost revert commit a917dd94b832 ("selftests/landlock: drop deprecated headers dependency") and partially fixes commit 49de12ba06ef ("selftests: drop KSFT_KHDR_INSTALL make target"):
> 

Guillaume,

Will you be able to look at this and send a patch on top? I will
send another pull request before merge window closes?

> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>   TEST_GEN_PROGS_EXTENDED := true
> 
>   OVERRIDE_TARGETS := 1
> +top_srcdir = ../../../..
>   include ../lib.mk
> 
> +khdr_dir = $(top_srcdir)/usr/include
> +
>   $(OUTPUT)/true: true.c
>       $(LINK.c) $< $(LDLIBS) -o $@ -static
> 
> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
> -    $(LINK.c) $< $(LDLIBS) -o $@ -lcap
> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
> +    $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
> 
> 
> This doesn't fix the header timestamp check though.
> 

thanks,
-- Shuah

Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

[Guillaume Tucker]

On 02/08/2022 15:29, Shuah Khan wrote:
> On 8/2/22 3:51 AM, Mickaël Salaün wrote:
>> Hi,
>>
>> This change breaks the Landlock test build when performed on the tools/testing/selftests/landlock directory because the (non-system/up-to-date) kernel headers aren't found. Looking at the use of top_srcdir and HDR_PATH, it seems that multiple subsystems are using this feature. I consider this change a regression.
>>
> 
> I did a build test from the top level before sending the pull request
> and didn't catch this breakage as a result. This breaks when build is
> run from the test directory.
> 
> We have had several problems related to khdr_dir and target and decided
> to move away from it with this change.
> 
>> It also removes the check on up-to-date kernel headers (thanks to the Makefile's target timestamp).
>>
>> I wasn't CCed for this change impacting Landlock [1]. Please keep in mind to add at least maintainers and related mailing-lists for changes on related subsystems.
>>
> 
> That is the usual practice and if I notice missing maintainers, I add
> them. We missed this one.
> 
>> The following patch almost revert commit a917dd94b832 ("selftests/landlock: drop deprecated headers dependency") and partially fixes commit 49de12ba06ef ("selftests: drop KSFT_KHDR_INSTALL make target"):
>>
> 
> Guillaume,
> 
> Will you be able to look at this and send a patch on top? I will
> send another pull request before merge window closes?

Sure, I'll take a look today.

Also I'll see if we can add some extra build tests in KernelCI
for the kselftest tree to catch issues like these automatically.

Best wishes,
Guillaume

>> --- a/tools/testing/selftests/landlock/Makefile
>> +++ b/tools/testing/selftests/landlock/Makefile
>> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>>   TEST_GEN_PROGS_EXTENDED := true
>>
>>   OVERRIDE_TARGETS := 1
>> +top_srcdir = ../../../..
>>   include ../lib.mk
>>
>> +khdr_dir = $(top_srcdir)/usr/include
>> +
>>   $(OUTPUT)/true: true.c
>>       $(LINK.c) $< $(LDLIBS) -o $@ -static
>>
>> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
>> -    $(LINK.c) $< $(LDLIBS) -o $@ -lcap
>> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
>> +    $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
>>
>>
>> This doesn't fix the header timestamp check though.
>>
> 
> thanks,
> -- Shuah

Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

[Shuah Khan]

On 8/2/22 10:44 PM, Guillaume Tucker wrote:
> On 02/08/2022 15:29, Shuah Khan wrote:
>> On 8/2/22 3:51 AM, Mickaël Salaün wrote:
>>> Hi,
>>>
>>> This change breaks the Landlock test build when performed on the tools/testing/selftests/landlock directory because the (non-system/up-to-date) kernel headers aren't found. Looking at the use of top_srcdir and HDR_PATH, it seems that multiple subsystems are using this feature. I consider this change a regression.
>>>
>>
>> I did a build test from the top level before sending the pull request
>> and didn't catch this breakage as a result. This breaks when build is
>> run from the test directory.
>>
>> We have had several problems related to khdr_dir and target and decided
>> to move away from it with this change.
>>
>>> It also removes the check on up-to-date kernel headers (thanks to the Makefile's target timestamp).
>>>
>>> I wasn't CCed for this change impacting Landlock [1]. Please keep in mind to add at least maintainers and related mailing-lists for changes on related subsystems.
>>>
>>
>> That is the usual practice and if I notice missing maintainers, I add
>> them. We missed this one.
>>
>>> The following patch almost revert commit a917dd94b832 ("selftests/landlock: drop deprecated headers dependency") and partially fixes commit 49de12ba06ef ("selftests: drop KSFT_KHDR_INSTALL make target"):
>>>
>>
>> Guillaume,
>>
>> Will you be able to look at this and send a patch on top? I will
>> send another pull request before merge window closes?
> 
> Sure, I'll take a look today.
> 

Thank you.

> Also I'll see if we can add some extra build tests in KernelCI
> for the kselftest tree to catch issues like these automatically.
> 

Yes. Adding tests to catch these automatically will be great.
Please refer to use-cases for kselftest for tips on tests to
add. Beig able to build from individual test directory is one
of the use-cases.

thanks,
-- Shuah

Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

[Guillaume Tucker]

On 03/08/2022 16:32, Shuah Khan wrote:
> On 8/2/22 10:44 PM, Guillaume Tucker wrote:
>> On 02/08/2022 15:29, Shuah Khan wrote:
>>> On 8/2/22 3:51 AM, Mickaël Salaün wrote:
>>>> Hi,
>>>>
>>>> This change breaks the Landlock test build when performed on the tools/testing/selftests/landlock directory because the (non-system/up-to-date) kernel headers aren't found. Looking at the use of top_srcdir and HDR_PATH, it seems that multiple subsystems are using this feature. I consider this change a regression.
>>>>
>>>
>>> I did a build test from the top level before sending the pull request
>>> and didn't catch this breakage as a result. This breaks when build is
>>> run from the test directory.
>>>
>>> We have had several problems related to khdr_dir and target and decided
>>> to move away from it with this change.
>>>
>>>> It also removes the check on up-to-date kernel headers (thanks to the Makefile's target timestamp).
>>>>
>>>> I wasn't CCed for this change impacting Landlock [1]. Please keep in mind to add at least maintainers and related mailing-lists for changes on related subsystems.
>>>>
>>>
>>> That is the usual practice and if I notice missing maintainers, I add
>>> them. We missed this one.
>>>
>>>> The following patch almost revert commit a917dd94b832 ("selftests/landlock: drop deprecated headers dependency") and partially fixes commit 49de12ba06ef ("selftests: drop KSFT_KHDR_INSTALL make target"):
>>>>
>>>
>>> Guillaume,
>>>
>>> Will you be able to look at this and send a patch on top? I will
>>> send another pull request before merge window closes?
>>
>> Sure, I'll take a look today.
>>
> 
> Thank you.

OK I just sent "selftests/landlock: fix broken include of linux/landlock.h"

This does fix the build when doing:

  make -C tools/testing/selftests/landlock

However I've also noticed the landlock test is failing to build
when make is invoked from the top-level directory and using a
sub-directory for the build output, in other words my earlier
patches didn't fix the build for this test, but that's not a
regression.

I'll see if that can be fixed too while also not breaking
the "-C" sub-make build.

>> Also I'll see if we can add some extra build tests in KernelCI
>> for the kselftest tree to catch issues like these automatically.
>>
> 
> Yes. Adding tests to catch these automatically will be great.
> Please refer to use-cases for kselftest for tips on tests to
> add. Beig able to build from individual test directory is one
> of the use-cases.

Great.  Well I shall try and get that set up before making further changes ;)

Best wishes,
Guillaume


P.S. The output of gen_tar is showing "-ne " on every line, is that expected?
     For example: -ne Emit Tests for alsa

Re: [GIT PULL] Kselftest update for Linux 5.20-rc1

[Shuah Khan]

On 8/3/22 2:22 PM, Guillaume Tucker wrote:
> On 03/08/2022 16:32, Shuah Khan wrote:
>> On 8/2/22 10:44 PM, Guillaume Tucker wrote:
>>> On 02/08/2022 15:29, Shuah Khan wrote:
>>>> On 8/2/22 3:51 AM, Mickaël Salaün wrote:
>>>>> Hi,
>>>>>
>>>>> This change breaks the Landlock test build when performed on the tools/testing/selftests/landlock directory because the (non-system/up-to-date) kernel headers aren't found. Looking at the use of top_srcdir and HDR_PATH, it seems that multiple subsystems are using this feature. I consider this change a regression.
>>>>>
>>>>
>>>> I did a build test from the top level before sending the pull request
>>>> and didn't catch this breakage as a result. This breaks when build is
>>>> run from the test directory.
>>>>
>>>> We have had several problems related to khdr_dir and target and decided
>>>> to move away from it with this change.
>>>>
>>>>> It also removes the check on up-to-date kernel headers (thanks to the Makefile's target timestamp).
>>>>>
>>>>> I wasn't CCed for this change impacting Landlock [1]. Please keep in mind to add at least maintainers and related mailing-lists for changes on related subsystems.
>>>>>
>>>>
>>>> That is the usual practice and if I notice missing maintainers, I add
>>>> them. We missed this one.
>>>>
>>>>> The following patch almost revert commit a917dd94b832 ("selftests/landlock: drop deprecated headers dependency") and partially fixes commit 49de12ba06ef ("selftests: drop KSFT_KHDR_INSTALL make target"):
>>>>>
>>>>
>>>> Guillaume,
>>>>
>>>> Will you be able to look at this and send a patch on top? I will
>>>> send another pull request before merge window closes?
>>>
>>> Sure, I'll take a look today.
>>>
>>
>> Thank you.
> 
> OK I just sent "selftests/landlock: fix broken include of linux/landlock.h"
> 
> This does fix the build when doing:
> 
>    make -C tools/testing/selftests/landlock
> 

Thank you for fixing this quickly.

> However I've also noticed the landlock test is failing to build
> when make is invoked from the top-level directory and using a
> sub-directory for the build output, in other words my earlier
> patches didn't fix the build for this test, but that's not a
> regression.
> 

Okay.

> I'll see if that can be fixed too while also not breaking
> the "-C" sub-make build.
> 

Sounds good. Supporting all these use-cases makes it a bit hard.

>>> Also I'll see if we can add some extra build tests in KernelCI
>>> for the kselftest tree to catch issues like these automatically.
>>>
>>

> Great.  Well I shall try and get that set up before making further changes ;)
> 

Thanks.

> 
> P.S. The output of gen_tar is showing "-ne " on every line, is that expected?
>       For example: -ne Emit Tests for alsa
> 

Hmm. I will try and let you. I haven't used this one in a bit.

thanks,
-- Shuah