[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20b9d059-6d10-43b8-90c8-cd46330c2326@lucifer.local>
Date: Mon, 23 Jun 2025 14:56:07 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: wang lian <lianux.mm@...il.com>, Andrew Morton <akpm@...ux-foundation.org>,
Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
linux-kselftest@...r.kernel.org, SeongJae Park <sj@...nel.org>,
zijing.zhang@...ton.me, ryncsn@...il.com, p1ucky0923@...il.com,
gkwang@...x-info.com, "Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH] selftests/mm: add test for (BATCH_PROCESS)MADV_DONTNEED
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
>
Powered by blists - more mailing lists