Re: [PATCH] mm/vma: avoid redundant file rmap tree re-insert on new_below=0 split

[Lorenzo Stoakes (ARM)]

On Fri, Sep 25, 2026 at 04:43:37PM +0100, Pedro Falcato wrote:
> On Fri, Sep 25, 2026 at 04:27:44PM +0100, Lorenzo Stoakes (ARM) wrote:
> > > This change skips the re-insert for that case.  vma_prepare() no longer
> > > removes vp->vma from the tree; instead vma_complete() detects that case and
> > > only recomputes shared.rb_subtree_last up the ancestor chain.  Everything
> > > else keeps the remove + re-insert path.
> >
> > This could really do with a diagram and a simple explanation.
> >
> > In general you should rewrite the entire commit message yourself and not
> > use the LLM output at all.
>
> +1 on this. Even with the Assisted-by, this needs to be understandable by
> hoomans.

Yes.

> > >
> > > Measured on v7.3-rc4, on a 2-socket 192C/384T system running UnixBench
> > > execl (384 concurrent execve of the same binary), dropping the redundant
> > > remove + re-insert yields ~14% higher throughput by shortening the
> > > i_mmap_rwsem write-side critical section during the file VMA splits that
> > > execve performs on the shared libraries.
>
> For what it's worth, I'm vaguely accepting of a similar change, but this needs
> to be _really_ well commented out, and ideally in file rmap code, _not_
> spaghetti'd in VMAs. The interval tree is complicated and some bits are not
> very intuitive. This needs to be robust. Not LLM'd into existence.
>
> This also reminds me that I should reboot the sharded file rmap effort...

Indeed, which is why I'm treating this as a report rather than a patch.

A member of the core team can do the actual work.

>
> >
> > This is really unconvincing I'm sorry. Real numbers please with statistical
> > evidence to back them.
> >
> > Additionally I notice you have not made one comment referring to locking
> > anywhere.
> >
> > This part of the kernel has VERY subtle and sensitive locking
> > requirements. I am not convinced you understand this, either.
> >
> > >
> > > Signed-off-by: Pan Deng <pan.deng@xxxxxxxxx>
> >
> > This patch feels like a hack. You are creating a whole new set of very
> > fragile assumptions that have to be maintained throughout.
> >
> > Again as above, a member of the core team should take this over.
> >
> > > Reviewed-by: Tianyou Li <tianyou.li@xxxxxxxxx>
> > > Reviewed-by: Wangyang Guo <wangyang.guo@xxxxxxxxx>
> > > Reviewed-by: Zhiguo Zhou <zhiguo.zhou@xxxxxxxxx>
> > > Reviewed-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
> >
> > Please don't do this.
> >
> > Upstream is not interested in private reviews. Review tags upstream are
> > based on review done in _public_.
>
> Yeah, this too. It's just noise.

Yes!

>
> --
> Pedro

--
Cheers, Lorenzo