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

On Wed, Aug 05, 2026 at 11:19:20AM +0200, David Hildenbrand (Arm) wrote:
> On 8/5/26 10:59, Lorenzo Stoakes (ARM) wrote:
> > On Wed, Aug 05, 2026 at 09:35:56AM +0200, David Hildenbrand (Arm) wrote:
> >> On 8/3/26 15:46, Lorenzo Stoakes (ARM) wrote:
> >>>
> >>> 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
> >
> > You can merge unfaulted ranges is the thing here.
> >
> > But anyway I actually wonder whether this whole branch shouldn't be:
> >
> > 	if (!vma->anon_vma) {
> > 		...
> > 	}
> >
> > Because that way we keep anon_pgoff updated even for MAP_SHARED mappings. This
> > isn't necessary and doesn't impact anything _except_ print_bad_page_map which
> > outputs both pgoffs.
>
> Agreed.
>
> >
> > But it'd be consistent, avoid any confusion about gating on VMA_SHARED, and
> > simplify the code :)
> >
> >>
> >> 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?
> >
> > A MAP_SHARED mapping of a read-only file becomes a MAP_PRIVATE !VMA_MAYWRITE_BIT
> > mapping and must adhere to the same contract.
>
> Right, but that is not an anon mapping, it's a file mapping that similarly
> cannot have anon folios, ever.
>
> >
> > Also mmap hooks can clear the VMA_MAYWRITE_BIT.
>
> Right, but again, if we'd have that being done to anon mappings, other things in
> MM would already be broken.
>
> We assume that anon folios can only ever end up in cow mappings.
>
> >
> > However:
> >
> > - If you're a driver clearing VMA_MAYWRITE_BIT you should only be doing this for
> >   'special' mappings anyway (I have a series I've not sent yet that establishes
> >   this as an invariant also) - and these are not mergeable anyway.
>
> Jup.
>
> >
> > - If you're a !VMA_MAYWRITE_BIT MAP_PRIVATE-file backed mappings you never set
> >   vma->anon_vma and always update anon pgoff so you always have alignment for
> >   purposes of merge.
>
> Jup.
>
> >
> > So I think also we can then change needs_adjacent_anon_pgoff() to:
> >
> > static bool needs_adjacent_anon_pgoff(const struct vma_merge_struct *vmg)
> > {
> > 	return vmg->file && is_cow_mapping(...);
> > }
>
> Agreed.
>
> >
> > [I have to create a vma_flags_t variant of is_cow_mapping()]
> >
> > With those two changes we gate on VMA_SHARED_BIT nowhere :)
>
> That's much clearer.

Yes agreed :)

>
> I was thinking for a second whether to have a more expressive
> "mapping_might_have_anon_folio" or sth like that. But it's a bit mouthful.
>
> Most instances of is_cow_mapping() in memory.c want to know exactly that.

Yeah and obviously once scalable CoW is in even that name will be wrong :)

I think the semantics are a non-obvious but I do like that we are explicit in
saying 'is a CoW mapping'. We just maybe need to be more consistent with that.

Maybe once scalable CoW is in can work to improve consistency of that...

>
> >
> >>
> >> 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.
> >
> > For pure anon yeah, see above for the MAP_SHARED->MAP_PRIVATE-file backed weird
> > case.
> >
>
> Agreed.

Cool will make the changes as above then! :)

>
> >>
> >> --
> >> Cheers,
> >>
> >> David
> >
> > (It's funny to me that if you want a truly read-only MAP_PRIVATE file-backed
> > mapping (no idea why you would but anyway) you have to MAP_SHARED, but an
> > actually MAP_PRIVATE file-backed mapping of a read-only file is writable [which
> > makes sense obviously] :)
> I guess this boils down to
>
> MAP_PRIVATE of a read-only file allows you to COW. Which is usually what you
> want when placing breakpoints / letting the debugger go wild.
>
> MAP_SHARED of a read-only file doesn't allow you to COW, and can consequently
> never become writable.
>
> It's confusing, yes.

Yeah indeed, it's just amusing to think about, but in practice makes perfect
sense.

Though honestly I wish we didn't remove VMA_SHARED_BIT in this situation and
then rely on the hugely-confusingly-named VMA_MAY_SHARE_BIT for stuff where we
_really_ need to figure out whether it was mapped shared.

But that's another thing :)

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo