Re: [RFC Patch 0/5] Make anon_vma operations testable

[Wei Yang <richard.weiyang@gmail.com>]

On Wed, Apr 30, 2025 at 09:47:16AM +0200, David Hildenbrand wrote:
>On 30.04.25 01:56, Wei Yang wrote:
>> On Tue, Apr 29, 2025 at 10:41:27AM +0100, Lorenzo Stoakes wrote:
>> > On Tue, Apr 29, 2025 at 11:38:23AM +0200, David Hildenbrand wrote:
>> > > On 29.04.25 11:31, Lorenzo Stoakes wrote:
>> > > > Wei,
>> > > > 
>> > > > NACK the whole series.
>> > > > 
>> > > > I'm really not sure how to get through to you. You were _explicitly_
>> > > > advised not to send this series. And yet you've sent it anyway.
>> > > > 
>> > > > I mean, I appreciate your enthusiasm and the fact you've made tests here
>> > > > etc. obviously. And you've clearly put a TON of work in. But I just don't
>> > > > know why you would when explicitly told not to without at least discussing
>> > > > it first?
>> > > > 
>> > > > This just isn't a great way of interacting with the community. We're all
>> > > > human, please try to have some empathy for others here, as I really do try
>> > > > to have with you as best I can.
>> > > > 
>> > > > This adds a ton of churn and LOCKS IN assumptions about how anon_vma works,
>> > > > clashes with other series (most notably series I've been working on), takes
>> > > > away from efforts I want to make to start to join file-backed and anon
>> > > > reverse mapping logic, separates the two in such a way as to encourage this
>> > > > to nonly grow and generally isn't conducive to where I want to go with
>> > > > rmap.
>> > > 
>> > > anon_vma, the unloved child. :)
>> > > 
>> > > I would love to see a simplification that makes it less special, and I can
>> > > understand how adding tests for the ways it is special can be
>> > > counter-productive.
>> > > 
>> > > > 
>> > > > This is part of why I explicitly told you please don't go down this road,
>> > > > because you're likely to end up doing work that doesn't get used. It's not
>> > > > a great use of your time either.
>> > > > 
>> > > > Since there's something useful here in tests, I may at a later date come
>> > > > back to those.
>> > > 
>> > > Agreed, skimming over the tests there are some nice diagrams and cases.
>> > > 
>> > > But I would hope that for most of these cases we could test on a higher
>> > > level: test our expectations when running real programs that we want to
>> > > check, especially when performing internal changes on how we handle anon
>> > > memory + rmap.
>> > > 
>> > > E.g., do fork(), then test if we can successfully perform rmap
>> > > lookups/updates (e.g., migrate folio to a different numa node etc).
>> > > 
>> > 
>> > That's a great point! Wei - if you could look at making some self-tests
>> > (i.e. that live in tools/testing/selftests/mm) that try to recreate _real_
>> > scenarios that use the rmap like this and assert correct behaviour there,
>> > that could be a positive way of moving forward with this.
>> > 
>> 
>> I am trying to understand what scenarios you want.
>
>That is exactly the task to figure out: how can we actually test our rmap
>implementation from a higher level. The example regarding fork and migration
>is possibly a low-hanging fruit.
>
>We might already have the functionality to achieve it, *maybe* we'd even want
>some extensions to make it all even easier to test.
>
>For example, MADV_PAGEOUT is refused on folios that are mapped into multiple
>processes. Maybe we'd want the option to *still* page it out, just like
>MPOL_MF_MOVE_ALL allows with CAP_SYS_NICE to *still* migrate a folio that is
>mapped into multiple processes.
>
>Some rmap tests could make sense for both, anon and pagecache folios.
>
>> 
>> Something like below?
>> 
>>    * fork and migrate a range in child
>>    * fork/unmap in parent and migrate a range in child
>> 
>> If the operation is successful, then we are good, right?
>
>Yes. And one can come up with a bunch of similar rmap test cases, like doing
>a partial mremap() of a THP, then testing if the rmap walk still works as
>expected, pairing the whole thing with for etc.
>
>One "problem" here is that even with MPOL_MF_MOVE_ALL,
>move_pages() will not move a folio if it already resides on the target node.
>So one always needs two NUMA nodes, which is a bit suboptimal for testing
>purposes.
>
>For testing purposes, it could have been helpful a couple of times already to
>just have a way of migrating a folio even if it already resides on the
>expected node.
>

Thanks for all those detail explanation. I need some time to digest it.

Since lack of some background knowledge, I may have further questions on this.
Hope won't bother you too much.

>-- 
>Cheers,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me