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

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

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@surriel.com>

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.

-- 
Cheers,

David