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

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

[Li Zhe]

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@bytedance.com>
---
 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;
+
 	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

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

[Jose A. Perez de Azpillaga]

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@bytedance.com>
> ---
>  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?

>  	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

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

[David Hildenbrand (Arm)]

On 9/30/26 08:43, 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@bytedance.com>
> ---
>  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.
> +	 */

"just an optimization" is not entirely true. There are workloads that depend on
huge pmd sharing to work. So I don't think this is acceptable.

There was a private discussion on this issue recently (for some weird reason the
deadlock was reported as a security issue), 2 months ago I think.

Oscar, I recall that you wanted to send a proper patch?

-- 
Cheers,

David

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@bytedance.com>
>> ---
>>   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

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@bytedance.com>
>>> ---
>>>   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

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

[Lorenzo Stoakes (ARM)]

On Mon, Oct 05, 2026 at 11:25:01AM +0200, David Hildenbrand (Arm) wrote:
> > 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)?

Yeah I don't see why not! I don't think there's anything that needs to be
private here.

--
Cheers, Lorenzo

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

[David Hildenbrand (Arm)]

On 10/9/26 13:52, Lorenzo Stoakes (ARM) wrote:
> On Mon, Oct 05, 2026 at 11:25:01AM +0200, David Hildenbrand (Arm) wrote:
>>> 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)?
> 
> Yeah I don't see why not! I don't think there's anything that needs to be
> private here.

Li, see below, can you work with the below?


----8<----
From 9e1c0676ea6c9cbcabf91cd8badf81a9d88fd3ef Mon Sep 17 00:00:00 2001
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
Date: Wed, 29 Jul 2026 18:33:08 +0100
Subject: [PATCH] fix

---
 mm/hugetlb.c | 103 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 79 insertions(+), 24 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index e93c4d2456aa..88ff2c4ad483 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5077,6 +5077,79 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
 	spin_unlock(dst_ptl);
 }

+/* Returns last address processed */
+static unsigned long prealloc_or_move_page_tables(struct vm_area_struct *vma,
+		struct vm_area_struct *new_vma, unsigned long old_addr,
+		unsigned long new_addr, unsigned long len, unsigned long sz,
+		unsigned long last_addr_mask, struct mmu_gather *tlb,
+		bool is_prealloc)
+{
+	unsigned long old_end = old_addr + len;
+	struct hstate *h = hstate_vma(vma);
+	struct mm_struct *mm = vma->vm_mm;
+	pte_t *src_pte, *dst_pte;
+
+	hugetlb_vma_assert_locked(vma);
+
+	for (; old_addr < old_end; old_addr += sz, new_addr += sz) {
+		src_pte = hugetlb_walk(vma, old_addr, sz);
+		if (!src_pte) {
+			old_addr |= last_addr_mask;
+			new_addr |= last_addr_mask;
+			continue;
+		}
+		if (huge_pte_none(huge_ptep_get(mm, old_addr, src_pte)))
+			continue;
+
+		if (is_prealloc) {
+			if (!huge_pte_alloc(mm, new_vma, new_addr, sz))
+				break;
+			continue;
+		}
+
+		if (huge_pmd_unshare(tlb, vma, old_addr, src_pte)) {
+			old_addr |= last_addr_mask;
+			new_addr |= last_addr_mask;
+			continue;
+		}
+
+		dst_pte = hugetlb_walk(new_vma, new_addr, sz);
+		if (!dst_pte)
+			break;
+
+		move_huge_pte(vma, old_addr, new_addr, src_pte, dst_pte, sz);
+		tlb_remove_huge_tlb_entry(h, tlb, src_pte, old_addr);
+	}
+
+	return old_addr;
+}
+
+/* Returns true if preallocation succeeded across range, otherwise false. */
+static bool prealloc_hugetlb_page_tables(struct vm_area_struct *vma,
+		struct vm_area_struct *new_vma, unsigned long old_addr,
+		unsigned long new_addr, unsigned long len, unsigned long sz,
+		unsigned long last_addr_mask)
+{
+	unsigned long old_end = old_addr + len;
+	unsigned long addr_end;
+
+	addr_end = prealloc_or_move_page_tables(vma, new_vma, old_addr, new_addr,
+			len, sz, last_addr_mask, NULL, /*is_prealloc=*/true);
+
+	return addr_end >= old_end;
+}
+
+/* Returns last processed address. */
+static unsigned long __move_hugetlb_page_tables(struct vm_area_struct *vma,
+		struct vm_area_struct *new_vma, unsigned long old_addr,
+		unsigned long new_addr, unsigned long len, unsigned long sz,
+		unsigned long last_addr_mask, struct mmu_gather *tlb)
+{
+	i_mmap_assert_write_locked(vma->vm_file->f_mapping);
+	return prealloc_or_move_page_tables(vma, new_vma, old_addr, new_addr,
+			len, sz, last_addr_mask, tlb, /*is_prealloc=*/false);
+}
+
 int move_hugetlb_page_tables(struct vm_area_struct *vma,
 			     struct vm_area_struct *new_vma,
 			     unsigned long old_addr, unsigned long new_addr,
@@ -5088,9 +5161,9 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,
 	struct mm_struct *mm = vma->vm_mm;
 	unsigned long old_end = old_addr + len;
 	unsigned long last_addr_mask;
-	pte_t *src_pte, *dst_pte;
 	struct mmu_notifier_range range;
 	struct mmu_gather tlb;
+	bool preallocated;

 	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, old_addr,
 				old_end);
@@ -5106,30 +5179,12 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,
 	last_addr_mask = hugetlb_mask_last_page(h);
 	/* Prevent race with file truncation */
 	hugetlb_vma_lock_write(vma);
+	preallocated = prealloc_hugetlb_page_tables(vma, new_vma, old_addr,
+			new_addr, len, sz, last_addr_mask);
 	i_mmap_lock_write(mapping);
-	for (; old_addr < old_end; old_addr += sz, new_addr += sz) {
-		src_pte = hugetlb_walk(vma, old_addr, sz);
-		if (!src_pte) {
-			old_addr |= last_addr_mask;
-			new_addr |= last_addr_mask;
-			continue;
-		}
-		if (huge_pte_none(huge_ptep_get(mm, old_addr, src_pte)))
-			continue;
-
-		if (huge_pmd_unshare(&tlb, vma, old_addr, src_pte)) {
-			old_addr |= last_addr_mask;
-			new_addr |= last_addr_mask;
-			continue;
-		}
-
-		dst_pte = huge_pte_alloc(mm, new_vma, new_addr, sz);
-		if (!dst_pte)
-			break;
-
-		move_huge_pte(vma, old_addr, new_addr, src_pte, dst_pte, sz);
-		tlb_remove_huge_tlb_entry(h, &tlb, src_pte, old_addr);
-	}
+	if (preallocated)
+		old_addr = __move_hugetlb_page_tables(vma, new_vma, old_addr,
+			new_addr, len, sz, last_addr_mask, &tlb);

 	tlb_flush_mmu_tlbonly(&tlb);
 	huge_pmd_unshare_flush(&tlb, vma);
--
2.55.0

-- 
Cheers,

David