Re: [PATCH] selftests/mm: add test for (BATCH_PROCESS)MADV_DONTNEED

[Lorenzo Stoakes]

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
>