Re: [PATCH] mm/gup: batch PTE-mapped large folios in gup_fast_pte_range()

[Yeoreum Yun]

On Mon, Oct 12, 2026 at 09:05:55PM +0200, David Hildenbrand (Arm) wrote:
> On 10/12/26 19:58, Rik van Riel wrote:
> > On Sun, 2026-10-11 at 03:52 +0800, Yuan-Hao Hsu wrote:
> >> On Wed, 07 Oct 2026 06:47:06 -0400, Rik van Riel wrote:
> >>> Since this is lockless, the PTEs could change under
> >>> us between when we walk them in folio_pte_batch_flags()
> >>> and the batched try_grab_folio().
> >>>
> >>> Is that race condition safe, or do we need to re-check
> >>> afterward that:
> >>> 1) the ptes still point at the same folio, and/or
> >>> 2) the folio still spans the entire range, and was
> >>>    not split before we grabbed the refcount?
> >>
> >> It is safe, because the batch only runs after the first page of the
> >> run has gone through the unchanged per-page path: the folio reference
> >> is already held (and for FOLL_PIN the _pincount already raised, with
> >> the smp_mb__after_atomic() in try_grab_folio_fast()) before any of
> >> the other ptes is read.
> >>
> >> 2) With that reference held the folio cannot be split:
> > 
> > You are right. The page tables can still be remapped
> > to point at something else after gup_fast_pte_range()
> > grabs a reference to the folio, but that can happen
> > no matter what precautions we take inside gup_fast,
> > because it can happen after gup_fast has returned.
> > 
> > I have no other concerns, so feel free to add my:
> > 
> > Reviewed-by: Rik van Riel <riel@xxxxxxxxxxx>
> 
> There are a bunch of things I don't like about this, including suspiciously LLM
> -sloppy looking stuff like
> 
> +	if (!pte_write(pte)) {
> +		for (i = 1; i < nr; i++) {
> +			if (gup_must_unshare(NULL, flags, page + i))
> +				break;
> +		}
> +		nr = i;
> +	}
> 
> I'll find someone to polish this up.

Thanks David, I'll post with a little bit fix in next rc-period.

-- 
Sincerely,
Yeoreum Yun