Re: [PATCH v5 11/14] selftests/mm: remove READ_ONLY_THP_FOR_FS in khugepaged

[Zi Yan]

On 9 May 2026, at 3:51, David Hildenbrand (Arm) wrote:

> On 5/6/26 15:11, Zi Yan wrote:
>> On 4 May 2026, at 12:23, Nico Pache wrote:
>>
>>> On 4/29/26 9:35 AM, Zi Yan wrote:
>>>> Change the requirement to a file system with large folio support and the
>>>> supported order needs to include PMD_ORDER.
>>>>
>>>> Also add tests of opening a file with read write permission and populating
>>>> folios with writes. Reuse the XFS image from split_huge_page_test.
>>>>
>>>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>>>> ---
>>>>   tools/testing/selftests/mm/khugepaged.c   | 131 +++++++++++++++-------
>>>>   tools/testing/selftests/mm/run_vmtests.sh |  12 +-
>>>>   2 files changed, 102 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
>>>> index a6bb9d50363d2..80b913185c643 100644
>>>> --- a/tools/testing/selftests/mm/khugepaged.c
>>>> +++ b/tools/testing/selftests/mm/khugepaged.c
>>>> @@ -49,7 +49,8 @@ struct mem_ops {
>>>>   	const char *name;
>>>>   };
>>>>  -static struct mem_ops *file_ops;
>>>> +static struct mem_ops *read_only_file_ops;
>>>> +static struct mem_ops *read_write_file_ops;
>>>>   static struct mem_ops *anon_ops;
>>>>   static struct mem_ops *shmem_ops;
>>>>  @@ -112,7 +113,8 @@ static void restore_settings(int sig)
>>>>   static void save_settings(void)
>>>>   {
>>>>   	printf("Save THP and khugepaged settings...");
>>>> -	if (file_ops && finfo.type == VMA_FILE)
>>>> +	if ((read_only_file_ops || read_write_file_ops) &&
>>>> +	    finfo.type == VMA_FILE)
>>>>   		thp_set_read_ahead_path(finfo.dev_queue_read_ahead_path);
>>>>   	thp_save_settings();
>>>>  @@ -364,11 +366,14 @@ static bool anon_check_huge(void *addr, int nr_hpages)
>>>>   	return check_huge_anon(addr, nr_hpages, hpage_pmd_size);
>>>>   }
>>>>  -static void *file_setup_area(int nr_hpages)
>>>> +static void *file_setup_area_common(int nr_hpages, bool read_only)
>>>>   {
>>>>   	int fd;
>>>>   	void *p;
>>>>   	unsigned long size;
>>>> +	int open_opt = read_only ? O_RDONLY : O_RDWR;
>>>> +	int mmap_prot = read_only ? PROT_READ : (PROT_READ | PROT_WRITE);
>>>> +	int mmap_opt = read_only ? MAP_PRIVATE : MAP_SHARED;
>>>>    	unlink(finfo.path);  /* Cleanup from previous failed tests */
>>>>   	printf("Creating %s for collapse%s...", finfo.path,
>>>> @@ -399,14 +404,15 @@ static void *file_setup_area(int nr_hpages)
>>>>   	munmap(p, size);
>>>>   	success("OK");
>>>>  -	printf("Opening %s read only for collapse...", finfo.path);
>>>> -	finfo.fd = open(finfo.path, O_RDONLY, 777);
>>>> +	printf("Opening %s %s for collapse...", finfo.path,
>>>> +	       read_only ? "read only" : "read-write");
>>>> +	finfo.fd = open(finfo.path, open_opt, 777);
>>>>   	if (finfo.fd < 0) {
>>>>   		perror("open()");
>>>>   		exit(EXIT_FAILURE);
>>>>   	}
>>>> -	p = mmap(BASE_ADDR, size, PROT_READ,
>>>> -		 MAP_PRIVATE, finfo.fd, 0);
>>>> +	p = mmap(BASE_ADDR, size, mmap_prot,
>>>> +		 mmap_opt, finfo.fd, 0);
>>>>   	if (p == MAP_FAILED || p != BASE_ADDR) {
>>>>   		perror("mmap()");
>>>>   		exit(EXIT_FAILURE);
>>>> @@ -418,6 +424,16 @@ static void *file_setup_area(int nr_hpages)
>>>>   	return p;
>>>>   }
>>>>  +static void *file_setup_read_only_area(int nr_hpages)
>>>> +{
>>>> +	return file_setup_area_common(nr_hpages, /* read_only= */ true);
>>>> +}
>>>> +
>>>> +static void *file_setup_read_write_area(int nr_hpages)
>>>> +{
>>>> +	return file_setup_area_common(nr_hpages, /* read_only= */ false);
>>>> +}
>>>> +
>>>>   static void file_cleanup_area(void *p, unsigned long size)
>>>>   {
>>>>   	munmap(p, size);
>>>> @@ -425,14 +441,25 @@ static void file_cleanup_area(void *p, unsigned long size)
>>>>   	unlink(finfo.path);
>>>>   }
>>>>  -static void file_fault(void *p, unsigned long start, unsigned long end)
>>>> +static void file_fault_common(void *p, unsigned long start, unsigned long end,
>>>> +		int madv_ops)
>>>>   {
>>>> -	if (madvise(((char *)p) + start, end - start, MADV_POPULATE_READ)) {
>>>> +	if (madvise(((char *)p) + start, end - start, madv_ops)) {
>>>>   		perror("madvise(MADV_POPULATE_READ");
>>>>   		exit(EXIT_FAILURE);
>>>>   	}
>>>>   }
>>>>  +static void file_fault_read(void *p, unsigned long start, unsigned long end)
>>>> +{
>>>> +	file_fault_common(p, start, end, MADV_POPULATE_READ);
>>>> +}
>>>> +
>>>> +static void file_fault_write(void *p, unsigned long start, unsigned long end)
>>>> +{
>>>> +	file_fault_common(p, start, end, MADV_POPULATE_WRITE);
>>>> +}
>>>> +
>>>>   static bool file_check_huge(void *addr, int nr_hpages)
>>>>   {
>>>>   	switch (finfo.type) {
>>>> @@ -488,10 +515,18 @@ static struct mem_ops __anon_ops = {
>>>>   	.name = "anon",
>>>>   };
>>>>  -static struct mem_ops __file_ops = {
>>>> -	.setup_area = &file_setup_area,
>>>> +static struct mem_ops __read_only_file_ops = {
>>>> +	.setup_area = &file_setup_read_only_area,
>>>>   	.cleanup_area = &file_cleanup_area,
>>>> -	.fault = &file_fault,
>>>> +	.fault = &file_fault_read,
>>>> +	.check_huge = &file_check_huge,
>>>> +	.name = "file",
>>>> +};
>>>> +
>>>> +static struct mem_ops __read_write_file_ops = {
>>>> +	.setup_area = &file_setup_read_write_area,
>>>> +	.cleanup_area = &file_cleanup_area,
>>>> +	.fault = &file_fault_write,
>>>>   	.check_huge = &file_check_huge,
>>>>   	.name = "file",
>>>>   };
>>>> @@ -504,6 +539,18 @@ static struct mem_ops __shmem_ops = {
>>>>   	.name = "shmem",
>>>>   };
>>>>  +static bool is_tmpfs(struct mem_ops *ops)
>>>> +{
>>>> +	return (ops == &__read_only_file_ops ||
>>>> +		ops == &__read_write_file_ops) &&
>>>> +	       finfo.type == VMA_SHMEM;
>>>> +}
>>>> +
>>>> +static bool is_anon(struct mem_ops *ops)
>>>> +{
>>>> +	return ops == &__anon_ops;
>>>> +}
>>>> +
>>>>   static void __madvise_collapse(const char *msg, char *p, int nr_hpages,
>>>>   			       struct mem_ops *ops, bool expect)
>>>>   {
>>>> @@ -512,6 +559,10 @@ static void __madvise_collapse(const char *msg, char *p, int nr_hpages,
>>>>    	printf("%s...", msg);
>>>>  +	/* read&write file collapse always fail */
>>>
>>> Just to confirm, you are adding the write part here so that before commit 13 & 14, the behavior is that it will fail. Whereas after with patch 13/14, we expect this behavior to be supported correct?
>> Yes.
>
> Confusing, we usually add new test after adding new functionality, not the other
> way around? :)

My intention is to make sure my changes do not alter existing read-only collapse behavior
before patch 13.


Best Regards,
Yan, Zi