Re: [PATCH v2 1/8] mm/rmap: improve anon_vma_clone(), unlink_anon_vmas() comments, add asserts

[Lorenzo Stoakes]

On Wed, Jan 14, 2026 at 06:33:39PM +0000, Lorenzo Stoakes wrote:
> On Wed, Jan 14, 2026 at 08:14:23AM -0800, Suren Baghdasaryan wrote:
> > > + *
> > > + * So this function needs only traverse the anon_vma_chain and free each
> > > + * allocated anon_vma_chain.
> > > + */
> > > +static void cleanup_partial_anon_vmas(struct vm_area_struct *vma)
> > > +{
> > > +       struct anon_vma_chain *avc, *next;
> > > +       bool locked = false;
> > > +
> > > +       /*
> > > +        * We exclude everybody else from being able to modify anon_vma's
> > > +        * underneath us.
> > > +        */
> > > +       mmap_assert_locked(vma->vm_mm);
> > > +
> > > +       list_for_each_entry_safe(avc, next, &vma->anon_vma_chain, same_vma) {
> > > +               struct anon_vma *anon_vma = avc->anon_vma;
> > > +
> > > +               /* All anon_vma's share the same root. */
> > > +               if (!locked) {
> > > +                       anon_vma_lock_write(anon_vma);
> > > +                       locked = true;
> > > +               }
> > > +
> > > +               anon_vma_interval_tree_remove(avc, &anon_vma->rb_root);
> > > +               list_del(&avc->same_vma);
> > > +               anon_vma_chain_free(avc);
> > > +       }
> >
> > Are you missing "if (locked) anon_vma_unlock_write()" here?
>
> Ugh god yes. This is an annoying irrelevance as we eliminate it anyway later but
> I'll do a fix-patch to avoid bisection hazard.
>
> > You could also avoid using "locked" variable by setting anon_vma =
> > NULL initially and using "if (anon_vma)" as an equivalent of "if
> > (locked)"
>
> I know, I did this on purpose to be explicit. The compiler will optimise this away.
>
> Anyway it's moot as this gets removed later so I say let's keep it as it is.

Ugh yeah it's just broken, as we obviously don't save anon_vma. I just hated
that way of doing this but I guess I have no choice.

And find solace in this being an intermediate patch that gets yoinked in a later
one :)