[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