[v3,00/11] A minor flurry of selftest/mm fixes

Hi,

Changes since v2 [1]:

* Added a new patch (sent separately earlier) at the end, to error out
  if "make headers" has not yet been run.

* Reworked and simplified the uffd movement patch. Now it only moves
  some uffd*() routines, not all, and doesn't have to touch the Makefile
  at all. This lighter touch also allowed me to drop the "move psize(),
  pshift() into vm_utils.c" entirely. I expect Peter Xu will be a little
  happier with this new approach.

* Fixed the commit description for the MADV_COLLAPSE patch.

* Added more Reviewed-by tags from David Hildenbrand and Peter Xu.

[1] https://lore.kernel.org/all/20230603021558.95299-1-jhubbard@nvidia.com/

John Hubbard (11):
  selftests/mm: fix uffd-stress unused function warning
  selftests/mm: fix unused variable warnings in hugetlb-madvise.c,
    migration.c
  selftests/mm: fix "warning: expression which evaluates to zero..." in
    mlock2-tests.c
  selftests/mm: fix invocation of tests that are run via shell scripts
  selftests/mm: .gitignore: add mkdirty, va_high_addr_switch
  selftests/mm: fix two -Wformat-security warnings in uffd builds
  selftests/mm: fix a "possibly uninitialized" warning in pkey-x86.h
  selftests/mm: fix build failures due to missing MADV_COLLAPSE
  selftests/mm: move certain uffd*() routines from vm_util.c to
    uffd-common.c
  Documentation: kselftest: "make headers" is a prerequisite
  selftests: error out if kernel header files are not yet built

 Documentation/dev-tools/kselftest.rst        |  1 +
 tools/testing/selftests/lib.mk               | 36 +++++++++++-
 tools/testing/selftests/mm/.gitignore        |  2 +
 tools/testing/selftests/mm/cow.c             |  7 ---
 tools/testing/selftests/mm/hugetlb-madvise.c |  8 ++-
 tools/testing/selftests/mm/khugepaged.c      | 10 ----
 tools/testing/selftests/mm/migration.c       |  5 +-
 tools/testing/selftests/mm/mlock2-tests.c    |  1 -
 tools/testing/selftests/mm/pkey-x86.h        |  2 +-
 tools/testing/selftests/mm/run_vmtests.sh    |  6 +-
 tools/testing/selftests/mm/uffd-common.c     | 59 ++++++++++++++++++++
 tools/testing/selftests/mm/uffd-common.h     |  5 ++
 tools/testing/selftests/mm/uffd-stress.c     | 10 ----
 tools/testing/selftests/mm/uffd-unit-tests.c | 16 ++----
 tools/testing/selftests/mm/vm_util.c         | 59 --------------------
 tools/testing/selftests/mm/vm_util.h         | 14 +++--
 16 files changed, 130 insertions(+), 111 deletions(-)


base-commit: f8dba31b0a826e691949cd4fdfa5c30defaac8c5

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:
> 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:
> 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:
> 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);
>