public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Alistair Popple <apopple@nvidia.com>
Cc: <linux-mm@kvack.org>, <zenghui.yu@linux.dev>,
	<Liam.Howlett@oracle.com>, <akpm@linux-foundation.org>,
	<david@kernel.org>, <jgg@ziepe.ca>, <leon@kernel.org>,
	<linux-kernel@vger.kernel.org>, <ljs@kernel.org>,
	<mhocko@suse.com>, <rppt@kernel.org>, <surenb@google.com>,
	<vbabka@kernel.org>, <dri-devel@lists.freedesktop.org>,
	<balbirs@nvidia.com>
Subject: Re: [PATCH 2/3] selftests/mm: hmm-tests: don't hardcode THP size to 2MB
Date: Wed, 1 Apr 2026 16:01:52 -0700	[thread overview]
Message-ID: <ac2j4BLA/jY01rgl@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <acyq8B7/Uwowj+kl@gsse-cloud1.jf.intel.com>

On Tue, Mar 31, 2026 at 10:19:44PM -0700, Matthew Brost wrote:
> On Tue, Mar 31, 2026 at 05:34:44PM +1100, Alistair Popple wrote:
> > Several HMM tests hardcode TWOMEG as the THP size. This is wrong on
> > architectures where the PMD size is not 2MB such as arm64 with 64K base
> 
> +1
> 
> But one question…
> 
> Can’t the THP size be derived via HPAGE_PMD_ORDER?
> 
> If not, then some DRM common code I’ve written will break on arm64.
> 

Ignore this I seemly forgot we are in userspace here.

Matt

