Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead

[Andrew Morton]

On Sun, 13 Sep 2026 23:36:44 +0700 Nguyen Ngoc Thang <ngocthang2710.1999@xxxxxxxxx> 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 first readahead call, since
> invalidate_lock is not yet held at that point), 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, after
> alloc_charge_folio() succeeds and 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.
> 
> 730633f0b7f9 added invalidate_lock acquisition around readahead but
> missed collapse_file(), which already locks pages while calling
> readahead; later filesystem conversions made the deadlock reachable by
> taking invalidate_lock before waiting on page locks during truncate.
> 
> Reported-by: syzbot+16bf7cd0ebeb1de93aa5@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=16bf7cd0ebeb1de93aa5
> Fixes: 730633f0b7f9 ("mm: Protect operations adding pages to page cache with invalidate_lock")

(You forgot to cc the original author)

Five years.  

I wonder why this hasn't been discovered by lockdep, AI, syzbot or any
other of the tools we've been using for so long.

Thanks for doing all this.

> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2257,6 +2257,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	enum scan_result result = SCAN_SUCCEED;
>  	int nr_none = 0;
>  	bool is_shmem = shmem_file(file);
> +	bool need_unlock = false;
>  
>  	/*
>  	 * MADV_COLLAPSE ignores shmem huge config, so do not check shmem
> @@ -2271,6 +2272,15 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>  	if (result != SCAN_SUCCEED)
>  		goto out;
>  
> +	/*
> +	 * Take invalidate_lock before any folio lock: the readahead below
> +	 * needs it, and truncate holds it while waiting on folio locks.
> +	 */
> +	if (!is_shmem) {

Is the shmem special-case a red flag?

Probably this fix an acceptable minimal-thing-for-backporting.

Question for maintainers as well as for yourself: but does this
indicate a need for a more architected redo?