Re: [PATCH 2/2] Fix an AB-BA deadlock in hugetlbfs_punch_hole() involving page migration.

[Jinchao Wang <wangjinchao600@gmail.com>]

On Thu, Jan 08, 2026 at 02:09:19PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 08, 2026 at 08:39:25PM +0800, Jinchao Wang wrote:
> > The deadlock occurs due to the following lock ordering:
> > 
> > Task A (punch_hole):             Task B (migration):
> > --------------------             -------------------
> > 1. i_mmap_lock_write(mapping)    1. folio_lock(folio)
> > 2. folio_lock(folio)             2. i_mmap_lock_read(mapping)
> >    (blocks waiting for B)           (blocks waiting for A)
> > 
> > Task A is blocked in the punch-hole path:
> >   hugetlbfs_fallocate
> >     hugetlbfs_punch_hole
> >       hugetlbfs_zero_partial_page
> >         filemap_lock_hugetlb_folio
> >           filemap_lock_folio
> >             __filemap_get_folio
> >               folio_lock
> > 
> > Task B is blocked in the migration path:
> >   migrate_pages
> >     migrate_hugetlbs
> >       unmap_and_move_huge_page
> >         remove_migration_ptes
> >           __rmap_walk_file
> >             i_mmap_lock_read
> > 
> > To break this circular dependency, use filemap_lock_folio_nowait() in
> > the punch-hole path. If the folio is already locked, Task A drops the
> > i_mmap_rwsem and retries. This allows Task B to finish its rmap walk
> > and release the folio lock.
> 
> It looks like you didn't read the lock ordering at the top of mm/rmap.c
> carefully enough:
> 
>  * hugetlbfs PageHuge() take locks in this order:
>  *   hugetlb_fault_mutex (hugetlbfs specific page fault mutex)
>  *     vma_lock (hugetlb specific lock for pmd_sharing)
>  *       mapping->i_mmap_rwsem (also used for hugetlb pmd sharing)
>  *         folio_lock
> 
Thanks for the correction, Matthew.

> So page migration is the one taking locks in the wrong order, not
> holepunch.  Maybe something like this instead?
> 
I will test your suggested change and resend the fix.

> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 5169f9717f60..4688b9e38cd2 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1458,6 +1458,7 @@ static int unmap_and_move_huge_page(new_folio_t get_new_folio,
>  	int page_was_mapped = 0;
>  	struct anon_vma *anon_vma = NULL;
>  	struct address_space *mapping = NULL;
> +	enum ttu_flags ttu = 0;
>  
>  	if (folio_ref_count(src) == 1) {
>  		/* page was freed from under us. So we are done. */
> @@ -1498,8 +1499,6 @@ static int unmap_and_move_huge_page(new_folio_t get_new_folio,
>  		goto put_anon;
>  
>  	if (folio_mapped(src)) {
> -		enum ttu_flags ttu = 0;
> -
>  		if (!folio_test_anon(src)) {
>  			/*
>  			 * In shared mappings, try_to_unmap could potentially
> @@ -1516,16 +1515,17 @@ static int unmap_and_move_huge_page(new_folio_t get_new_folio,
>  
>  		try_to_migrate(src, ttu);
>  		page_was_mapped = 1;
> -
> -		if (ttu & TTU_RMAP_LOCKED)
> -			i_mmap_unlock_write(mapping);
>  	}
>  
>  	if (!folio_mapped(src))
>  		rc = move_to_new_folio(dst, src, mode);
>  
>  	if (page_was_mapped)
> -		remove_migration_ptes(src, !rc ? dst : src, 0);
> +		remove_migration_ptes(src, !rc ? dst : src,
> +				ttu ? RMP_LOCKED : 0);
> +
> +	if (ttu & TTU_RMAP_LOCKED)
> +		i_mmap_unlock_write(mapping);
>  
>  unlock_put_anon:
>  	folio_unlock(dst);