Re: [PATCH v3 07/13] selftest/mm: register existing mapping with userfaultfd in hugepage-mremap

["David Hildenbrand (Arm)" <david@kernel.org>]

On 4/1/26 16:43, Sayali Patil wrote:
> 
> On 01/04/26 19:48, David Hildenbrand (Arm) wrote:
>> On 3/27/26 08:16, Sayali Patil wrote:
>>> Previously, register_region_with_uffd() created a new anonymous
>>> mapping and overwrote the address supplied by the caller before
>>> registering the range with userfaultfd.
>>>
>>> As a result, userfaultfd was applied to an unrelated anonymous mapping
>>> instead of the hugetlb region used by the test.
>>>
>>> Remove the extra mmap() and register the caller-provided address range
>>> directly using UFFDIO_REGISTER_MODE_MISSING, so that faults are
>>> generated for the hugetlb mapping used by the test.
>>>
>>> This ensures userfaultfd operates on the actual hugetlb test region and
>>> validates the expected fault handling.
>>>
>>> Before patch:
>>>  running ./hugepage-mremap
>>>  -------------------------
>>>  TAP version 13
>>>  1..1
>>>   Map haddr: Returned address is 0x7eaa40000000
>>>   Map daddr: Returned address is 0x7daa40000000
>>>   Map vaddr: Returned address is 0x7faa40000000
>>>   Address returned by mmap() = 0x7fff9d000000
>>>   Mremap: Returned address is 0x7faa40000000
>>>   First hex is 0
>>>   First hex is 3020100
>>>  ok 1 Read same data
>>>  Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>  [PASS]
>>>  ok 1 hugepage-mremap
>>>
>>> After patch:
>>>  running ./hugepage-mremap
>>>  -------------------------
>>>  TAP version 13
>>>  1..1
>>>   Map haddr: Returned address is 0x7eaa40000000
>>>   Map daddr: Returned address is 0x7daa40000000
>>>   Map vaddr: Returned address is 0x7faa40000000
>>>   Registered memory at address 0x7eaa40000000 with userfaultfd
>>>   Mremap: Returned address is 0x7faa40000000
>>>   First hex is 0
>>>   First hex is 3020100
>>>  ok 1 Read same data
>>>  Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>  [PASS]
>>>  ok 1 hugepage-mremap
>> Okay, so we tested mremap() of something that is not even hugetlb.
>>
>>> Fixes: 12b613206474 ("mm, hugepages: add hugetlb vma mremap() test")
>>> Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
>>> Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
>>> ---
>>>  tools/testing/selftests/mm/hugepage-mremap.c | 21 +++++---------------
>>>  1 file changed, 5 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/hugepage-mremap.c b/tools/testing/selftests/mm/hugepage-mremap.c
>>> index b8f7d92e5a35..e611249080d6 100644
>>> --- a/tools/testing/selftests/mm/hugepage-mremap.c
>>> +++ b/tools/testing/selftests/mm/hugepage-mremap.c
>>> @@ -85,25 +85,14 @@ static void register_region_with_uffd(char *addr, size_t len)
>>>  	if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1)
>>>  		ksft_exit_fail_msg("ioctl-UFFDIO_API: %s\n", strerror(errno));
>>>  
>>> -	/* Create a private anonymous mapping. The memory will be
>>> -	 * demand-zero paged--that is, not yet allocated. When we
>>> -	 * actually touch the memory, it will be allocated via
>>> -	 * the userfaultfd.
>>> -	 */
>>> -
>>> -	addr = mmap(NULL, len, PROT_READ | PROT_WRITE,
>>> -		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>> -	if (addr == MAP_FAILED)
>>> -		ksft_exit_fail_msg("mmap: %s\n", strerror(errno));
>>> -
>>> -	ksft_print_msg("Address returned by mmap() = %p\n", addr);
>>> -
>>> -	/* Register the memory range of the mapping we just created for
>>> -	 * handling by the userfaultfd object. In mode, we request to track
>>> -	 * missing pages (i.e., pages that have not yet been faulted in).
>>> +	/* Register the passed memory range for handling by the userfaultfd object.
>> /*
>>  * ...
>>
>> While at it.
>>
>>> +	 * In mode, we request to track missing pages
>>> +	 * (i.e., pages that have not yet been faulted in).
>>>  	 */
>>>  	if (uffd_register(uffd, addr, len, true, false, false))
>>>  		ksft_exit_fail_msg("ioctl-UFFDIO_REGISTER: %s\n", strerror(errno));
>>> +
>>> +	ksft_print_msg("Registered memory at address %p with userfaultfd\n", addr);
>>>  }
>>>  
>>>  int main(int argc, char *argv[])
>> Yes, that code is extremely weird. I wonder if this was some
>> copy-and-paste from other uffd test code.
>>
>> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
>>
>>
> Hi David,
> 
> Yes, the test operates on hugetlb mappings created with
> |MAP_HUGETLB | MAP_POPULATE|and sets up userfaultfd. Consequently,
> registering it with |UFFDIO_REGISTER_MODE_MISSING| does not result in
> any userfaults.
> 
> Originally, the helper function created a separate anonymous mapping and
> registered it with userfaultfd instead of the address supplied by the
> caller. However, the test operates on hugetlb mappings, and the registered
> anonymous mapping is never used in the |mremap()| path being exercised.
> 
> Would it be better to remove userfaultfd registration entirely from this
> test, since that path is not actually being tested?

If it's tested with your change now (which I think that's what
happenes), this is fine.

It was just very weird before, because it tested something fairly unrelated.

-- 
Cheers,

David