[PATCH v2] selftests: mincore: fix tmpfs mincore test failure

[PATCH v2] selftests: mincore: fix tmpfs mincore test failure

[Baolin Wang]

When running mincore test cases, I encountered the following failures:

"
mincore_selftest.c:359:check_tmpfs_mmap:Expected ra_pages (511) == 0 (0)
mincore_selftest.c:360:check_tmpfs_mmap:Read-ahead pages found in memory
check_tmpfs_mmap: Test terminated by assertion
          FAIL  global.check_tmpfs_mmap
not ok 5 global.check_tmpfs_mmap
FAILED: 4 / 5 tests passed
"

The reason for the test case failure is that my system automatically enabled
tmpfs large folio allocation by adding the 'transparent_hugepage_tmpfs=always'
cmdline. However, the test case still expects the tmpfs mounted on /dev/shm to
allocate small folios, which leads to assertion failures when verifying readahead
pages.

As discussed with David, there's no reason to continue checking the readahead
logic for tmpfs. Drop it to fix this issue.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
Changes from v1:
 - Drop the readahead logic check, per David.
---
 .../testing/selftests/mincore/mincore_selftest.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/mincore/mincore_selftest.c b/tools/testing/selftests/mincore/mincore_selftest.c
index e949a43a6145..0fd4b00bd345 100644
--- a/tools/testing/selftests/mincore/mincore_selftest.c
+++ b/tools/testing/selftests/mincore/mincore_selftest.c
@@ -286,8 +286,7 @@ TEST(check_file_mmap)
 
 /*
  * Test mincore() behavior on a page backed by a tmpfs file.  This test
- * performs the same steps as the previous one. However, we don't expect
- * any readahead in this case.
+ * performs the same steps as the previous one.
  */
 TEST(check_tmpfs_mmap)
 {
@@ -298,7 +297,6 @@ TEST(check_tmpfs_mmap)
 	int page_size;
 	int fd;
 	int i;
-	int ra_pages = 0;
 
 	page_size = sysconf(_SC_PAGESIZE);
 	vec_size = FILE_SIZE / page_size;
@@ -341,8 +339,7 @@ TEST(check_tmpfs_mmap)
 	}
 
 	/*
-	 * Touch a page in the middle of the mapping. We expect only
-	 * that page to be fetched into memory.
+	 * Touch a page in the middle of the mapping.
 	 */
 	addr[FILE_SIZE / 2] = 1;
 	retval = mincore(addr, FILE_SIZE, vec);
@@ -351,15 +348,6 @@ TEST(check_tmpfs_mmap)
 		TH_LOG("Page not found in memory after use");
 	}
 
-	i = FILE_SIZE / 2 / page_size + 1;
-	while (i < vec_size && vec[i]) {
-		ra_pages++;
-		i++;
-	}
-	ASSERT_EQ(ra_pages, 0) {
-		TH_LOG("Read-ahead pages found in memory");
-	}
-
 	munmap(addr, FILE_SIZE);
 	close(fd);
 	free(vec);
-- 
2.43.5

Re: [PATCH v2] selftests: mincore: fix tmpfs mincore test failure

[Zi Yan]

On 7 Apr 2025, at 7:31, Baolin Wang wrote:

> When running mincore test cases, I encountered the following failures:
>
> "
> mincore_selftest.c:359:check_tmpfs_mmap:Expected ra_pages (511) == 0 (0)
> mincore_selftest.c:360:check_tmpfs_mmap:Read-ahead pages found in memory
> check_tmpfs_mmap: Test terminated by assertion
>           FAIL  global.check_tmpfs_mmap
> not ok 5 global.check_tmpfs_mmap
> FAILED: 4 / 5 tests passed
> "
>
> The reason for the test case failure is that my system automatically enabled
> tmpfs large folio allocation by adding the 'transparent_hugepage_tmpfs=always'
> cmdline. However, the test case still expects the tmpfs mounted on /dev/shm to
> allocate small folios, which leads to assertion failures when verifying readahead
> pages.
>
> As discussed with David, there's no reason to continue checking the readahead
> logic for tmpfs. Drop it to fix this issue.
>
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
> Changes from v1:
>  - Drop the readahead logic check, per David.
> ---
>  .../testing/selftests/mincore/mincore_selftest.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
>

