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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Cheers,

David / dhildenb