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

[David Hildenbrand (Arm)]

On 10/5/26 11:03, Li Zhe wrote:
> 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.
@Lorenzo, if Oscar is too busy, I guess we can paste the overall idea for the
fix here as well (publicly)?

-- 
Cheers,

David