Re: [PATCH v3 09/15] mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios

On 7/29/26 18:48, Lorenzo Stoakes (ARM) wrote:
> Currently anonymous folios belonging to CoW'd MAP_PRIVATE file-backed
> mappings are indexed by their page offset within the file in which they
> were originally mapped.
> 
> This differs from anonymous folios belonging to pure anon mappings which
> are indexed by their anonymous page offset (the address at which they'd
> belong in the VMA when first faulted).
> 
> This change fixes this inconsistency, always indexing anonymous folios by
> their anonymous page offset regardless of the VMA to which they belong.
> 

Yes, this is great.

> The foundations have been laid such that we need only switch this
> functionality on such by:
> 
> * Using linear_anon_page_index() in __folio_set_anon() to assign the
>   folio's index to the anonymous linear index rather than the file-backed
>   one.
> 
> * Otherwise using linear_anon_page_index() in all instances where
>   anonymous folios are being referenced or manipulated.
> 
> * Replacing vma_address() with vma_filebacked_address() or
>   vma_anon_address() as appropriate.
> 
> * Updating the merging logic to check that anonymous page offsets are
>   aligned as well as filebacked ones for MAP_PRIVATE file-backed VMAs,
>   introducing needs_adjacent_anon_pgoff() to figure out when this is
>   required.
> 
> * Updating linear_folio_page_index() to invoke linear_anon_page_index()
>   if the folio is anonymous.
> 
> * Updating vma_address_end() to use the VMA's anonymous page offset when
>   pvmw->pgoff is anonymous.
> 
> * Correcting folio_within_range() to use anonymous page offset for
>   anonymous folios.
> 
> This will have no impact on merging of anonymous VMAs, whose page offset
> and anonymous page offset are identical, nor will it impact shared
> file-backed VMAs, which will continue to be merged based on the file-backed
> page offset.
> 
> However, MAP_PRIVATE file-backed mappings must now be aligned on anonymous
> page offset as well.
> 
> In most instances this should have no impact on merging of file-backed
> mappings, which are usually not merged all that often, let alone
> MAP_PRIVATE mapped ones, and rarely remapped and faulted before being moved
> back in place (the case in which a merge may now fail).

Agreed.

[...]

>  
>  /**
> - * vma_address - Find the virtual address a page range is mapped at.
> - * @vma: The vma which maps this object.
> - * @pgoff: The page offset within its object.
> - * @nr_pages: The number of pages to consider.
> - *
> - * If any page in this range is mapped by this VMA, return the first address
> - * where any of these pages appear.  Otherwise, return -EFAULT.
> - */
> -static inline unsigned long vma_address(const struct vm_area_struct *vma,
> -		pgoff_t pgoff, unsigned long nr_pages)
> -{
> -	return __vma_address(vma, pgoff, vma_start_pgoff(vma), nr_pages);
> -}

Good to see this go.
[...]

>  	rmap_item->anon_vma = vma->anon_vma;
> -	rmap_item->linear_page_index = linear_page_index(vma, rmap_item->address);
> +	/* The VMA is always anon/MAP_PRIVATE-file backed so use anon index. */
> +	rmap_item->linear_page_index = linear_anon_page_index(vma, rmap_item->address);

You can drop this comment: It's an anon_vma + linear_page_index pair, so the
anon_page_index is implied.

[...]

>  	WRITE_ONCE(folio->mapping, (struct address_space *) anon_vma);
> -	folio->index = linear_page_index(vma, address);
> +	folio->index = linear_anon_page_index(vma, address);


Apart from the LGTM.

-- 
Cheers,

David