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 <[email protected]>
>>> Signed-off-by: Sayali Patil <[email protected]>
>>> ---
>>> 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) <[email protected]>
>>
>>
> 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
