Message-ID: <0665a8d3-7094-4732-9518-01ac313959e1@arm.com>
Date: Mon, 16 Jun 2025 21:57:10 +0530
From: Dev Jain <dev.jain@....com>
To: Aboorva Devarajan <aboorvad@...ux.ibm.com>, akpm@...ux-foundation.org,
 Liam.Howlett@...cle.com, lorenzo.stoakes@...cle.com, shuah@...nel.org,
 pfalcato@...e.de, david@...hat.com, ziy@...dia.com,
 baolin.wang@...ux.alibaba.com, npache@...hat.com, ryan.roberts@....com,
 baohua@...nel.org
Cc: linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, donettom@...ux.ibm.com, ritesh.list@...il.com
Subject: Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.


On 16/06/25 9:36 pm, Aboorva Devarajan wrote:
> From: Donet Tom <donettom@...ux.ibm.com>
>
> In this patch, we are fixing three issues in the virtual_address_range
> test.
>
> 1. validate_addr() checks if the allocated address is within the range.
> In the current implementation, if addr is greater than HIGH_ADDR_MARK,
> the test fails. However, addr will be greater than HIGH_ADDR_MARK if
> high_addr is set. Therefore, if high_addr is set, we should not check
> the (addr > HIGH_ADDR_MARK) condition.
>
> 2.In main(), the high address is stored in hptr, but for mark_range(),
> the address passed is ptr, not hptr. Fixed this by changing ptr[i] to
> hptr[i] in mark_range() function call.
>
> 3./proc/self/maps may not always have gaps smaller than MAP_CHUNK_SIZE.
> The gap between the first high address mapping and the previous mapping
> is not smaller than MAP_CHUNK_SIZE.

For this, can't we just elide the check when we cross the high boundary?
As I see it you are essentially nullifying the purpose of validate_complete_va_space;
I had written that function so as to have an alternate way of checking VA exhaustion
without relying on mmap correctness in a circular way.

Btw @Lorenzo, validate_complete_va_space was written by me as my first patch ever for
the Linux kernel : ) from the limited knowledge I have of VMA stuff, I guess the
only requirement for VMA alignment is PAGE_SIZE in this test, therefore, the only
check required is that the gap between two VMAs should be at least MAP_CHUNK_SIZE?
Or can such a gap still exist even when the VA has been exhausted?

>
> $cat /proc/3713/maps
> 10000000-10010000 r-xp 00000000 fd:00 36140094
> 10010000-10020000 r--p 00000000 fd:00 36140094
> 10020000-10030000 rw-p 00010000 fd:00 36140094
> 4ee80000-4eeb0000 rw-p 00000000 00:00 0
> 578f0000-57c00000 rw-p 00000000 00:00 0
> 57c00000-7fff97c00000 r--p 00000000 00:00 0
> 7fff97c00000-7fff97e20000 r-xp 00000000 fd:00 33558923
> 7fff97e20000-7fff97e30000 r--p 00220000 fd:00 33558923
> 7fff97e30000-7fff97e40000 rw-p 00230000 fd:00 33558923
> 7fff97f40000-7fff98020000 r-xp 00000000 fd:00 33558924
> 7fff98020000-7fff98030000 r--p 000d0000 fd:00 33558924
> 7fff98030000-7fff98040000 rw-p 000e0000 fd:00 33558924
> 7fff98050000-7fff98090000 r--p 00000000 00:00 0
> 7fff98090000-7fff980a0000 r-xp 00000000 00:00 0
> 7fff980a0000-7fff980f0000 r-xp 00000000 fd:00 2634
> 7fff980f0000-7fff98100000 r--p 00040000 fd:00 2634
> 7fff98100000-7fff98110000 rw-p 00050000 fd:00 2634
> 7fffcf8a0000-7fffcf9b0000 rw-p 00000000 00:00 0
> 1000000000000-1000040000000 r--p 00000000 00:00 0   --> High Addr
> 2000000000000-2000040000000 r--p 00000000 00:00 0
> 4000000000000-4000040000000 r--p 00000000 00:00 0
> 8000000000000-8000040000000 r--p 00000000 00:00 0
> e800098110000-fffff98110000 r--p 00000000 00:00 0
> $
>
> In this patch, the condition that checks for gaps smaller than MAP_CHUNK_SIZE has been removed.
>
> Fixes: d1d86ce28d0f ("selftests/mm: virtual_address_range: conform to TAP format output")
> Fixes: b2a79f62133a ("selftests/mm: virtual_address_range: unmap chunks after validation")
> Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()")
> Signed-off-by: Donet Tom <donettom@...ux.ibm.com>
> Signed-off-by: Aboorva Devarajan <aboorvad@...ux.ibm.com>
> ---
>   tools/testing/selftests/mm/virtual_address_range.c | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
> index b380e102b22f..606e601a8984 100644
> --- a/tools/testing/selftests/mm/virtual_address_range.c
> +++ b/tools/testing/selftests/mm/virtual_address_range.c
> @@ -80,7 +80,7 @@ static void validate_addr(char *ptr, int high_addr)
>   	if (high_addr && addr < HIGH_ADDR_MARK)
>   		ksft_exit_fail_msg("Bad address %lx\n", addr);
>   
> -	if (addr > HIGH_ADDR_MARK)
> +	if (!high_addr && addr > HIGH_ADDR_MARK)
>   		ksft_exit_fail_msg("Bad address %lx\n", addr);
>   }
>   
> @@ -117,7 +117,7 @@ static int validate_lower_address_hint(void)
>   
>   static int validate_complete_va_space(void)
>   {
> -	unsigned long start_addr, end_addr, prev_end_addr;
> +	unsigned long start_addr, end_addr;
>   	char line[400];
>   	char prot[6];
>   	FILE *file;
> @@ -134,7 +134,6 @@ static int validate_complete_va_space(void)
>   	if (file == NULL)
>   		ksft_exit_fail_msg("cannot open /proc/self/maps\n");
>   
> -	prev_end_addr = 0;
>   	while (fgets(line, sizeof(line), file)) {
>   		const char *vma_name = NULL;
>   		int vma_name_start = 0;
> @@ -151,12 +150,6 @@ static int validate_complete_va_space(void)
>   		if (start_addr & (1UL << 63))
>   			return 0;
>   
> -		/* /proc/self/maps must have gaps less than MAP_CHUNK_SIZE */
> -		if (start_addr - prev_end_addr >= MAP_CHUNK_SIZE)
> -			return 1;
> -
> -		prev_end_addr = end_addr;
> -
>   		if (prot[0] != 'r')
>   			continue;
>   
> @@ -223,8 +216,7 @@ int main(int argc, char *argv[])
>   
>   		if (hptr[i] == MAP_FAILED)
>   			break;
> -
> -		mark_range(ptr[i], MAP_CHUNK_SIZE);
> +		mark_range(hptr[i], MAP_CHUNK_SIZE);
>   		validate_addr(hptr[i], 1);
>   	}
>   	hchunks = i;

Powered by blists - more mailing lists