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