Acked-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v2] selftests: mincore: fix tmpfs mincore test failure

[David Hildenbrand]

On 07.04.25 13:31, Baolin Wang wrote:
> When running mincore test cases, I encountered the following failures:
> 
> "
> mincore_selftest.c:359:check_tmpfs_mmap:Expected ra_pages (511) == 0 (0)
> mincore_selftest.c:360:check_tmpfs_mmap:Read-ahead pages found in memory
> check_tmpfs_mmap: Test terminated by assertion
>            FAIL  global.check_tmpfs_mmap
> not ok 5 global.check_tmpfs_mmap
> FAILED: 4 / 5 tests passed
> "
> 
> The reason for the test case failure is that my system automatically enabled
> tmpfs large folio allocation by adding the 'transparent_hugepage_tmpfs=always'
> cmdline. However, the test case still expects the tmpfs mounted on /dev/shm to
> allocate small folios, which leads to assertion failures when verifying readahead
> pages.
> 
> As discussed with David, there's no reason to continue checking the readahead
> logic for tmpfs. Drop it to fix this issue.
> 

BTW, should we add a Fixes: ? Could only be tmpfs support that changed 
the behavior but didn't fixup the test case.

> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
> Changes from v1:
>   - Drop the readahead logic check, per David.
> ---
>   .../testing/selftests/mincore/mincore_selftest.c | 16 ++--------------
>   1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/testing/selftests/mincore/mincore_selftest.c b/tools/testing/selftests/mincore/mincore_selftest.c
> index e949a43a6145..0fd4b00bd345 100644
> --- a/tools/testing/selftests/mincore/mincore_selftest.c
> +++ b/tools/testing/selftests/mincore/mincore_selftest.c
> @@ -286,8 +286,7 @@ TEST(check_file_mmap)
>   
>   /*
>    * Test mincore() behavior on a page backed by a tmpfs file.  This test
> - * performs the same steps as the previous one. However, we don't expect
> - * any readahead in this case.
> + * performs the same steps as the previous one.
>    */
>   TEST(check_tmpfs_mmap)
>   {
> @@ -298,7 +297,6 @@ TEST(check_tmpfs_mmap)
>   	int page_size;
>   	int fd;
>   	int i;
> -	int ra_pages = 0;
>   
>   	page_size = sysconf(_SC_PAGESIZE);
>   	vec_size = FILE_SIZE / page_size;
> @@ -341,8 +339,7 @@ TEST(check_tmpfs_mmap)
>   	}
>   
>   	/*
> -	 * Touch a page in the middle of the mapping. We expect only
> -	 * that page to be fetched into memory.
> +	 * Touch a page in the middle of the mapping.
>   	 */
>   	addr[FILE_SIZE / 2] = 1;
>   	retval = mincore(addr, FILE_SIZE, vec);
> @@ -351,15 +348,6 @@ TEST(check_tmpfs_mmap)
>   		TH_LOG("Page not found in memory after use");
>   	}
>   
> -	i = FILE_SIZE / 2 / page_size + 1;
> -	while (i < vec_size && vec[i]) {
> -		ra_pages++;
> -		i++;
> -	}
> -	ASSERT_EQ(ra_pages, 0) {
> -		TH_LOG("Read-ahead pages found in memory");
> -	}
> -
>   	munmap(addr, FILE_SIZE);
>   	close(fd);
>   	free(vec);

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH v2] selftests: mincore: fix tmpfs mincore test failure

[Baolin Wang]