> Matt
> 
> > pages where THP is 512MB. Fix this by using read_pmd_pagesize() from
> > vm_util instead.
> > 
> > While here also replace the custom file_read_ulong() helper used to
> > parse the default hugetlbfs page size from /proc/meminfo with the
> > existing default_huge_page_size() from vm_util.
> > 
> > [1] https://lore.kernel.org/linux-mm/8bd0396a-8997-4d2e-a13f-5aac033083d7@linux.dev/
> > 
> > Fixes: fee9f6d1b8df ("mm/hmm/test: add selftests for HMM")
> > Fixes: 519071529d2a ("selftests/mm/hmm-tests: new tests for zone device THP migration")
> > Reported-by: Zenghui Yu <zenghui.yu@linux.dev>
> > Closes: https://lore.kernel.org/linux-mm/8bd0396a-8997-4d2e-a13f-5aac033083d7@linux.dev/
> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> > ---
> >  tools/testing/selftests/mm/hmm-tests.c | 83 +++++---------------------
> >  1 file changed, 16 insertions(+), 67 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c
> > index e8328c89d855..788689497e92 100644
> > --- a/tools/testing/selftests/mm/hmm-tests.c
> > +++ b/tools/testing/selftests/mm/hmm-tests.c
> > @@ -34,6 +34,7 @@
> >   */
> >  #include <lib/test_hmm_uapi.h>
> >  #include <mm/gup_test.h>
> > +#include <mm/vm_util.h>
> >  
> >  struct hmm_buffer {
> >  	void		*ptr;
> > @@ -548,7 +549,7 @@ TEST_F(hmm, anon_write_child)
> >  
> >  	for (migrate = 0; migrate < 2; ++migrate) {
> >  		for (use_thp = 0; use_thp < 2; ++use_thp) {
> > -			npages = ALIGN(use_thp ? TWOMEG : HMM_BUFFER_SIZE,
> > +			npages = ALIGN(use_thp ? read_pmd_pagesize() : HMM_BUFFER_SIZE,
> >  				       self->page_size) >> self->page_shift;
> >  			ASSERT_NE(npages, 0);
> >  			size = npages << self->page_shift;
> > @@ -728,7 +729,7 @@ TEST_F(hmm, anon_write_huge)
> >  	int *ptr;
> >  	int ret;
> >  
> > -	size = 2 * TWOMEG;
> > +	size = 2 * read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > @@ -744,7 +745,7 @@ TEST_F(hmm, anon_write_huge)
> >  			   buffer->fd, 0);
> >  	ASSERT_NE(buffer->ptr, MAP_FAILED);
> >  
> > -	size = TWOMEG;
> > +	size /= 2;
> >  	npages = size >> self->page_shift;
> >  	map = (void *)ALIGN((uintptr_t)buffer->ptr, size);
> >  	ret = madvise(map, size, MADV_HUGEPAGE);
> > @@ -770,54 +771,6 @@ TEST_F(hmm, anon_write_huge)
> >  	hmm_buffer_free(buffer);
> >  }
> >  
> > -/*
> > - * Read numeric data from raw and tagged kernel status files.  Used to read
> > - * /proc and /sys data (without a tag) and from /proc/meminfo (with a tag).
> > - */
> > -static long file_read_ulong(char *file, const char *tag)
> > -{
> > -	int fd;
> > -	char buf[2048];
> > -	int len;
> > -	char *p, *q;
> > -	long val;
> > -
> > -	fd = open(file, O_RDONLY);
> > -	if (fd < 0) {
> > -		/* Error opening the file */
> > -		return -1;
> > -	}
> > -
> > -	len = read(fd, buf, sizeof(buf));
> > -	close(fd);
> > -	if (len < 0) {
> > -		/* Error in reading the file */
> > -		return -1;
> > -	}
> > -	if (len == sizeof(buf)) {
> > -		/* Error file is too large */
> > -		return -1;
> > -	}
> > -	buf[len] = '\0';
> > -
> > -	/* Search for a tag if provided */
> > -	if (tag) {
> > -		p = strstr(buf, tag);
> > -		if (!p)
> > -			return -1; /* looks like the line we want isn't there */
> > -		p += strlen(tag);
> > -	} else
> > -		p = buf;
> > -
> > -	val = strtol(p, &q, 0);
> > -	if (*q != ' ') {
> > -		/* Error parsing the file */
> > -		return -1;
> > -	}
> > -
> > -	return val;
> > -}
> > -
> >  /*
> >   * Write huge TLBFS page.
> >   */
> > @@ -826,15 +779,13 @@ TEST_F(hmm, anon_write_hugetlbfs)
> >  	struct hmm_buffer *buffer;
> >  	unsigned long npages;
> >  	unsigned long size;
> > -	unsigned long default_hsize;
> > +	unsigned long default_hsize = default_huge_page_size();
> >  	unsigned long i;
> >  	int *ptr;
> >  	int ret;
> >  
> > -	default_hsize = file_read_ulong("/proc/meminfo", "Hugepagesize:");
> > -	if (default_hsize < 0 || default_hsize*1024 < default_hsize)
> > +	if (!default_hsize)
> >  		SKIP(return, "Huge page size could not be determined");
> > -	default_hsize = default_hsize*1024; /* KB to B */
> >  
> >  	size = ALIGN(TWOMEG, default_hsize);
> >  	npages = size >> self->page_shift;
> > @@ -1606,7 +1557,7 @@ TEST_F(hmm, compound)
> >  	struct hmm_buffer *buffer;
> >  	unsigned long npages;
> >  	unsigned long size;
> > -	unsigned long default_hsize;
> > +	unsigned long default_hsize = default_huge_page_size();
> >  	int *ptr;
> >  	unsigned char *m;
> >  	int ret;
> > @@ -1614,10 +1565,8 @@ TEST_F(hmm, compound)
> >  
> >  	/* Skip test if we can't allocate a hugetlbfs page. */
> >  
> > -	default_hsize = file_read_ulong("/proc/meminfo", "Hugepagesize:");
> > -	if (default_hsize < 0 || default_hsize*1024 < default_hsize)
> > +	if (!default_hsize)
> >  		SKIP(return, "Huge page size could not be determined");
> > -	default_hsize = default_hsize*1024; /* KB to B */
> >  
> >  	size = ALIGN(TWOMEG, default_hsize);
> >  	npages = size >> self->page_shift;
> > @@ -2106,7 +2055,7 @@ TEST_F(hmm, migrate_anon_huge_empty)
> >  	int *ptr;
> >  	int ret;
> >  
> > -	size = TWOMEG;
> > +	size = read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > @@ -2158,7 +2107,7 @@ TEST_F(hmm, migrate_anon_huge_zero)
> >  	int ret;
> >  	int val;
> >  
> > -	size = TWOMEG;
> > +	size = read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > @@ -2221,7 +2170,7 @@ TEST_F(hmm, migrate_anon_huge_free)
> >  	int *ptr;
> >  	int ret;
> >  
> > -	size = TWOMEG;
> > +	size = read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > @@ -2280,7 +2229,7 @@ TEST_F(hmm, migrate_anon_huge_fault)
> >  	int *ptr;
> >  	int ret;
> >  
> > -	size = TWOMEG;
> > +	size = read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > @@ -2332,7 +2281,7 @@ TEST_F(hmm, migrate_partial_unmap_fault)
> >  {
> >  	struct hmm_buffer *buffer;
> >  	unsigned long npages;
> > -	unsigned long size = TWOMEG;
> > +	unsigned long size = read_pmd_pagesize();
> >  	unsigned long i;
> >  	void *old_ptr;
> >  	void *map;
> > @@ -2398,7 +2347,7 @@ TEST_F(hmm, migrate_remap_fault)
> >  {
> >  	struct hmm_buffer *buffer;
> >  	unsigned long npages;
> > -	unsigned long size = TWOMEG;
> > +	unsigned long size = read_pmd_pagesize();
> >  	unsigned long i;
> >  	void *old_ptr, *new_ptr = NULL;
> >  	void *map;
> > @@ -2498,7 +2447,7 @@ TEST_F(hmm, migrate_anon_huge_err)
> >  	int *ptr;
> >  	int ret;
> >  
> > -	size = TWOMEG;
> > +	size = read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > @@ -2593,7 +2542,7 @@ TEST_F(hmm, migrate_anon_huge_zero_err)
> >  	int *ptr;
> >  	int ret;
> >  
> > -	size = TWOMEG;
> > +	size = read_pmd_pagesize();
> >  
> >  	buffer = malloc(sizeof(*buffer));
> >  	ASSERT_NE(buffer, NULL);
> > -- 
> > 2.53.0
> > 


  reply	other threads:[~2026-04-01 23:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31  6:34 [PATCH 0/3] Minor hmm_test fixes and cleanups Alistair Popple
2026-03-31  6:34 ` [PATCH 1/3] lib: test_hmm: evict device pages on file close to avoid use-after-free Alistair Popple
2026-03-31  8:47   ` Balbir Singh
2026-04-05  4:35   ` Zenghui Yu
2026-04-05  4:47   ` Zenghui Yu
2026-03-31  6:34 ` [PATCH 2/3] selftests/mm: hmm-tests: don't hardcode THP size to 2MB Alistair Popple
2026-03-31  8:51   ` Balbir Singh
2026-04-01  5:19   ` Matthew Brost
2026-04-01 23:01     ` Matthew Brost [this message]
2026-04-02  6:32   ` Sayali Patil
2026-03-31  6:34 ` [PATCH 3/3] lib: test_hmm: Implement a device release method Alistair Popple
2026-03-31  8:53   ` Balbir Singh
2026-04-05  4:47   ` Zenghui Yu
2026-04-01  0:33 ` [PATCH 0/3] Minor hmm_test fixes and cleanups Andrew Morton
2026-04-01  1:20   ` Alistair Popple

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac2j4BLA/jY01rgl@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=balbirs@nvidia.com \
    --cc=david@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=zenghui.yu@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox