On 6/6/23 12:16 PM, John Hubbard wrote:

uffd_minor_feature() was unused. Remove it in order to fix the associated clang build warning.

Reviewed-by: David Hildenbrand david@redhat.com Reviewed-by: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/uffd-stress.c | 10 ---------- 1 file changed, 10 deletions(-)

diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index f1ad9eef1c3a..995ff13e74c7 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -88,16 +88,6 @@ static void uffd_stats_reset(struct uffd_args *args, unsigned long n_cpus) } } -static inline uint64_t uffd_minor_feature(void) -{

• if (test_type == TEST_HUGETLB && map_shared)

• return UFFD_FEATURE_MINOR_HUGETLBFS;
  

• else if (test_type == TEST_SHMEM)

• return UFFD_FEATURE_MINOR_SHMEM;
  

• else

• return 0;
  

-}

static void *locking_thread(void *arg) { unsigned long cpu = (unsigned long) arg;

On 6/6/23 12:16 PM, John Hubbard wrote:

Dummy variables are required in order to make these two (similar) routines work, so in both cases, declare the variables as volatile in order to avoid the clang compiler warning.

Furthermore, in order to ensure that each test actually does what is intended, add an asm volatile invocation (thanks to David Hildenbrand for the suggestion), with a clarifying comment so that it survives future maintenance.

Reviewed-by: David Hildenbrand david@redhat.com Reviewed-by: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/hugetlb-madvise.c | 8 ++++++-- tools/testing/selftests/mm/migration.c | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c index 28426e30d9bc..d55322df4b73 100644 --- a/tools/testing/selftests/mm/hugetlb-madvise.c +++ b/tools/testing/selftests/mm/hugetlb-madvise.c @@ -65,11 +65,15 @@ void write_fault_pages(void *addr, unsigned long nr_pages) void read_fault_pages(void *addr, unsigned long nr_pages) {

• unsigned long dummy = 0;

• volatile unsigned long dummy = 0; unsigned long i;

• for (i = 0; i < nr_pages; i++)

• for (i = 0; i < nr_pages; i++) { dummy += *((unsigned long *)(addr + (i * huge_page_size)));

• /* Prevent the compiler from optimizing out the entire loop: */
  

• asm volatile("" : "+r" (dummy));
  

• }

} int main(int argc, char **argv) diff --git a/tools/testing/selftests/mm/migration.c b/tools/testing/selftests/mm/migration.c index 1cec8425e3ca..379581567f27 100644 --- a/tools/testing/selftests/mm/migration.c +++ b/tools/testing/selftests/mm/migration.c @@ -95,12 +95,15 @@ int migrate(uint64_t *ptr, int n1, int n2) void *access_mem(void *ptr) {

• uint64_t y = 0;

• volatile uint64_t y = 0; volatile uint64_t *x = ptr;

while (1) { pthread_testcancel(); y += *x;

• /* Prevent the compiler from optimizing out the writes to y: */
  

• asm volatile("" : "+r" (y));
  

  }

return NULL;

On 6/6/23 12:16 PM, John Hubbard wrote:

The stop variable is a char*, and the code was assigning a char value to it. This was generating a warning when compiling with clang.

However, as both David and Peter pointed out, stop is not even used after the problematic assignment to a char type. So just delete that line entirely.

Reviewed-by: David Hildenbrand david@redhat.com Reviewed-by: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/mlock2-tests.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c index 11b2301f3aa3..80cddc0de206 100644 --- a/tools/testing/selftests/mm/mlock2-tests.c +++ b/tools/testing/selftests/mm/mlock2-tests.c @@ -50,7 +50,6 @@ static int get_vm_area(unsigned long addr, struct vm_boundaries *area) printf("cannot parse /proc/self/maps\n"); goto out; }

• stop = '\0';
  

sscanf(line, "%lx", &start); sscanf(end_addr, "%lx", &end);

On 6/6/23 12:16 PM, John Hubbard wrote:

We cannot depend upon git to reliably retain the executable bit on shell scripts, or so I was told several years ago while working on this same run_vmtests.sh script. And sure enough, things such as test_hmm.sh are lately failing to run, due to lacking execute permissions.

Fix this by explicitly adding "bash" to each of the shell script invocations. Leave fixing the overall approach to another day.

Acked-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/run_vmtests.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 4893eb60d96d..8f81432e4bac 100644 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -242,18 +242,18 @@ if [ $VADDR64 -ne 0 ]; then if [ "$ARCH" == "$ARCH_ARM64" ]; then echo 6 > /proc/sys/vm/nr_hugepages fi

• CATEGORY="hugevm" run_test ./va_high_addr_switch.sh

• CATEGORY="hugevm" run_test bash ./va_high_addr_switch.sh if [ "$ARCH" == "$ARCH_ARM64" ]; then echo $prev_nr_hugepages > /proc/sys/vm/nr_hugepages fi

fi # VADDR64 # vmalloc stability smoke test -CATEGORY="vmalloc" run_test ./test_vmalloc.sh smoke +CATEGORY="vmalloc" run_test bash ./test_vmalloc.sh smoke CATEGORY="mremap" run_test ./mremap_dontunmap -CATEGORY="hmm" run_test ./test_hmm.sh smoke +CATEGORY="hmm" run_test bash ./test_hmm.sh smoke # MADV_POPULATE_READ and MADV_POPULATE_WRITE tests CATEGORY="madv_populate" run_test ./madv_populate

On 6/6/23 12:16 PM, John Hubbard wrote:

These new build products were left out of .gitignore, so add them now.

I'd added to the instructions that object files should be added to .gitignore. But still sometimes people forget.

Reviewed-by: David Hildenbrand david@redhat.com Reviewed-by: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/.gitignore | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 8917455f4f51..ab215303d8e9 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -39,3 +39,5 @@ local_config.h local_config.mk ksm_functional_tests mdwe_test +mkdirty +va_high_addr_switch \ No newline at end of file

On 6/6/23 12:16 PM, John Hubbard wrote:

The uffd tests generate two compile time warnings from clang's -Wformat-security setting. These trigger at the call sites for uffd_test_start() and uffd_test_skip().

1. Fix the uffd_test_start() issue by removing the intermediate

test_name variable (thanks to David Hildenbrand for showing how to do this).

2. Fix the uffd_test_skip() issue by observing that there is no need for

a macro and a variable args approach, because all callers of uffd_test_skip() pass in a simple char* string, without any format specifiers. So just change uffd_test_skip() into a regular C function.

Reviewed-by: David Hildenbrand david@redhat.com Reviewed-by: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/uffd-unit-tests.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c b/tools/testing/selftests/mm/uffd-unit-tests.c index 269c86768a02..04d91f144d1c 100644 --- a/tools/testing/selftests/mm/uffd-unit-tests.c +++ b/tools/testing/selftests/mm/uffd-unit-tests.c @@ -109,12 +109,11 @@ static void uffd_test_pass(void) ksft_inc_fail_cnt(); \ } while (0) -#define uffd_test_skip(...) do { \

