Re: [PATCH v3 06/15] mm: propagate VMA anonymous page offset on map, remap, split + merge

On Mon, Aug 03, 2026 at 12:52:42PM +0200, David Hildenbrand (Arm) wrote:
> On 7/29/26 18:48, Lorenzo Stoakes (ARM) wrote:
> > We must correctly update VMA anonymous page offset state on all VMA
> > operations that would result in it changing, with special attention given
> > to remapping.
> >
> > We cover most cases by simply updating vma_set_range() to do so (with a new
> > anonymous page offset parameter), but also notably must update the merging
> > and mapping logic to propagate this parameter correctly.
> >
> > The remap logic remains the same - we may update the anonymous page offset
> > if the VMA is unfaulted, but now this applies to MAP_PRIVATE file-backed
> > mappings too, so we update the code to reflect this.
> >
> > Note that we use __linear_anon_page_index() upon remap as the VMA may be
> > shared, in order that we update the field consistently regardless of VMA
> > type.
> >
> > Similarly, pass through anon page offset to the merge logic, updating the
> > vma_merge_struct struct to propagate it, and also use
> > __linear_anon_page_index() to obtain the anonymous page index so it can be
> > safely used for both shared and MAP_PRIVATE file-backed mappings.
> >
> > Finally, we update insert_vm_struct() to correctly set the anonymous page
> > offset on insertion of a VMA.
> >
> > We simply ensure state is correctly propagated here, so no functional
> > changes are intended.
> >
> > Also while we're here, replace a VM_BUG_ON_VMA() with a
> > VM_WARN_ON_ONCE_VMA().
> >
> > Also update VMA userland tests to reflect this change.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
>
>
> [...]
>
> >  	struct vm_area_struct *vma = *vmap;
> >  	unsigned long vma_start = vma->vm_start;
> > @@ -1919,11 +1929,14 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
> >  	VMG_VMA_STATE(vmg, &vmi, NULL, vma, addr, addr + len);
> >
> >  	/*
> > -	 * If anonymous vma has not yet been faulted, update new pgoff
> > -	 * to match new location, to increase its chance of merging.
> > +	 * If a vma has not yet been faulted, update its anonymous pgoff to
> > +	 * match the new location to increase its chance of merging.
> >  	 */
> > -	if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma)) {
> > -		pgoff = addr >> PAGE_SHIFT;
> > +	if (!vma->anon_vma && !vma_test(vma, VMA_SHARED_BIT)) {
>
> Could we also use is_cow_mapping() ?

No this would be incorrect.

A read-only mapping would become unmergeable here. So this is something apart
from the rmap aspect, and it is a contract that upon move of an unfaulted
mapping (which for read-only anon would always be unfaulted) that vma->vm_pgoff
is updated.

This is later in this series extended to MAP_PRIVATE file-backed mappings also.

Also it would break the faulted_in_anon_vma logic and result in incorrect
VM_WARN_ON_ONCE() invocations below.

As for other -> is_cow_mapping() changes:

* linear_anon_page_index() - [as discussed already] - fine
* vma_anon_address() - convert to is_cow_mapping() [have to be faulted in to hit it]
* needs_adjacent_anon_pgoff() - don't convert - same merging argument as above.

>
>
> > +		anon_pgoff = addr >> PAGE_SHIFT;
> > +
> > +		if (vma_is_anonymous(vma))
> > +			pgoff = anon_pgoff;
> >  		faulted_in_anon_vma = false;
> >  	}
> >
>
> Acked-by: David Hildenbrand (Arm) <[email protected]>

Thanks!

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo