[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c76429c3-2689-4722-99c5-f577af679aa4@arm.com>
Date: Tue, 22 Apr 2025 12:03:08 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Suren Baghdasaryan <surenb@...gle.com>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>,
Matthew Wilcox <willy@...radead.org>, Vlastimil Babka <vbabka@...e.cz>,
"Paul E . McKenney" <paulmck@...nel.org>, Jann Horn <jannh@...gle.com>,
David Hildenbrand <david@...hat.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
linux-kselftest@...r.kernel.org, linux-api@...r.kernel.org,
John Hubbard <jhubbard@...dia.com>, Juan Yescas <jyescas@...gle.com>,
Kalesh Singh <kaleshsingh@...gle.com>
Subject: Re: [PATCH 3/4] tools/selftests: expand all guard region tests to
file-backed
On 22/04/2025 11:47, Lorenzo Stoakes wrote:
> On Tue, Apr 22, 2025 at 11:37:57AM +0100, Ryan Roberts wrote:
>> On 13/02/2025 18:17, Lorenzo Stoakes wrote:
>>> Extend the guard region tests to allow for test fixture variants for anon,
>>> shmem, and local file files.
>>>
>>> This allows us to assert that each of the expected behaviours of anonymous
>>> memory also applies correctly to file-backed (both shmem and an a file
>>> created locally in the current working directory) and thus asserts the same
>>> correctness guarantees as all the remaining tests do.
>>>
>>> The fixture teardown is now performed in the parent process rather than
>>> child forked ones, meaning cleanup is always performed, including unlinking
>>> any generated temporary files.
>>>
>>> Additionally the variant fixture data type now contains an enum value
>>> indicating the type of backing store and the mmap() invocation is
>>> abstracted to allow for the mapping of whichever backing store the variant
>>> is testing.
>>>
>>> We adjust tests as necessary to account for the fact they may now reference
>>> files rather than anonymous memory.
>>
>> Hi Lorenzo,
>>
>> I'm getting a test failure in v6.15-rc3 on arm64:
>>
>> ----8<----
>> # RUN guard_regions.shmem.uffd ...
>> # guard-regions.c:1467:uffd:Expected ioctl(uffd, UFFDIO_REGISTER, ®) (-1) ==
>> 0 (0)
>> # uffd: Test terminated by assertion
>> # FAIL guard_regions.shmem.uffd
>> not ok 45 guard_regions.shmem.uffd
>> ----8<----
>>
>> The ioctl is returning EINVAL.
>
> Hm strange, that works fine <resists urge to say 'on my machine'> on x86-64. Is
> userfaultfd enabled in your config, to ask a silly question?
Yep, and the anon version of the test is passing, as are all the uffd tests.
>
> It'd be odd for this to vary depending upon arch.
>
> So a factor here is a _stupidity_ in the testing - does your system mount /tmp
> as tmpfs or an actual file system? As the test code unconditionally assumes /tmp
> is indeed going to get you a shmem file.
Ahh that's probably it. I'm on Ubuntu and it looks like /tmp is just a dir on
the rootfs (XFS in my case).
Forcing a tmpfs to /tmp solved it.
Looks like uffd-unit-tests (see shmem_allocate_area()) is just using memfd.
Would it be reasonable to take that approach? Or just use anon+shared via mmap?
Thanks,
Ryan
>
> It's shameful to be honest. But actually I suspect this more than anything
> else...
>
>>
>> [...]
>>
>>> @@ -1281,6 +1398,9 @@ TEST_F(guard_regions, uffd)
>>> struct uffdio_register reg;
>>> struct uffdio_range range;
>>>
>>> + if (!is_anon_backed(variant))
>>
>> Perhaps this should be filtering out shmem too? Although the manual for
>> userfaultfd implies that shmem is supported:
>
> Yeah it should work with it fine.
>
>>
>> """
>> Up to Linux 4.11, userfaultfd can be used only with anonymous private memory
>> mappings. Since Linux 4.11, userfaultfd can be also used with hugetlbfs and
>> shared memory mappings.
>> """
>>
>> But I'm not sure if that's referring specifically to UFFDIO_REGISTER_MODE_MISSING?
>>
>> Any ideas before I start debugging further?
>>
>> Thanks,
>> Ryan
>>
>>> + SKIP(return, "uffd only works on anon backing");
>>> +
>>> /* Set up uffd. */
>>> uffd = userfaultfd(0);
>>> if (uffd == -1 && errno == EPERM)
>>> @@ -1290,8 +1410,8 @@ TEST_F(guard_regions, uffd)
>>> ASSERT_EQ(ioctl(uffd, UFFDIO_API, &api), 0);
>>>
>>> /* Map 10 pages. */
>>> - ptr = mmap(NULL, 10 * page_size, PROT_READ | PROT_WRITE,
>>> - MAP_ANON | MAP_PRIVATE, -1, 0);
>>> + ptr = mmap_(self, variant, NULL, 10 * page_size,
>>> + PROT_READ | PROT_WRITE, 0, 0);
>>> ASSERT_NE(ptr, MAP_FAILED);
>>>
>>> /* Register the range with uffd. */
>>
Powered by blists - more mailing lists