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
