On Mon, Jun 23, 2025 at 03:49:05PM +0200, David Hildenbrand wrote: > On 23.06.25 14:35, Lorenzo Stoakes wrote: > > +cc Liam, David, Vlastimil, Jann > > > > (it might not be obvious from get_maintainers.pl but please cc > > maintainers/reviewers of the thing you are adding a test for, thanks!) > > > > Overall I'm not in favour of us taking this patch. > > > > There are a number of issues with it (explained inline below), but those > > aside, > > it seems to be: > > > > - Checking whether a simple anon buffer of arbitrary size is zapped by > > MADV_DONTNEED. > > > > - Printing out a dubious microbenchmark that seems to be mostly asserting > > that > > fewer sycalls are faster when using process_madvise() locally. > > > > And I'm struggling to see the value of that. > > We have other tests that should already severely break if MADV_DONTNEED > doesn't work ... but sure, we could think about more elaborate functional > tests when they provide a clear benefit. (zapping all kinds of memory types, > anon/ksm/huge zeropage/pagecache/hugetlb/ ... and using /proc/self/pagemap > to see if the page table mappings are already gone)
Yes right, exactly. > > I don't think we have a lot of process_madvise selftests, right? No and that's an issue. It'd be good to have some of those, but not as a benchmark. I think the only stuff we have right now is in the guard-region tests which I added to assert it worked with MADV_GUARD_INSTALL/REMOVE. It'd be good to have stuff that tested remote process stuff (tricky to set up, maybe test has to fork itself etc.) as well as the stuff I added allowing self-madvise(). But again we'd probably really want to find a way to exercise this stuff properly. > > hugtlb handling that was added recently is already tested to some degree in > hugetlb-madvise.c. > > In general, I'm not a fan of selftests that measure syscall performance ... :) > > > -- > Cheers, > > David / dhildenb >
