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

[David Hildenbrand <david@redhat.com>]

On 30.04.25 17:44, Wei Yang wrote:
> 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.

Sure, feel free to reach out. Having more selftests that test rmap 
behavior could be really helpful.

-- 
Cheers,

David / dhildenb