• printf("skipped [reason: ");	\
  

• printf(__VA_ARGS__);		\
  

• printf("]\n");			\
  

• ksft_inc_xskip_cnt();		\
  

• } while (0)

+static void uffd_test_skip(const char *message) +{

• printf("skipped [reason: %s]\n", message);
• ksft_inc_xskip_cnt();

+} /*

• Returns 1 if specific userfaultfd supported, 0 otherwise. Note, we'll

@@ -1149,7 +1148,6 @@ int main(int argc, char *argv[]) uffd_test_case_t *test; mem_type_t *mem_type; uffd_test_args_t args;

• char test_name[128]; const char *errmsg; int has_uffd, opt; int i, j;

@@ -1192,10 +1190,8 @@ int main(int argc, char *argv[]) mem_type = &mem_types[j]; if (!(test->mem_targets & mem_type->mem_flag)) continue;

• 	snprintf(test_name, sizeof(test_name),
  

• 		 "%s on %s", test->name, mem_type->name);
  

• 	uffd_test_start(test_name);
  

• 	uffd_test_start("%s on %s", test->name, mem_type->name);
  if (!uffd_feature_supported(test)) {
  	uffd_test_skip("feature missing");
  	continue;
  

On 6/6/23 12:16 PM, John Hubbard wrote:

This fixes a real bug, too, because xstate_size() was assuming that the stack variable xstate_size was initialized to zero. That's not guaranteed nor even especially likely.

Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/pkey-x86.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/pkey-x86.h b/tools/testing/selftests/mm/pkey-x86.h index 72c14cd3ddc7..e32ae8a1cd99 100644 --- a/tools/testing/selftests/mm/pkey-x86.h +++ b/tools/testing/selftests/mm/pkey-x86.h @@ -132,7 +132,7 @@ int pkey_reg_xstate_offset(void) unsigned int ecx; unsigned int edx; int xstate_offset;

• int xstate_size;

• int xstate_size = 0; unsigned long XSTATE_CPUID = 0xd; int leaf;

On 6/6/23 12:16 PM, John Hubbard wrote:

MADV_PAGEOUT, MADV_POPULATE_READ, MADV_COLLAPSE are conditionally defined as necessary. However, that was being done in .c files, and a new build failure came up that would have been automatically avoided had these been in a common header file.

So consolidate and move them all to vm_util.h, which fixes the build failure.

An alternative approach from Muhammad Usama Anjum was: rely on "make headers" being required, and include asm-generic/mman-common.h. This works in the sense that it builds, but it still generates warnings about duplicate MADV_* symbols, and the goal here is to get a fully clean (no warnings) build here.

Reviewed-by: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/cow.c | 7 ------- tools/testing/selftests/mm/khugepaged.c | 10 ---------- tools/testing/selftests/mm/vm_util.h | 10 ++++++++++ 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index dc9d6fe86028..8882b05ec9c8 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -30,13 +30,6 @@ #include "../kselftest.h" #include "vm_util.h" -#ifndef MADV_PAGEOUT -#define MADV_PAGEOUT 21 -#endif -#ifndef MADV_COLLAPSE -#define MADV_COLLAPSE 25 -#endif

static size_t pagesize; static int pagemap_fd; static size_t thpsize; diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c index 97adc0f34f9c..e88ee039d0eb 100644 --- a/tools/testing/selftests/mm/khugepaged.c +++ b/tools/testing/selftests/mm/khugepaged.c @@ -22,16 +22,6 @@ #include "vm_util.h" -#ifndef MADV_PAGEOUT -#define MADV_PAGEOUT 21 -#endif -#ifndef MADV_POPULATE_READ -#define MADV_POPULATE_READ 22 -#endif -#ifndef MADV_COLLAPSE -#define MADV_COLLAPSE 25 -#endif

#define BASE_ADDR ((void *)(1UL << 30)) static unsigned long hpage_pmd_size; static unsigned long page_size; diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index b950bd16083a..07f39ed2efba 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -63,3 +63,13 @@ int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, #define PAGEMAP_PRESENT(ent) (((ent) & (1ull << 63)) != 0) #define PAGEMAP_PFN(ent) ((ent) & ((1ull << 55) - 1))

+#ifndef MADV_PAGEOUT +#define MADV_PAGEOUT 21 +#endif +#ifndef MADV_POPULATE_READ +#define MADV_POPULATE_READ 22 +#endif +#ifndef MADV_COLLAPSE +#define MADV_COLLAPSE 25 +#endif

On 6/6/23 12:16 PM, John Hubbard wrote:

There are only three uffd*() routines that are used outside of the uffd selftests. Leave these in vm_util.c, where they are available to any mm selftest program:

uffd_register()
uffd_unregister()
uffd_register_with_ioctls().

A few other uffd*() routines, however, are only used by the uffd-focused tests found in uffd-stress.c and uffd-unit-tests.c. Move those routines into uffd-common.c.

Cc: Peter Xu peterx@redhat.com Acked-by: David Hildenbrand david@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Tested-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/mm/uffd-common.c | 59 ++++++++++++++++++++++++ tools/testing/selftests/mm/uffd-common.h | 5 ++ tools/testing/selftests/mm/vm_util.c | 59 ------------------------ tools/testing/selftests/mm/vm_util.h | 4 -- 4 files changed, 64 insertions(+), 63 deletions(-)

diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c index 61c6250adf93..ba20d7504022 100644 --- a/tools/testing/selftests/mm/uffd-common.c +++ b/tools/testing/selftests/mm/uffd-common.c @@ -616,3 +616,62 @@ int copy_page(int ufd, unsigned long offset, bool wp) { return __copy_page(ufd, offset, false, wp); }

+int uffd_open_dev(unsigned int flags) +{

• int fd, uffd;
• fd = open("/dev/userfaultfd", O_RDWR | O_CLOEXEC);
• if (fd < 0)

• return fd;
  

• uffd = ioctl(fd, USERFAULTFD_IOC_NEW, flags);
• close(fd);
• return uffd;

+}

+int uffd_open_sys(unsigned int flags) +{ +#ifdef __NR_userfaultfd

• return syscall(__NR_userfaultfd, flags);

+#else

• return -1;

+#endif +}

+int uffd_open(unsigned int flags) +{

• int uffd = uffd_open_sys(flags);
• if (uffd < 0)

• uffd = uffd_open_dev(flags);
  

• return uffd;

+}

+int uffd_get_features(uint64_t *features) +{

• struct uffdio_api uffdio_api = { .api = UFFD_API, .features = 0 };
• /*

• * This should by default work in most kernels; the feature list
  

• * will be the same no matter what we pass in here.
  

• */
  

