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

[Sayali Patil <sayalip@linux.ibm.com>]

On 02/04/26 13:01, David Hildenbrand (Arm) wrote:
> 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.
> 
Thanks for the review. Yes, tested with this change and it behaves as 
expected now.