Re: [PATCH] mm/rmap: Move anon_vma initialization to anon_vma_ctor()

[David Hildenbrand <david@redhat.com>]

On 16.04.25 14:59, Lorenzo Stoakes wrote:
> +cc David, Liam
> 
> On Wed, Apr 16, 2025 at 09:34:30AM +0800, Ye Liu wrote:
>>
>> 在 2025/4/15 19:28, Harry Yoo 写道:
>>> On Tue, Apr 15, 2025 at 05:25:48PM +0800, Ye Liu wrote:
>>>> From: Ye Liu <liuye@kylinos.cn>
>>>>
>>>> Currently, some initialization of anon_vma is performed in
>>>> anon_vma_alloc(). Move the initialization to anon_vma_ctor()
>>>> so that all object setup is handled in one place.
>>>>
>>>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>>>> ---
>>> NACK unless the patch explains how the object's initial state
>>> ('constructed state') is preserved between uses.
>>>
>>> anon_vma_ctor() is a slab constructor. That means it is called only once
>>> when a slab (folio) is allocated, and not called again when an anon_vma
>>> is allocated from an existing slab (folio). In other words it is not called
>>> everytime an object allocated via kmem_cache_alloc() interface.
>> Thank you for the feedback. You're absolutely right — I misunderstood
>> how the slab constructor (ctor) works. I had assumed it would be called
>> every time an object is allocated via kmem_cache_alloc(), but I now
>> realize it is only called once when a new slab is initialized, not on
>> every object allocation.
>>> This patch looks very dangerous to me and makes me question whether you
>>> tested it before submission.
>>>
>> Appreciate you catching this — and yes, I'll test it more thoroughly
>> before submitting other patches.
>>
>> Drop it.
>>
>> Thanks,
>> Ye
>>
> 
> I agree with Harry's assessment, also this is something that needs to be cc'd to
> other developers - I am more generally working on anon_vma at the moment and
> this is vma-adjacent even if slab-ish :)

I stumbled over this patch on linux-mm and agreed with Harry's 
assessment as well. :)

-- 
Cheers,

David / dhildenb