Re: [PATCH v9 6/6] selftests/mm: add a GUP selftest

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

On 9/11/26 06:32, Sarthak Sharma wrote:
> Hi David!
> 
> On 9/9/26 10:33 PM, David Hildenbrand (Arm) wrote:
>>>
>>> This is an interesting change. We can keep this open for discussion
>>> here. If required, I can work on this in the future.
>>
>> Yes, we should in general try moving all test modules out of the core.
>>
>>>
>>> This is a pre requisite check. Every test opens and closes /dev/zero and
>>> /sys/kernel/debug/gup_test of its own. So I wanted to check before
>>> running the harness if these two are available, so that we don't have
>>> setup failures for 60 test cases.
>>
>> But why /dev/zero? We should understand why that would possibly be required.
> 
> This was carried over from the old test, where /dev/zero was the default
> backing for mmap unless the user selected another file to back the
> mapping. Now since we don't support file backed mappings, we can
> directly use MAP_ANONYMOUS here. Thanks for pointing it out, I'll remove
> it from this patch.
> 
>>
>>>
>>>
>>> mm selftests normally skip if the test is not run as root. So I tried
>>> keeping the same thing here. Do you think I should change it to fail?
>>
>> If other tests do that, it's fine!
>>
>> [...]
>>
>>>
>>> HUGETLB_TARGET_SIZE is the target mapping size and
>>> default_huge_page_size() gives the size of a single hugetlb page.
>>>
>>> Using default_huge_page_size() would reduce coverage for 2MB hugetlb
>>> pages. The old test set self->size to be 256 MB for the hugetlb case.
>>>
>>> Now it was discussed in a previous version of this patchset that we can
>>> derive the self->size for hugetlb case by fixing the nr_hugepages and
>>> multiplying by hugetlb size, and thought 128 would be a good number for
>>> nr_hugepages [1].
>>>
>>> But in case the hugetlb pages are very large, eg we can have 1 GB
>>> hugepages as well, reserving 128 GB is not a good idea. So I tried to
>>> keep the target size of the mapping as 256 MB, as it was before in the
>>> old gup test. If the hugetlb pages are larger than this, we'll reserve
>>> only one of them. Else, we'll reserve (256 MB /
>>> default_huge_page_size()) hugetlb pages, which comes out to be 128 for
>>> the case of 2MB hugetlb pages.
>> It's odd that 2M gets better test coverage than 512M or 1G.
>>
>> Is there really a lot of value in testing 128 2M pages? Would, like, 2 already
>> be good enough?
> 
> Yup, seems like keeping 128 pages is not adding an extra value. I'll go
> with 2 hugetlb pages to test both pinning within a hugetlb page and
> across the hugetlb boundary. This would make things a lot simpler.

Thanks!

-- 
Cheers,

David