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

[David Hildenbrand (Arm)]

On 4/30/26 17:27, Zi Yan wrote:
> On 30 Apr 2026, at 11:16, Zi Yan wrote:
> 
>> On 29 Apr 2026, at 11:35, 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(-)
>>>
> 
> <snip>
> 
>>> -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");
>>
>> Sashiko:
>> Since madv_ops can now be either MADV_POPULATE_READ or MADV_POPULATE_WRITE,
>> will this hardcoded error message be misleading if the write fault path
>> fails?
>>
>> Answer:
>> Will send a fixup.
> 
> 
> This is the fixup:
> From 76e301cf5198f33d07492e224ec627b94902b4b6 Mon Sep 17 00:00:00 2001
> From: Zi Yan <ziy@xxxxxxxxxx>
> Date: Thu, 30 Apr 2026 11:22:30 -0400
> Subject: [PATCH] selftests/mm: khugepaged perror fixup.
> 
> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
> ---
>  tools/testing/selftests/mm/khugepaged.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index 80b913185c643..97b8fcc490c76 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -445,7 +445,10 @@ static void file_fault_common(void *p, unsigned long start, unsigned long end,
>  		int madv_ops)
>  {
>  	if (madvise(((char *)p) + start, end - start, madv_ops)) {
> -		perror("madvise(MADV_POPULATE_READ");
> +		if (madv_ops == MADV_POPULATE_READ)
> +			perror("madvise(MADV_POPULATE_READ");
> +		else if (madv_ops == MADV_POPULATE_WRITE)
> +			perror("madvise(MADV_POPULATE_WRITE");

Alternatively, just "madvise()". It's unexpected to fail in any case and would
have to be debugged ...

-- 
Cheers,

David