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

[David Hildenbrand]

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@xxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxx>
> > > > ---
> > > 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