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

On 8/3/26 15:46, Lorenzo Stoakes (ARM) wrote:
> 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,

I'd assume that we should never even consider anon_pgoff when merging
!is_cow_mapping(), it doesn't make any sense.

No anon folios -> no anon_vma -> no anon_pgoff

But I think I am missing one detail here:

> 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.

"read-only anon": I assume you mean an anon mapping that does not have
VM_MAYWRITE set?

I recall that that's a combination that cannot be created. While you can create
something that does not have VM_WRITE set, IIRC VM_MAYWRITE is always set for
anon vmas.

-- 
Cheers,

David