Re: [PATCH] mm: use memory barriers for non PMD-sized folios to sync

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] mm: sync_with_folio_pmd_zap() triggers BUILD_BUG() when huge
  leaves disabled
--

commit 6fcd156071f8ff19dadc585e086208ca97feaaa2
Author: Baolin Wang <baolin.wang@linux.alibaba.com>

mm: use memory barriers for non PMD-sized folios to sync

This commit replaces the PMD lock in sync_with_folio_pmd_zap() with an
smp_rmb() paired with an smp_wmb() in zap_pte_range() when synchronizing
concurrent zapping of non-PMD-sized folios.

> diff --git a/mm/internal.h b/mm/internal.h
> index cd3675c044e0a..a277bf4841f80 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -592,6 +592,7 @@ void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>   * sync_with_folio_pmd_zap - sync with concurrent zapping of folios
>   * @mm: The mm_struct.
>   * @pmdp: Pointer to the pmd that was found to be pmd_none().
> + * @nr_pages: The number of pages in the folio.
>   *
>   * 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 updated the folio
> @@ -600,13 +601,30 @@ void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>   * against later mapcount reads. Skipping the PMD without synchronization
>   * could make folio unmapping code assume that unmapping failed.
>   *
> - * Wait for concurrent zapping to complete by grabbing the PTL.
> + * Wait for concurrent zapping to complete by grabbing the PTL for PMD-sized
> + * folios, otherwise smp_rmb() is sufficient.
>   */
> -static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp)
> +static inline void sync_with_folio_pmd_zap(struct mm_struct *mm, pmd_t *pmdp,
> +		unsigned int nr_pages)
>  {
> -	spinlock_t *ptl = pmd_lock(mm, pmdp);
> +	if (nr_pages >= HPAGE_PMD_NR) {
> +		spinlock_t *ptl = pmd_lock(mm, pmdp);
>  
> -	spin_unlock(ptl);
> +		spin_unlock(ptl);
> +		return;
> +	}

[Severity: High]
Does evaluating HPAGE_PMD_NR against the runtime nr_pages argument break
the build when CONFIG_PGTABLE_HAS_HUGE_LEAVES is disabled?

When both CONFIG_TRANSPARENT_HUGEPAGE and CONFIG_HUGETLB_PAGE are
disabled, HPAGE_PMD_SHIFT expands to a BUILD_BUG():

include/linux/huge_mm.h:
#ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES
#define HPAGE_PMD_SHIFT PMD_SHIFT
#define HPAGE_PUD_SHIFT PUD_SHIFT
#else
#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
#define HPAGE_PUD_SHIFT ({ BUILD_BUG(); 0; })
#endif

#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)

Because zap_pmd_range() and page_vma_mapped_walk() call
sync_with_folio_pmd_zap() unconditionally, the compiler cannot eliminate
the HPAGE_PMD_NR evaluation and triggers BUILD_BUG().  Guarding the check
with IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) avoids evaluating
HPAGE_PMD_NR when transparent hugepages are disabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/09736bc2e01bec0ca7436733078c82c7e0cc6627.1794376686.git.baolin.wang@linux.alibaba.com?part=1