Re: [PATCH] mm/hugetlb: avoid recursive i_mmap_rwsem in PMD sharing

[Li Zhe]

On 9/30/26 5:31 PM, Jose A. Perez de Azpillaga wrote:
> On Wed, Sep 30, 2026 at 02:43:08PM +0800, Li Zhe wrote:
>> huge_pmd_share() always takes i_mmap_rwsem for read before looking for a
>> shareable PMD page table.
>>
>> That is unsafe for callers that already hold the same mapping lock for
>> write.  In particular, move_hugetlb_page_tables() takes i_mmap_rwsem for
>> write to prevent truncation races, then calls huge_pte_alloc() for the
>> destination address.  If the destination PUD is empty and PMD sharing is
>> possible, huge_pte_alloc() can call huge_pmd_share(), which then tries to
>> take the same rwsem for read and can deadlock on itself.
>>
>> PMD sharing is only an optimization.  If the read side of i_mmap_rwsem
>> cannot be acquired immediately, fall back to allocating a private PMD
>> table instead of blocking in the sharing path.
>>
>> Signed-off-by: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
>> ---
>>   mm/hugetlb.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index cea25773a6c95..75631101a6807 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -6995,7 +6995,15 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
>>   	pte_t *spte = NULL;
>>   	pte_t *pte;
>>
>> -	i_mmap_lock_read(mapping);
>> +	/*
>> +	 * Some callers already hold i_mmap_rwsem for write, for example
>> +	 * move_hugetlb_page_tables(). PMD sharing is only an optimization, so
>> +	 * fall back to a private PMD table instead of blocking on the same
>> +	 * rwsem in read mode.
>> +	 */
>> +	if (!i_mmap_trylock_read(mapping))
>> +		goto alloc;
> try deadlock is not a fallback here. both callers already hold the write
> lock, and that is enough for the walk, so sharing is never attempted on
> these paths.
>
> hugetlb_change_protection() does thr same thing on the uffd-wp path and
> is not in the changelog.
>
> and no Fixes tag either. does this want Cc: stable?


Thanks for the review.

I agree that holding the write lock should be enough for the walk, and
that avoiding PMD sharing on these paths is not the right direction.

David mentioned that Oscar was planning to send a proper fix for this
issue, so I will wait for that patch instead of moving forward with this
trylock fallback approach.

Thanks,
Zhe

>
>>   	mapping_rmap_tree_foreach(svma, mapping, idx, idx) {
>>   		if (svma == vma)
>>   			continue;
>> @@ -7024,8 +7032,10 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
>>   	}
>>   	spin_unlock(&mm->page_table_lock);
>>   out:
>> -	pte = (pte_t *)pmd_alloc(mm, pud, addr);
>>   	i_mmap_unlock_read(mapping);
>> +
>> +alloc:
>> +	pte = (pte_t *)pmd_alloc(mm, pud, addr);
>>   	return pte;
>>   }
>>
>> --
>> 2.20.1
>>
> --
> cheers,
> jose a. p-a