Re: [PATCH v3 1/2] kselftest: mm: prevent random failure of huge page split for khugepaged

[Yeoreum Yun]

Hi Lorenzo,
> >  	if (!rss_anon_before)
> >  		ksft_exit_fail_msg("No RssAnon is allocated before split\n");
> >
> > +	/* Prevent khugepaged from collapsing the pages. */
> > +	if (madvise(one_page, len, MADV_NOHUGEPAGE))
> > +		ksft_print_msg("madvise(MADV_NOHUGEPAGE) failed\n");
> 
> This should probably terminate the test no? There's no reason to expect this to
> fail and it's better to fail then to risk a flake :)

Okay. I thought it was rare, it seemed enough with just message.
I'll change with your suggesttion.

> 
> > +
> >  	/* split all THPs */
> >  	write_debugfs(PID_FMT, getpid(), (uint64_t)one_page,
> >  		      (uint64_t)one_page + len, 0);
> > @@ -227,6 +231,10 @@ static void split_pmd_thp_to_order(int order)
> >  	if (!check_huge_anon(one_page, 4 * pmd_pagesize, 4, pmd_pagesize))
> >  		ksft_exit_fail_msg("No THP is allocated\n");
> >
> > +	/* Prevent khugepaged from collapsing the pages. */
> > +	if (madvise(one_page, len, MADV_NOHUGEPAGE))
> > +		ksft_print_msg("madvise(MADV_NOHUGEPAGE) failed\n");
> > +
> 
> Same comment as above, also since this is a repeated pattern, I think it's worth
> abstracting it like:
> 
> 	static void madv_nohuge(char *ptr, size_t len)
> 	{
> 		if (!madvise(ptr, len, MADV_NOHUGEPAGE))
> 			return;
> 
> 		ksft_exit_fail_msg("MADV_NOHUGEPAGE failed, err=%d\n", errno);
> 	}

Acked.

> 
> >  	/* split all THPs */
> >  	write_debugfs(PID_FMT, getpid(), (uint64_t)one_page,
> >  		(uint64_t)one_page + len, order);
> > @@ -313,6 +321,10 @@ static void split_pte_mapped_thp(void)
> >  		goto out;
> >  	}
> >
> > +	/* Prevent khugepaged from collapsing the pages. */
> > +	if (madvise(thp_area, thp_area_size, MADV_NOHUGEPAGE))
> > +		ksft_print_msg("madvise(MADV_NOHUGEPAGE) failed\n");
> > +
> >  	/* Split all THPs through the remapped pages. */
> >  	write_debugfs(PID_FMT, getpid(), (uint64_t)page_area,
> >  		      (uint64_t)page_area + page_area_size, 0);
> > @@ -542,6 +554,9 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
> >  		ksft_test_result_skip("Pagecache folio split skipped\n");
> >  		return -2;
> >  	}
> > +	/* Prevent khugepaged from collapsing the pages. */
> > +	if (madvise(*addr, fd_size, MADV_NOHUGEPAGE))
> > +		ksft_print_msg("madvise(MADV_NOHUGEPAGE) failed\n");
> 
> Obviously same comments re: this and above
> 

Thanks!

[...]

-- 
Sincerely,
Yeoreum Yun