• int fd = uffd_open(UFFD_USER_MODE_ONLY);
• if (fd < 0)

• /* Maybe the kernel is older than user-only mode? */
  

• fd = uffd_open(0);
  

• if (fd < 0)

• return fd;
  

• if (ioctl(fd, UFFDIO_API, &uffdio_api)) {

• close(fd);
  

• return -errno;
  

• }
• *features = uffdio_api.features;
• close(fd);
• return 0;

+} diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h index 6068f2346b86..197f5262fe0d 100644 --- a/tools/testing/selftests/mm/uffd-common.h +++ b/tools/testing/selftests/mm/uffd-common.h @@ -110,6 +110,11 @@ int __copy_page(int ufd, unsigned long offset, bool retry, bool wp); int copy_page(int ufd, unsigned long offset, bool wp); void *uffd_poll_thread(void *arg); +int uffd_open_dev(unsigned int flags); +int uffd_open_sys(unsigned int flags); +int uffd_open(unsigned int flags); +int uffd_get_features(uint64_t *features);

#define TEST_ANON 1 #define TEST_HUGETLB 2 #define TEST_SHMEM 3 diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index 9b06a5034808..681277615839 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -242,62 +242,3 @@ int uffd_unregister(int uffd, void *addr, uint64_t len) return ret; }

