Re: [PATCH v3 2/6] mm: gup: clarify a reason to check pte/pmd in gup_fast_pte_range()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] mm: gup: duplicate comment with typos in gup_fast_pte_range()
--

commit a105945d4180d981c8509ad500ce443d7fbb662c
Author: Yeoreum Yun <yeoreum.yun@arm.com>

mm: gup: clarify a reason to check pte/pmd in gup_fast_pte_range()

This commit adds an inline comment in gup_fast_pte_range() explaining why
the PMD and PTE are rechecked after grabbing the folio.

> diff --git a/mm/gup.c b/mm/gup.c
> index 7d43ce717b246..7f9348c633d58 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2881,6 +2881,14 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
>  		if (!folio)
>  			goto pte_unmap;
>  
> +		/*
> +		 * Recheck the PTE after grabbing the folio to detect concurrent
> +		 * unmapping or protection changes such as by KSM.
> +		 *
> +		 * Also, recheck the PMD: THP collpase can detach the PTE table
> +		 * while its entries remain unchaged. Checking the PTE alone
> +		 * would not dtect that we are walking a detached page table.
> +		 */
>  		if (unlikely(pmd_val(pmd) != pmd_val(pmdp_get_lockless(pmdp))) ||
>  		    unlikely(pte_val(pte) != pte_val(ptep_get_lockless(ptep)))) {
>  			gup_put_folio(folio, 1, flags);

[Severity: Low]
This isn't a bug, but does this comment duplicate the existing block
comment directly above gup_fast_pte_range()?

mm/gup.c:gup_fast_pte_range() {
/*
 * GUP-fast relies on pte change detection to avoid concurrent pgtable
 * operations.
 *
 * To pin the page, GUP-fast needs to do below in order:
 * (1) pin the page (by prefetching pte), then (2) check pte not changed.
 ...
 * For THP collapse, it's a bit more complicated because GUP-fast may be
 * walking a pgtable page that is being freed (pte is still valid but pmd
 * can be cleared already).  To avoid race in such condition, we need to
 * also check pmd here to make sure pmd doesn't change (corresponds to
 * pmdp_collapse_flush() in the THP collapse code path).
 */
}

The commit message also repeats the subject line verbatim without
explaining what is missing from the existing function comment.  If the
new comment is kept or merged into the existing one, note that it has
three typos: "collpase" (collapse), "unchaged" (unchanged), and "dtect"
(detect).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-refactor_gup_fast-v3-0-2aaef02df0e1@arm.com?part=2