All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zi Yan" <ziy@nvidia.com>
To: "Nguyen Ngoc Thang" <ngocthang2710.1999@gmail.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"David Hildenbrand" <david@kernel.org>,
	"Lorenzo Stoakes" <ljs@kernel.org>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Pedro Falcato" <pfalcato@suse.de>
Cc: "Baolin Wang" <baolin.wang@linux.alibaba.com>,
	"Liam R . Howlett" <liam@infradead.org>,
	"Nico Pache" <nico.pache@linux.dev>,
	"Ryan Roberts" <ryan.roberts@arm.com>,
	"Dev Jain" <dev.jain@arm.com>, "Barry Song" <baohua@kernel.org>,
	"Lance Yang" <lance.yang@linux.dev>,
	"Usama Arif" <usama.arif@linux.dev>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
Date: Sun, 13 Sep 2026 12:31:32 -0400	[thread overview]
Message-ID: <DLEBZ0GIGNQ8.17U563YYJ37AA@nvidia.com> (raw)
In-Reply-To: <20260913101142.28802-1-ngocthang2710.1999@gmail.com>

+willy and heat

On Sun Sep 13, 2026 at 6:11 AM EDT, Nguyen Ngoc Thang wrote:
> collapse_file() calls page_cache_sync_readahead() to fault in missing
> pages before collapsing them into a THP. That helper takes
> mapping->invalidate_lock itself for the duration of the call, then
> drops it -- but truncate (e.g. ext4_setattr() -> truncate_pagecache())
> takes invalidate_lock and then waits on each page's folio lock while
> holding it. If collapse_file() has already locked one of those folios
> by the time truncate reaches it, and then tries to acquire
> invalidate_lock again (e.g. on the next iteration, or via a nested
> readahead call), the two paths can deadlock/hang on each other's lock:
> truncate blocked on the folio lock collapse holds, and collapse
> blocked waiting for invalidate_lock that truncate holds.
>
> Reproducing this over ~150,000 collapse iterations with truncate
> racing concurrently reliably hits hung_task: blocked tasks within
> about 20 seconds on an unpatched kernel.
>
> Fix it by taking invalidate_lock_shared once for the whole scan, before
> locking any folio, and using page_cache_ra_unbounded() directly in the
> readahead call site instead of page_cache_sync_readahead(), since the
> latter would try to retake the lock we already hold.
> page_cache_ra_unbounded() does not clamp to EOF like the helper it
> replaces, so clamp the requested range explicitly.
>
> Reported-by: syzbot+16bf7cd0ebeb1de93aa5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=16bf7cd0ebeb1de93aa5
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
>  mm/khugepaged.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 11ff98d55c76..690ccbcdf593 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2267,6 +2267,13 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	VM_WARN_ON_ONCE(!is_shmem && !mapping_pmd_folio_support(mapping));
>  	VM_WARN_ON_ONCE(start & (HPAGE_PMD_NR - 1));
>  
> +	/*
> +	 * Take invalidate_lock before any folio lock: the readahead below
> +	 * needs it, and truncate holds it while waiting on folio locks.
> +	 */
> +	if (!is_shmem)
> +		filemap_invalidate_lock_shared(mapping);
> +
>  	result = alloc_charge_folio(&new_folio, mm, cc, HPAGE_PMD_ORDER);
>  	if (result != SCAN_SUCCEED)
>  		goto out;
> @@ -2337,10 +2344,20 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  			}
>  		} else {	/* !is_shmem */
>  			if (!folio || xa_is_value(folio)) {
> +				DEFINE_READAHEAD(ractl, file, &file->f_ra,
> +						  mapping, index);
> +				pgoff_t eof = DIV_ROUND_UP(i_size_read(mapping->host),
> +							    PAGE_SIZE);
> +
>  				xas_unlock_irq(&xas);
> -				page_cache_sync_readahead(mapping, &file->f_ra,
> -							  file, index,
> -							  end - index);
> +				/*
> +				 * invalidate_lock held above; don't retake it.
> +				 * page_cache_ra_unbounded(), unlike the readahead
> +				 * helper this replaces, does not clamp to EOF.
> +				 */
> +				if (index < eof)
> +					page_cache_ra_unbounded(&ractl,
> +						min(end, eof) - index, 0);
>  				/* drain lru cache to help folio_isolate_lru() */
>  				lru_add_drain();
>  				folio = filemap_lock_folio(mapping, index);
> @@ -2672,6 +2689,8 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	folio_unlock(new_folio);
>  	folio_put(new_folio);
>  out:
> +	if (!is_shmem)
> +		filemap_invalidate_unlock_shared(mapping);
>  	VM_BUG_ON(!list_empty(&pagelist));
>  	trace_mm_khugepaged_collapse_file(mm, new_folio, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
>  	return result;




-- 
Best Regards,
Yan, Zi



  parent reply	other threads:[~2026-09-13 16:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13 10:11 [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead Nguyen Ngoc Thang
2026-09-13 16:12 ` Lance Yang
2026-09-13 16:16   ` Lance Yang
2026-09-13 16:31 ` Zi Yan [this message]
2026-09-13 16:36   ` [PATCH v2] " Nguyen Ngoc Thang
2026-09-13 18:17     ` Andrew Morton
2026-09-13 18:48       ` Lorenzo Stoakes (ARM)
2026-09-13 18:49         ` Lorenzo Stoakes (ARM)
2026-09-13 22:34     ` Matthew Wilcox
2026-09-13 18:43   ` [PATCH] " Lorenzo Stoakes (ARM)
  -- strict thread matches above, loose matches on Subject: below --
2026-09-13 10:08 Nguyen Ngoc Thang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLEBZ0GIGNQ8.17U563YYJ37AA@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=ngocthang2710.1999@gmail.com \
    --cc=nico.pache@linux.dev \
    --cc=pfalcato@suse.de \
    --cc=ryan.roberts@arm.com \
    --cc=usama.arif@linux.dev \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.