-int uffd_open_dev(unsigned int flags) -{

• int fd, uffd;
• fd = open("/dev/userfaultfd", O_RDWR | O_CLOEXEC);
• if (fd < 0)

• return fd;
  

• uffd = ioctl(fd, USERFAULTFD_IOC_NEW, flags);
• close(fd);
• return uffd;

-}

-int uffd_open_sys(unsigned int flags) -{ -#ifdef __NR_userfaultfd

• return syscall(__NR_userfaultfd, flags);

-#else

• return -1;

-#endif -}

-int uffd_open(unsigned int flags) -{

• int uffd = uffd_open_sys(flags);
• if (uffd < 0)

• uffd = uffd_open_dev(flags);
  

• return uffd;

-}

-int uffd_get_features(uint64_t *features) -{

• struct uffdio_api uffdio_api = { .api = UFFD_API, .features = 0 };
• /*

• * This should by default work in most kernels; the feature list
  

• * will be the same no matter what we pass in here.
  

• */
  

• int fd = uffd_open(UFFD_USER_MODE_ONLY);
• if (fd < 0)

• /* Maybe the kernel is older than user-only mode? */
  

• fd = uffd_open(0);
  

• if (fd < 0)

• return fd;
  

• if (ioctl(fd, UFFDIO_API, &uffdio_api)) {

• close(fd);
  

• return -errno;
  

• }
• *features = uffdio_api.features;
• close(fd);
• return 0;

-} diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index 07f39ed2efba..c2d4ff798b91 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -48,10 +48,6 @@ unsigned long default_huge_page_size(void); int uffd_register(int uffd, void *addr, uint64_t len, bool miss, bool wp, bool minor); int uffd_unregister(int uffd, void *addr, uint64_t len); -int uffd_open_dev(unsigned int flags); -int uffd_open_sys(unsigned int flags); -int uffd_open(unsigned int flags); -int uffd_get_features(uint64_t *features); int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, bool miss, bool wp, bool minor, uint64_t *ioctls);

