Re: [PATCH 02/53] selftests/mm: khugepaged: enable collapse_single_pte_entry_compound for shmem

[Mike Rapoport <rppt@kernel.org>]

On Mon, Apr 20, 2026 at 12:20:43PM +0530, Sarthak Sharma wrote:
> Hi Mike
> 
> On 4/6/26 7:46 PM, Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> > 
> > A comment in collapse_single_pte_entry_compound() says it can't run on
> > shmem because "MADV_DONTNEED can't evict tmpfs pages".
> > But MADV_REMOVE can!
> > 
> > Use MADV_REMOVE for tmpfs to evict pages and enable
> > collapse_single_pte_entry_compound() test for shmem.
> > 
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > --->  tools/testing/selftests/mm/khugepaged.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> > index 3fe7ef04ac62..e6fb01ca44ed 100644
> > --- a/tools/testing/selftests/mm/khugepaged.c
> > +++ b/tools/testing/selftests/mm/khugepaged.c
> > @@ -783,20 +783,17 @@ static void collapse_max_ptes_swap(struct collapse_context *c, struct mem_ops *o
> >  
> >  static void collapse_single_pte_entry_compound(struct collapse_context *c, struct mem_ops *ops)
> >  {
> > +	int advise = MADV_DONTNEED;
> >  	void *p;
> >  
> >  	p = alloc_hpage(ops);
> >  
> > -	if (is_tmpfs(ops)) {
> > -		/* MADV_DONTNEED won't evict tmpfs pages */
> > -		printf("tmpfs...");
> > -		skip("Skip");
> > -		goto skip;
> > -	}
> > +	if (is_tmpfs(ops))
> > +		advise = MADV_REMOVE;
> 
> is_tmpfs(ops) will always return false for shmem_ops, since the function
> definition does not handle the shmem_ops case. Therefore, this advise
> will always remain as MADV_DONTNEED.

is_tmpfs() will be true for file ops when file is on tmpfs.
 
> Also, I am able to run the shmem tests using MADV_DONTNEED and the tests
> succeed. So, perhaps we don't need to use MADV_REMOVE in this case?

I read the comment and changed the advice to the one that will actually
evict the pages from the page cache.
Looking closely it this and collapse_max_ptes_none() test, I don't think
presence of the pages in the page cache would somehow affect splitting and
collapsing, so apparently the skips for is_tmpfs() cases can be just
dropped.
 
> >  
> >  	madvise(p, hpage_pmd_size, MADV_NOHUGEPAGE);
> >  	printf("Split huge page leaving single PTE mapping compound page...");
> > -	madvise(p + page_size, hpage_pmd_size - page_size, MADV_DONTNEED);
> > +	madvise(p + page_size, hpage_pmd_size - page_size, advise);
> >  	if (ops->check_huge(p, 0))
> >  		success("OK");
> >  	else
> > @@ -805,7 +802,6 @@ static void collapse_single_pte_entry_compound(struct collapse_context *c, struc
> >  	c->collapse("Collapse PTE table with single PTE mapping compound page",
> >  		    p, 1, ops, true);
> >  	validate_memory(p, 0, page_size);
> > -skip:
> >  	ops->cleanup_area(p, hpage_pmd_size);
> >  }
> >  
> > @@ -1251,8 +1247,10 @@ int main(int argc, char **argv)
> >  
> >  	TEST(collapse_single_pte_entry_compound, khugepaged_context, anon_ops);
> >  	TEST(collapse_single_pte_entry_compound, khugepaged_context, file_ops);
> > +	TEST(collapse_single_pte_entry_compound, khugepaged_context, shmem_ops);
> >  	TEST(collapse_single_pte_entry_compound, madvise_context, anon_ops);
> >  	TEST(collapse_single_pte_entry_compound, madvise_context, file_ops);
> > +	TEST(collapse_single_pte_entry_compound, madvise_context, shmem_ops);
> >  
> >  	TEST(collapse_full_of_compound, khugepaged_context, anon_ops);
> >  	TEST(collapse_full_of_compound, khugepaged_context, file_ops);
> 

-- 
Sincerely yours,
Mike.