Re: [PATCH v2] mm/hugetlb: fix a deadlock with pagecache_folio and hugetlb_fault_mutex_table

On Wed, 21 May 2025, Gavin Guo wrote:

... V1 -> V2 Suggested-by Oscar Salvador:

• Use folio_test_locked to replace the unnecessary parameter passing.

mm/hugetlb.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 7ae38bfb9096..ed501f134eff 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6226,6 +6226,12 @@ static vm_fault_t hugetlb_wp(struct folio *pagecache_folio, u32 hash; folio_put(old_folio);

• 	/*
  

• 	 * The pagecache_folio needs to be unlocked to avoid
  

• 	 * deadlock when the child unmaps the folio.
  

• 	 */
  

• 	if (pagecache_folio)
  

• 		folio_unlock(pagecache_folio);
  /*
   * Drop hugetlb_fault_mutex and vma_lock before
   * unmapping.  unmapping needs to hold vma_lock
  

@@ -6823,8 +6829,13 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, out_ptl: spin_unlock(vmf.ptl);

• /*

• * hugetlb_wp() might have already unlocked pagecache_folio, so
  

• * skip it if that is the case.
  

• */
  

  if (pagecache_folio) {

• folio_unlock(pagecache_folio);
  

• if (folio_test_locked(pagecache_folio))
  

• 	folio_unlock(pagecache_folio);
  

  folio_put(pagecache_folio); }

out_mutex:

NAK!

I have not (and shall not) review V1, but was hoping someone else would save me from rejecting this V2 idea immediately.

Unless you have a very strong argument why this folio is invisible to the rest of the world, including speculative accessors like compaction (and the name "pagecache_folio" suggests very much the reverse): the pattern of unlocking a lock when you see it locked is like (or worse than) having no locking at all - it is potentially unlocking someone else's lock.

Hugh