Re: [PATCH v3 01/15] mm/vma: introduce VMA anon page offset field and add helpers

On Mon, Aug 03, 2026 at 11:28:28AM +0200, David Hildenbrand (Arm) wrote:
> On 7/29/26 18:48, Lorenzo Stoakes (ARM) wrote:
> > This patch establishes fields within the vm_area_struct type to store the
> > anonymous page offset of VMAs.
>
> Nit: Avoid "This patch". Just say "Establish ..." (I was surprised to find
> that even Documentation/process/submitting-patches.rst document this)

Yeah ever since Boris nagged me I've (possibly reluctantly? :P) found that using
the active voice is a lot clearer and have been using that more so.

Will fix up on respin.

>
> >
> > The anonymous page offset of a VMA is equal to vma->vm_start >> PAGE_SHIFT
> > if they are unfaulted or were not remapped, otherwise it is equal to this
> > value at the point of first fault.
> >
> > Currently, anonymous folios belonging to CoW'd MAP_PRIVATE-mapped
> > file-backed VMAs are tracked by their file offset. By adding anonymous
> > offset as a property of VMAs, we can now track them by their anonymous page
> > offset instead.
> >
> > By tracking this, we provide the means by which to eliminate this
> > inconsistency, and more importantly lay the foundations for future work for
> > the scalable CoW anonymous rmap rework.
> >
> > This patch simply adds the fields and some simple helpers. Subsequent
> > patches will update mm code to make use of these fields correctly.
> >
> > The fields chosen are packed in the VMA such that, for 64-bit kernel
> > builds, no additional space is taken up.
> >
> > The first field is present on cacheline 0 containing key VMA fields, and
> > the second on cacheline 3, which contains file-backed reverse mapping
> > fields.
> >
> > Given the relative time spent accessing reverse mapping fields as well as
> > updating them, there shouldn't be any performance impact here from false
> > sharing.
> >
> > Update the VMA userland tests to account for this change.
> >
> > No callsites are updated yet, so no functional change intended.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
> > ---
>
> [...]
>
> > +static inline void __vma_set_anon_pgoff(struct vm_area_struct *vma, pgoff_t pgoff)
> > +{
> > +#ifdef CONFIG_64BIT
> > +	vma->__vm_anon_pgoff_hi = pgoff >> 32;
> > +#endif
> > +	vma->__vm_anon_pgoff_lo = pgoff & GENMASK(31, 0);
> > +}
> > +
> > +static inline void vma_set_anon_pgoff(struct vm_area_struct *vma, pgoff_t pgoff)
> > +{
> > +	vma_assert_can_modify(vma);
> > +	__vma_set_anon_pgoff(vma, pgoff);
> > +}
> > +
> >  static inline void vma_add_pgoff(struct vm_area_struct *vma, pgoff_t delta)
> >  {
> >  	vma_assert_can_modify(vma);
> > diff --git a/mm/vma_init.c b/mm/vma_init.c
> > index 715feee283f0..baa7e82f47e3 100644
> > --- a/mm/vma_init.c
> > +++ b/mm/vma_init.c
> > @@ -51,6 +51,7 @@ static void vm_area_init_from(const struct vm_area_struct *src,
> >  	dest->vm_end = src->vm_end;
> >  	dest->anon_vma = src->anon_vma;
> >  	dest->vm_pgoff = vma_start_pgoff(src);
> > +	__vma_set_anon_pgoff(dest, vma_start_anon_pgoff(src));
>
> It would be cleaner if both interfaces would either contain "start" or not contain it.
>
> The former would be better
>
> 	__vma_set_start_anon_pgoff(dest, vma_start_anon_pgoff(src));

Ack that's fair, will update it.

>
> Acked-by: David Hildenbrand (Arm) <[email protected]>

Thanks!
>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo