Re: [PATCH v1 1/3] selftests/mm: factor out detection of hugetlb page sizes into vm_util

[Lorenzo Stoakes]

On Fri, May 19, 2023 at 12:27:21PM +0200, David Hildenbrand wrote:
> Let's factor detection out into vm_util, to be reused by a new test.
>

Bit of a nit as it's hardly vitally important, but perhaps worth mentioning
that you also refactor the function to accept any array (this is a
requirement rather than simply refactoring a thing but still :)

> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
>  tools/testing/selftests/mm/cow.c     | 29 ++--------------------------
>  tools/testing/selftests/mm/vm_util.c | 27 ++++++++++++++++++++++++++
>  tools/testing/selftests/mm/vm_util.h |  1 +
>  3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c
> index dc9d6fe86028..7f3b620d9cb7 100644
> --- a/tools/testing/selftests/mm/cow.c
> +++ b/tools/testing/selftests/mm/cow.c
> @@ -14,7 +14,6 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <fcntl.h>
> -#include <dirent.h>
>  #include <assert.h>
>  #include <sys/mman.h>
>  #include <sys/ioctl.h>
> @@ -70,31 +69,6 @@ static void detect_huge_zeropage(void)
>  	close(fd);
>  }
>
> -static void detect_hugetlbsizes(void)
> -{
> -	DIR *dir = opendir("/sys/kernel/mm/hugepages/");
> -
> -	if (!dir)
> -		return;
> -
> -	while (nr_hugetlbsizes < ARRAY_SIZE(hugetlbsizes)) {
> -		struct dirent *entry = readdir(dir);
> -		size_t kb;
> -
> -		if (!entry)
> -			break;
> -		if (entry->d_type != DT_DIR)
> -			continue;
> -		if (sscanf(entry->d_name, "hugepages-%zukB", &kb) != 1)
> -			continue;
> -		hugetlbsizes[nr_hugetlbsizes] = kb * 1024;
> -		nr_hugetlbsizes++;
> -		ksft_print_msg("[INFO] detected hugetlb size: %zu KiB\n",
> -			       kb);
> -	}
> -	closedir(dir);
> -}
> -
>  static bool range_is_swapped(void *addr, size_t size)
>  {
>  	for (; size; addr += pagesize, size -= pagesize)
> @@ -1717,7 +1691,8 @@ int main(int argc, char **argv)
>  	if (thpsize)
>  		ksft_print_msg("[INFO] detected THP size: %zu KiB\n",
>  			       thpsize / 1024);
> -	detect_hugetlbsizes();
> +	nr_hugetlbsizes = detect_hugetlb_page_sizes(hugetlbsizes,
> +						    ARRAY_SIZE(hugetlbsizes));
>  	detect_huge_zeropage();
>
>  	ksft_print_header();
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index 9b06a5034808..5cf84d860076 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <string.h>
>  #include <fcntl.h>
> +#include <dirent.h>
>  #include <sys/ioctl.h>
>  #include <linux/userfaultfd.h>
>  #include <sys/syscall.h>
> @@ -198,6 +199,32 @@ unsigned long default_huge_page_size(void)
>  	return hps;
>  }
>
> +int detect_hugetlb_page_sizes(size_t sizes[], int max)
> +{
> +	DIR *dir = opendir("/sys/kernel/mm/hugepages/");
> +	int count = 0;
> +
> +	if (!dir)
> +		return 0;
> +
> +	while (count < max) {
> +		struct dirent *entry = readdir(dir);
> +		size_t kb;
> +
> +		if (!entry)
> +			break;
> +		if (entry->d_type != DT_DIR)
> +			continue;
> +		if (sscanf(entry->d_name, "hugepages-%zukB", &kb) != 1)
> +			continue;
> +		sizes[count++] = kb * 1024;
> +		ksft_print_msg("[INFO] detected hugetlb page size: %zu KiB\n",
> +			       kb);
> +	}
> +	closedir(dir);
> +	return count;
> +}
> +
>  /* If `ioctls' non-NULL, the allowed ioctls will be returned into the var */
>  int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len,
>  			      bool miss, bool wp, bool minor, uint64_t *ioctls)
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index b950bd16083a..99b795528716 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -44,6 +44,7 @@ bool check_huge_file(void *addr, int nr_hpages, uint64_t hpage_size);
>  bool check_huge_shmem(void *addr, int nr_hpages, uint64_t hpage_size);
>  int64_t allocate_transhuge(void *ptr, int pagemap_fd);
>  unsigned long default_huge_page_size(void);
> +int detect_hugetlb_page_sizes(size_t sizes[], int max);
>
>  int uffd_register(int uffd, void *addr, uint64_t len,
>  		  bool miss, bool wp, bool minor);
> --
> 2.40.1
>

Looks good to me,

Reviewed-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>