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

["David Hildenbrand (Arm)" <david@kernel.org>]

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)

> 
> 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) <ljs@kernel.org>
> ---

[...]

> +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));

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David