On 6/6/23 12:16 PM, John Hubbard wrote:

As per a discussion with Muhammad Usama Anjum [1], the following is how one is supposed to build selftests:

make headers && make -C tools/testing/selftests/mm

However, that's not yet documented anywhere. So add it to Documentation/dev-tools/kselftest.rst .

[1] https://lore.kernel.org/all/bf910fa5-0c96-3707-cce4-5bcc656b6274@collabora.c...

Cc: Peter Xu peterx@redhat.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: Jonathan Corbet corbet@lwn.net Cc: linux-doc@vger.kernel.org Reviewed-by: David Hildenbrand david@redhat.com Signed-off-by: John Hubbard jhubbard@nvidia.com

Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

Documentation/dev-tools/kselftest.rst | 1 + 1 file changed, 1 insertion(+)

diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index 12b575b76b20..6e35d042199c 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -36,6 +36,7 @@ Running the selftests (hotplug tests are run in limited mode) To build the tests::

• $ make headers $ make -C tools/testing/selftests

To run the tests::

On 6/6/23 00:38, Muhammad Usama Anjum wrote: ...

...

+kernel_header_files:

• @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \
• if [ $$? -ne 0 ]; then \

•        RED='\033[1;31m';                                                  \
  

•        NOCOLOR='\033[0m';                                                 \
  

•        echo;                                                              \
  

•        echo -e "$${RED}error$${NOCOLOR}: missing kernel header files.";   \
  

•        echo "Please run this and try again:";                             \
  

•        echo;                                                              \
  

•        echo "    cd $(top_srcdir)";                                       \
  

•        echo "    make headers";                                           \
  

•        echo;                                                              \
  

•    exit 1; \
  

• fi

Thank you for adding this. This is outputting error for every selftest directory. We should try to make it even better by just aborting the Make-ing process the first time headers aren't detected. We can do this now or later, fine by me.

OK, I see. Yes, this can be improved by adding the same mechanism to the selftests/Makefile, that is in selftests/mm/Makefile.

I'd like to keep both, because as I mentioned earlier, mm folks like to run just that one Makefile, sometimes, and selftests/mm/Makefile is not invoking the top level Makefile. Rather, it includes lib.mk--which the top level Makefile does *not* include.

Arguably, using includes instead of recursive Make, would improve this framework: reduce duplication such as the above. But that's a larger project and just food for thought at this point.

