Re: [PATCH] smaps: fix BUG_ON in smaps_hugetlb_range

[Jinjiang Tu <tujinjiang@huawei.com>]

在 2025/7/21 17:11, Dev Jain 写道:
>
> 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 :)
Thanks, I will send v2 to update it after the disscussion finishes.
>
> 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.
>