Message-ID: <e40f8959-6f89-46e3-b2cb-bfe6de615c54@arm.com>
Date: Wed, 18 Jun 2025 16:05:22 +0530
From: Dev Jain <dev.jain@....com>
To: Donet Tom <donettom@...ux.ibm.com>
Cc: 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, linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, ritesh.list@...il.com
Subject: Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.


On 18/06/25 3:36 pm, Donet Tom wrote:
> On Mon, Jun 16, 2025 at 09:57:10PM +0530, Dev Jain wrote:
> Hi Dev
>
>> 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.
>>
> In this test, we first allocate 128TB of low memory and verify that
> the allocated area falls within the expected range.
>
> Next, we allocate memory at high addresses and check whether the
> returned addresses are within the specified limits. To allocate
> memory at a high address, we pass a hint address. This hint address
> is derived using HIGH_ADDR_SHIFT, which is set to 48 — corresponding
> to 256TB. So, we are requesting allocation at the 256TB address, and
> the memory is successfully allocated there. Since the low address
> region is allocated up to 128TB, there is a gap between the low address
> VMA and the high address VMA .
>
> Additionally, we use a random number to generate the hint address, so
> the actual allocated address will vary but will always be above 256TB.
>
>
> 10000000-10010000 r-xp 00000000 fd:05 134255559
> 10010000-10020000 r--p 00000000 fd:05 134255559
> 10020000-10030000 rw-p 00010000 fd:05 134255559
> 10022b80000-10022bb0000 rw-p 00000000 00:00 0
> 7fff5c000000-7fff9c000000 r--p 00000000 00:00 0
> 7fff9cb30000-7fff9ce40000 rw-p 00000000 00:00 0
> 7fff9ce40000-7fff9d070000 r-xp 00000000 fd:00 792355
> 7fff9d070000-7fff9d080000 r--p 00230000 fd:00 792355
> 7fff9d080000-7fff9d090000 rw-p 00240000 fd:00 792355
> 7fff9d090000-7fff9d170000 r-xp 00000000 fd:00 792358
> 7fff9d170000-7fff9d180000 r--p 000d0000 fd:00 792358
> 7fff9d180000-7fff9d190000 rw-p 000e0000 fd:00 792358
> 7fff9d1a0000-7fff9d1e0000 r--p 00000000 00:00 0
> 7fff9d1e0000-7fff9d1f0000 r-xp 00000000 00:00 0
> 7fff9d1f0000-7fff9d240000 r-xp 00000000 fd:00 792351
> 7fff9d240000-7fff9d250000 r--p 00040000 fd:00 792351
> 7fff9d250000-7fff9d260000 rw-p 00050000 fd:00 792351
> 7fffecfa0000-7fffecfd0000 rw-p 00000000 00:00 0
> 1000000000000-1000040000000 r--p 00000000 00:00 0           -> High address
> 2000000000000-2000040000000 r--p 00000000 00:00 0
> 4000000000000-4000040000000 r--p 00000000 00:00 0
> 8000000000000-8000040000000 r--p 00000000 00:00 0
> e80009d260000-fffff9d260000 r--p 00000000 00:00 0
>                      
>
> The high address we are getting is at 256TB because we are explicitly
> requesting it. The gap between the high address VMA and the previous VMA
> is large because the low memory allocation goes up to 128TB.
>
> If I understand correctly, this test is verifying whether the allocated
> address falls within the expected range, and validate_complete_va_space()
> is validating the allocated virtual address space.
>
> Why do we need to check whether the gap between two VMAs is within
> MAP_CHUNK_SIZE? Should it validate only the allocated VMAs?

If you were trying to say here "Shouldn't it validate only the allocated VMAs",
that is exactly what we are doing. By "allocated" we mean mmapped. And those
VMAs will be shown in /proc/self/maps. This test is verifying whether mmap
can exhaust a process' VA space or not, therefore (assume no high address)
the gap between any two consecutive VMAs must not be greater than MAP_CHUNK_SIZE,
for if it was, then mmap should have been able to find it and allocate it there,
reducing that gap.

Now, taking into consideration the high address thingy, since the test completely
exhaust the low and high VA space, the gap condition should separately hold on
the low and high VA space. But it may not hold at the low-high boundary. So
you can simply avoid the check when you detect for the first time that the VMA
you are reading from /proc/self/maps comes from the high VA space.

>
> Thanks
> Donet
>   
>> 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