On 2025/4/7 23:04, David Hildenbrand wrote:
> On 07.04.25 13:31, Baolin Wang wrote:
>> When running mincore test cases, I encountered the following failures:
>>
>> "
>> mincore_selftest.c:359:check_tmpfs_mmap:Expected ra_pages (511) == 0 (0)
>> mincore_selftest.c:360:check_tmpfs_mmap:Read-ahead pages found in memory
>> check_tmpfs_mmap: Test terminated by assertion
>>            FAIL  global.check_tmpfs_mmap
>> not ok 5 global.check_tmpfs_mmap
>> FAILED: 4 / 5 tests passed
>> "
>>
>> The reason for the test case failure is that my system automatically 
>> enabled
>> tmpfs large folio allocation by adding the 
>> 'transparent_hugepage_tmpfs=always'
>> cmdline. However, the test case still expects the tmpfs mounted on 
>> /dev/shm to
>> allocate small folios, which leads to assertion failures when 
>> verifying readahead
>> pages.
>>
>> As discussed with David, there's no reason to continue checking the 
>> readahead
>> logic for tmpfs. Drop it to fix this issue.
>>
> 
> BTW, should we add a Fixes: ? Could only be tmpfs support that changed 
> the behavior but didn't fixup the test case.

I was hesitant about whether a Fixes tag is needed, because this issue 
can be traced back to the earliest patch set that added THP support to 
tmpfs. However, the test issue wasn't exposed because the 'huge=' 
parameter wasn't included when mounting /dev/shm.

After the cmdline 'transparent_hugepage_tmpfs' was added for tmpfs, the 
issue is exposed easily. So I am inclined to add a Fixes tag for the 
patch added the 'transparent_hugepage_tmpfs' cmdline.

Fixes: d635ccdb435c ("mm: shmem: add a kernel command line to change the 
default huge policy for tmpfs")

>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> ---
>> Changes from v1:
>>   - Drop the readahead logic check, per David.
>> ---
>>   .../testing/selftests/mincore/mincore_selftest.c | 16 ++--------------
>>   1 file changed, 2 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mincore/mincore_selftest.c 
>> b/tools/testing/selftests/mincore/mincore_selftest.c
>> index e949a43a6145..0fd4b00bd345 100644
>> --- a/tools/testing/selftests/mincore/mincore_selftest.c
>> +++ b/tools/testing/selftests/mincore/mincore_selftest.c
>> @@ -286,8 +286,7 @@ TEST(check_file_mmap)
>>   /*
>>    * Test mincore() behavior on a page backed by a tmpfs file.  This test
>> - * performs the same steps as the previous one. However, we don't expect
>> - * any readahead in this case.
>> + * performs the same steps as the previous one.
>>    */
>>   TEST(check_tmpfs_mmap)
>>   {
>> @@ -298,7 +297,6 @@ TEST(check_tmpfs_mmap)
>>       int page_size;
>>       int fd;
>>       int i;
>> -    int ra_pages = 0;
>>       page_size = sysconf(_SC_PAGESIZE);
>>       vec_size = FILE_SIZE / page_size;
>> @@ -341,8 +339,7 @@ TEST(check_tmpfs_mmap)
>>       }
>>       /*
>> -     * Touch a page in the middle of the mapping. We expect only
>> -     * that page to be fetched into memory.
>> +     * Touch a page in the middle of the mapping.
>>        */
>>       addr[FILE_SIZE / 2] = 1;
>>       retval = mincore(addr, FILE_SIZE, vec);
>> @@ -351,15 +348,6 @@ TEST(check_tmpfs_mmap)
>>           TH_LOG("Page not found in memory after use");
>>       }
>> -    i = FILE_SIZE / 2 / page_size + 1;
>> -    while (i < vec_size && vec[i]) {
>> -        ra_pages++;
>> -        i++;
>> -    }
>> -    ASSERT_EQ(ra_pages, 0) {
>> -        TH_LOG("Read-ahead pages found in memory");
>> -    }
>> -
>>       munmap(addr, FILE_SIZE);
>>       close(fd);
>>       free(vec);
> 
> Acked-by: David Hildenbrand <david@redhat.com>

Thanks.