Re: [PATCH v1] mm: centralize+fix comments about compound_mapcount() in new sync_with_folio_pmd_zap()

[Matthew Wilcox]

On Mon, Feb 23, 2026 at 05:39:20PM +0100, David Hildenbrand (Arm) wrote:
>  void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>  
> +/**
> + * sync_with_folio_pmd_zap - sync with concurrent zapping of a folio PMD
> + * @mm: The mm_struct.
> + * @pmdp: Pointer to the pmd that was found to be pmd_none().
> + *
> + * When we stumble over a pmd_none() without holding the PTL while unmapping a
> + * folio that could have been mapped at that PMD, it could be that concurrent
> + * zapping of the PMD is not complete yet. While the PMD might be pmd_none()
> + * already, the folio might still appear to be mapped (folio_mapped()).
> + *
> + * Wait for concurrent zapping to complete by grabbing the PTL.
> + */

I like this.  The one thing we've lost is the name of the function which
does the zapping, which I think was a helpful detail.  Perhaps not to
someone who's deep in "how page tables work", but I wouldn't know where
to look for the counterpart to this.  So how about:

Option A:
+ * When we stumble over a pmd_none() without holding the PTL while
+ * unmapping a folio that could have been mapped at that PMD, 
+ * zap_huge_pmd() may not be complete yet. While the PMD might be pmd_none()
+ * already, the folio might still appear to be mapped (folio_mapped()).

Option B:
+ * When we find a pmd_none() while unmapping a folio without holding
+ * the PTL, zap_huge_pmd() may have cleared the PMD but not yet
+ * modified the folio to indicate that it's unmapped.

(for both options, I'm just changing that one paragraph; the paragraph
starting "Wait", I would leave unchanged)

> +static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp)
> +{
> +	spinlock_t *ptl = pmd_lock(mm, pmdp);
> +
> +	spin_unlock(ptl);
> +}
> +
>  struct zap_details;
>  void unmap_page_range(struct mmu_gather *tlb,
>  			     struct vm_area_struct *vma,
> diff --git a/mm/memory.c b/mm/memory.c
> index 876bf73959c6..c87d796050ba 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2006,13 +2006,7 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb,
>  		} else if (details && details->single_folio &&
>  			   folio_test_pmd_mappable(details->single_folio) &&
>  			   next - addr == HPAGE_PMD_SIZE && pmd_none(*pmd)) {
> -			spinlock_t *ptl = pmd_lock(tlb->mm, pmd);
> -			/*
> -			 * Take and drop THP pmd lock so that we cannot return
> -			 * prematurely, while zap_huge_pmd() has cleared *pmd,
> -			 * but not yet decremented compound_mapcount().
> -			 */
> -			spin_unlock(ptl);
> +			sync_with_folio_pmd_zap(tlb->mm, pmd);
>  		}
>  		if (pmd_none(*pmd)) {
>  			addr = next;
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index b38a1d00c971..a4d52fdb3056 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -269,11 +269,6 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>  			spin_unlock(pvmw->ptl);
>  			pvmw->ptl = NULL;
>  		} else if (!pmd_present(pmde)) {
> -			/*
> -			 * If PVMW_SYNC, take and drop THP pmd lock so that we
> -			 * cannot return prematurely, while zap_huge_pmd() has
> -			 * cleared *pmd but not decremented compound_mapcount().
> -			 */
>  			const softleaf_t entry = softleaf_from_pmd(pmde);
>  
>  			if (softleaf_is_device_private(entry)) {
> @@ -284,11 +279,9 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>  			if ((pvmw->flags & PVMW_SYNC) &&
>  			    thp_vma_suitable_order(vma, pvmw->address,
>  						   PMD_ORDER) &&
> -			    (pvmw->nr_pages >= HPAGE_PMD_NR)) {
> -				spinlock_t *ptl = pmd_lock(mm, pvmw->pmd);
> +			    (pvmw->nr_pages >= HPAGE_PMD_NR))
> +				sync_with_folio_pmd_zap(mm, pvmw->pmd);
>  
> -				spin_unlock(ptl);
> -			}
>  			step_forward(pvmw, PMD_SIZE);
>  			continue;
>  		}
> -- 
> 2.43.0
> 
>