Re: [PATCH] smaps: fix BUG_ON in smaps_hugetlb_range

[Dev Jain <dev.jain@arm.com>]

On 21/07/25 1:44 pm, Jinjiang Tu wrote:
> smaps_hugetlb_range() handles the pte without holdling ptl, and may be
> concurrenct with migration, leaing to BUG_ON in pfn_swap_entry_to_page().
> The race is as follows.
>
> smaps_hugetlb_range              migrate_pages
>    huge_ptep_get
>                                     remove_migration_ptes
>                                  folio_unlock
>    pfn_swap_entry_folio
>      BUG_ON
>
> To fix it, hold ptl lock in smaps_hugetlb_range().
>
> Fixes: 25ee01a2fca0 ("mm: hugetlb: proc: add hugetlb-related fields to /proc/PID/smaps")
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
>   fs/proc/task_mmu.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 751479eb128f..0102ab3aaec1 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1020,10 +1020,13 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
>   {
>       struct mem_size_stats *mss = walk->private;
>       struct vm_area_struct *vma = walk->vma;
> -     pte_t ptent = huge_ptep_get(walk->mm, addr, pte);
>       struct folio *folio = NULL;
>       bool present = false;
> +     spinlock_t *ptl;
> +     pte_t ptent;
>
> +     ptl = huge_pte_lock(hstate_vma(vma), walk->mm, pte);
> +     ptent = huge_ptep_get(walk->mm, addr, pte);
>       if (pte_present(ptent)) {
>               folio = page_folio(pte_page(ptent));
>               present = true;
> @@ -1042,6 +1045,7 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
>               else
>                       mss->private_hugetlb += huge_page_size(hstate_vma(vma));
>       }
> +     spin_unlock(ptl);
>       return 0;
>   }
>   #else

LGTM but will wait for others...

the subject line at the very least should be
"fix race between smaps and migration"; or,
reading smaps_pte_range it doesn't jump
at me why one would run smaps_hugetlb_range without
the PTL, it can be "take PTL in smaps_hugetlb_range".
The current subject line doesn't sound right,
"fixing BUG_ON" means nothing :)

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.