Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 28 May 2023 15:49:07 +0100
From:   Lorenzo Stoakes <lstoakes@gmail.com>
To:     David Hildenbrand <david@redhat.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linux-kselftest@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Shuah Khan <shuah@kernel.org>, Jens Axboe <axboe@kernel.dk>,
        Peter Xu <peterx@redhat.com>, Jason Gunthorpe <jgg@nvidia.com>,
        John Hubbard <jhubbard@nvidia.com>, Jan Kara <jack@suse.cz>
Subject: Re: [PATCH v1 1/3] selftests/mm: factor out detection of hugetlb
 page sizes into vm_util
Message-ID: <e49b5642-014b-4a95-9ba8-738bfd02b2ea@lucifer.local>
References: <20230519102723.185721-1-david@redhat.com>
 <20230519102723.185721-2-david@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230519102723.185721-2-david@redhat.com>
Precedence: bulk

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@redhat.com>
> ---
>  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@gmail.com>