Re: [PATCH v2 1/1] mm/hugetlb: fix possible deadlocks in hugetlb VMA unmap paths

[Lance Yang]

On 2025/11/10 20:17, Harry Yoo wrote:
> On Mon, Nov 10, 2025 at 07:15:53PM +0800, Lance Yang wrote:
> > From: Lance Yang <lance.yang@xxxxxxxxx>
> >
> > The hugetlb VMA unmap path contains several potential deadlocks, as
> > reported by syzbot. These deadlocks occur in __hugetlb_zap_begin(),
> > move_hugetlb_page_tables(), and the retry path of
> > hugetlb_unmap_file_folio() (affecting remove_inode_hugepages() and
> > unmap_vmas()), where vma_lock is acquired before i_mmap_lock. This lock
> > ordering conflicts with other paths like hugetlb_fault(), which establish
> > the correct dependency as i_mmap_lock -> vma_lock.
> >
> > Possible unsafe locking scenario:
> >
> > CPU0                                 CPU1
> > ----                                 ----
> > lock(&vma_lock->rw_sema);
> >                                       lock(&i_mmap_lock);
> >                                       lock(&vma_lock->rw_sema);
> > lock(&i_mmap_lock);
> >
> > Resolve the circular dependencies reported by syzbot across multiple call
> > chains by reordering the locks in all conflicting paths to consistently
> > follow the established i_mmap_lock -> vma_lock order.
>
> But mm/rmap.c says:
> > * 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! You are right, I was mistaken ...

> I think the commit message should explain why the locking order described
> above is incorrect (or when it became incorrect) and fix the comment?

I think the locking order documented in mm/rmap.c (vma_lock -> i_mmap_lock)
is indeed the correct one to follow.

This fix has it backwards then. I'll rework it to fix the actual violations.

Thanks,
Lance