Anyway, this works nicely on my system, and I'll attach it as a patch also in case you want to try it out. What do you think of this:

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 90a62cf75008..bdca160063d8 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -144,10 +144,12 @@ ifneq ($(KBUILD_OUTPUT),) abs_objtree := $(realpath $(abs_objtree)) BUILD := $(abs_objtree)/kselftest KHDR_INCLUDES := -isystem ${abs_objtree}/usr/include + KHDR_DIR := ${abs_objtree}/usr/include else BUILD := $(CURDIR) abs_srctree := $(shell cd $(top_srcdir) && pwd) KHDR_INCLUDES := -isystem ${abs_srctree}/usr/include + KHDR_DIR := ${abs_srctree}/usr/include DEFAULT_INSTALL_HDR_PATH := 1 endif

@@ -161,7 +163,7 @@ export KHDR_INCLUDES # all isn't the first target in the file. .DEFAULT_GOAL := all

-all: +all: kernel_header_files @ret=1; \ for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ @@ -172,6 +174,23 @@ all: ret=$$((ret * $$?)); \ done; exit $$ret;

+kernel_header_files: + @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ + if [ $$? -ne 0 ]; then \ + RED='\033[1;31m'; \ + NOCOLOR='\033[0m'; \ + echo; \ + echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ + echo "Please run this and try again:"; \ + echo; \ + echo " cd $(top_srcdir)"; \ + echo " make headers"; \ + echo; \ + exit 1; \ + fi + +.PHONY: kernel_header_files + run_tests: all @for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \

thanks,

On 6/6/23 12:16 PM, John Hubbard wrote:

As per a discussion with Muhammad Usama Anjum [1], the following is how one is supposed to build selftests:

make headers && make -C tools/testing/selftests/mm

Change the selftest build system's lib.mk to fail out with a helpful message if that prerequisite "make headers" has not been done yet.

[1] https://lore.kernel.org/all/bf910fa5-0c96-3707-cce4-5bcc656b6274@collabora.c...

Cc: David Hildenbrand david@redhat.com Cc: Peter Xu peterx@redhat.com Cc: Muhammad Usama Anjum usama.anjum@collabora.com Cc: Jonathan Corbet corbet@lwn.net Cc: linux-doc@vger.kernel.org Signed-off-by: John Hubbard jhubbard@nvidia.com

Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com

---

tools/testing/selftests/lib.mk | 36 +++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 05400462c779..b8ea03b9a015 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -44,10 +44,22 @@ endif selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST)))) top_srcdir = $(selfdir)/../../.. -ifeq ($(KHDR_INCLUDES),) -KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include +ifneq ($(KBUILD_OUTPUT),)

• # Make's built-in functions such as $(abspath ...), $(realpath ...) cannot
• # expand a shell special character '~'. We use a somewhat tedious way here.
• abs_objtree := $(shell cd $(top_srcdir) && mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd)
• $(if $(abs_objtree),, \
• $(error failed to create output directory "$(KBUILD_OUTPUT)"))
• # $(realpath ...) resolves symlinks
• abs_objtree := $(realpath $(abs_objtree))
• KHDR_DIR := ${abs_objtree}/usr/include

+else

• abs_srctree := $(shell cd $(top_srcdir) && pwd)
• KHDR_DIR := ${abs_srctree}/usr/include

endif +KHDR_INCLUDES := -isystem $(KHDR_DIR)

# The following are built by lib.mk common compile rules. # TEST_CUSTOM_PROGS should be used by tests that require # custom build rule and prevent common build rule use. @@ -58,7 +70,25 @@ 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) +all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \

• $(TEST_GEN_FILES)
  

+kernel_header_files:

• @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \
• if [ $$? -ne 0 ]; then \

•        RED='\033[1;31m';                                                  \
  

•        NOCOLOR='\033[0m';                                                 \
  

•        echo;                                                              \
  

•        echo -e "$${RED}error$${NOCOLOR}: missing kernel header files.";   \
  

•        echo "Please run this and try again:";                             \
  

•        echo;                                                              \
  

•        echo "    cd $(top_srcdir)";                                       \
  

•        echo "    make headers";                                           \
  

•        echo;                                                              \
  

•    exit 1; \
  

• fi

+.PHONY: kernel_header_files define RUN_TESTS BASE_DIR="$(selfdir)"; \