Re: [PATCH 3/5] mm/rmap: refactor some code around lazyfree folio unmapping

[David Hildenbrand (Arm)]

On 7/7/26 14:11, Dev Jain wrote:
> For lazyfree folio unmapping, after clearing the ptes we must abort the
> operation if the folio got dirtied or it has unexpected references.
> 
> Refactor this logic into a function which will return whether we need
> to abort or not.
> 
> If we abort, we restore the ptes and bail out of try_to_unmap_one.
> Otherwise adjust the rss stats of the mm and jump to a label.
> 
> Also rename that label from "discard" to "finish_unmap"; the former
> is appropriate in the lazyfree context, but the code following the label
> is executed for other successful unmap code paths too, so 'discard' does
> not sound correct for them.
> 
> No functional change intended.
> 
> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
> ---
>  mm/rmap.c | 87 +++++++++++++++++++++++++++++++------------------------
>  1 file changed, 49 insertions(+), 38 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 934773dfa2f2a..00b571c2a1bab 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2067,6 +2067,52 @@ static bool try_to_unmap_hugetlb_one(struct folio *folio,
>  	return ret;
>  }
>  
> +static inline bool ttu_anon_lazyfree_folio(struct vm_area_struct *vma,
> +		struct folio *folio)
> +{
> +	int ref_count, map_count;
> +
> +	/*
> +	 * Synchronize with gup_pte_range():
> +	 * - clear PTE; barrier; read refcount
> +	 * - inc refcount; barrier; read PTE
> +	 */
> +	smp_mb();
> +
> +	ref_count = folio_ref_count(folio);
> +	map_count = folio_mapcount(folio);
> +
> +	/*
> +	 * Order reads for page refcount and dirty flag
> +	 * (see comments in __remove_mapping()).
> +	 */
> +	smp_rmb();
> +
> +	if (folio_test_dirty(folio) && !(vma->vm_flags & VM_DROPPABLE)) {
> +		/*
> +		 * redirtied either using the page table or a previously
> +		 * obtained GUP reference.
> +		 */
> +		folio_set_swapbacked(folio);
> +		return false;
> +	}
> +
> +	if (ref_count != 1 + map_count) {
> +		/*
> +		 * Additional reference. Could be a GUP reference or any
> +		 * speculative reference. GUP users must mark the folio
> +		 * dirty if there was a modification. This folio cannot be
> +		 * reclaimed right now either way, so act just like nothing
> +		 * happened.
> +		 * We'll come back here later and detect if the folio was
> +		 * dirtied when the additional reference is gone.
> +		 */
> +		return false;
> +	}
> +
> +	return true;

You could simply do


/*
 * Additional references could be due to GUP or from a speculative
 * lookup. ...
 */
return ref_count == 1 + map_count;


Nothing else jumped at me

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David