Re: [PATCH RFC] mm: userfaultfd: correct dirty flags set for both present and swap pte

[David Hildenbrand]

On 08.05.25 11:07, Barry Song wrote:
> From: Barry Song <v-songbaohua@xxxxxxxx>
>
> As David pointed out, what truly matters for mremap and userfaultfd
> move operations is the soft dirty bit. The current comment and
> implementation—which always sets the dirty bit for present PTEs
> and fails to set the soft dirty bit for swap PTEs—are incorrect.
> This patch updates the behavior to correctly set the soft dirty bit
> for both present and swap PTEs in accordance with mremap.
>
> Reported-by: David Hildenbrand <david@xxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-mm/02f14ee1-923f-47e3-a994-4950afb9afcc@xxxxxxxxxx/
> Cc: Peter Xu <peterx@xxxxxxxxxx>
> Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Cc: Lokesh Gidra <lokeshgidra@xxxxxxxxxx>
> Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
> ---
>   mm/userfaultfd.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index e8ce92dc105f..bc473ad21202 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -1064,8 +1064,13 @@ static int move_present_pte(struct mm_struct *mm,
>   	src_folio->index = linear_page_index(dst_vma, dst_addr);
>   
>   	orig_dst_pte = folio_mk_pte(src_folio, dst_vma->vm_page_prot);
> -	/* Follow mremap() behavior and treat the entry dirty after the move */
> -	orig_dst_pte = pte_mkwrite(pte_mkdirty(orig_dst_pte), dst_vma);
> +	/* Set soft dirty bit so userspace can notice the pte was moved */
> +#ifdef CONFIG_MEM_SOFT_DIRTY
> +	orig_dst_pte = pte_mksoft_dirty(orig_dst_pte);
> +#endif
> +	if (pte_dirty(orig_src_pte))
> +		orig_dst_pte = pte_mkdirty(orig_dst_pte);
> +	orig_dst_pte = pte_mkwrite(orig_dst_pte, dst_vma);
>   
>   	set_pte_at(mm, dst_addr, dst_pte, orig_dst_pte);
>   out:
> @@ -1100,6 +1105,9 @@ static int move_swap_pte(struct mm_struct *mm, struct vm_area_struct *dst_vma,
>   	}
>   
>   	orig_src_pte = ptep_get_and_clear(mm, src_addr, src_pte);
> +#ifdef CONFIG_MEM_SOFT_DIRTY
> +	orig_src_pte = pte_swp_mksoft_dirty(orig_src_pte);
> +#endif
>   	set_pte_at(mm, dst_addr, dst_pte, orig_src_pte);
>   	double_pt_unlock(dst_ptl, src_ptl);

Yeah, I think that should be the right thing to do.

--
Cheers,

David / dhildenb