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

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

[Nguyen Ngoc Thang]

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;
-- 
2.43.0

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

[Lance Yang]

Hi Nguyen,

Good catch, thanks!

On Sun, Sep 13, 2026 at 05:11:42PM +0700, 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.

That can happen on the first readahead call.

>
>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

Ouch, we should add a Fixes tag and Cc stable:

Fixes: 730633f0b7f9 ("mm: Protect operations adding pages to page cache with invalidate_lock")
Cc: stable@vger.kernel.org

730633f0b7f9 added invalidate_lock acquisition to readahead but missed
collapse_file(), which already called it with page locks held. The
subsequent filesystem conversions made the deadlock possible by taking
invalidate_lock before locking pages :( right?

>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);

Could we take the lock after alloc_charge_folio() succeeds, before
locking any folio? That would keep allocation and charging outside the
critical section. And if allocation fails, we should skip the unlock :)

Cheers, Lance

>+
> 	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;
>-- 
>2.43.0
>
>

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

[Lance Yang]

On 2026/9/14 00:12, Lance Yang wrote:
> Hi Nguyen,
> 
> Good catch, thanks!
> 
> On Sun, Sep 13, 2026 at 05:11:42PM +0700, 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.
> 
> That can happen on the first readahead call.
> 
>>
>> 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
> 
> Ouch, we should add a Fixes tag and Cc stable:
> 
> Fixes: 730633f0b7f9 ("mm: Protect operations adding pages to page cache with invalidate_lock")
> Cc: stable@vger.kernel.org
> 
> 730633f0b7f9 added invalidate_lock acquisition to readahead but missed
> collapse_file(), which already called it with page locks held. The
> subsequent filesystem conversions made the deadlock possible by taking
> invalidate_lock before locking pages :( right?
> 
>> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
>> ---

Forgot to mention:

I reproduced the hang too, and it goes away with this patch applied :)

Tested-by: Lance Yang <lance.yang@linux.dev>

>> 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);
> 
> Could we take the lock after alloc_charge_folio() succeeds, before
> locking any folio? That would keep allocation and charging outside the
> critical section. And if allocation fails, we should skip the unlock :)
> 
> Cheers, Lance
> 
>> +
>> 	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;
>> -- 
>> 2.43.0
>>
>>

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

[Zi Yan]

+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

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

[Nguyen Ngoc Thang]

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=16bf7cd0ebeb1de93aa5
Fixes: 730633f0b7f9 ("mm: Protect operations adding pages to page cache with invalidate_lock")
Tested-by: Lance Yang <lance.yang@linux.dev>
Cc: stable@vger.kernel.org
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
v2:
- Take invalidate_lock after alloc_charge_folio() succeeds instead of
  before, so allocation/charging stays outside the critical section,
  and skip the unlock on the allocation-failure path (Lance Yang)
- Add Fixes and Cc: stable tags (Lance Yang)
- Add Tested-by (Lance Yang)

 mm/khugepaged.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 11ff98d55c76..a5fcafd57505 100644
--- 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) {
+		filemap_invalidate_lock_shared(mapping);
+		need_unlock = true;
+	}
+
 	mapping_set_update(&xas, mapping);
 
 	__folio_set_locked(new_folio);
@@ -2337,10 +2347,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 +2692,8 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	folio_unlock(new_folio);
 	folio_put(new_folio);
 out:
+	if (need_unlock)
+		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;
-- 
2.43.0

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@gmail.com> 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@syzkaller.appspotmail.com
> 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?

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

[Lorenzo Stoakes (ARM)]

(Nguyen - do not send v2 in reply to v1, look across mm and see how things are
done here).

somebody who has a

On Sun, Sep 13, 2026 at 11:17:00AM -0700, Andrew Morton wrote:
> On Sun, 13 Sep 2026 23:36:44 +0700 Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com> 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@syzkaller.appspotmail.com
> > 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)

Also a change log, and that mm doesn't like sending a respin in-reply-to a
previous version. Which is all consistent with somebody using e.g. openclaw to
pepper generated patches across the kernel...

>
> 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.

See https://lore.kernel.org/linux-mm/aqbtfms0_2ULBIT7@gremlin/

I am not really entirely happy with somebody who has sent a flurry of patches
across disparate subsystems with no previous track record being in charge of a
potentially backported fix.

It's David's decision but I think this fix should be taken over by somebody
else.

See https://lore.kernel.org/all/?q=f%3ANguyen+Ngoc+Thang

>
> > --- 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?
>

--
Cheers, Lorenzo

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

[Lorenzo Stoakes (ARM)]

On Sun, Sep 13, 2026 at 07:48:44PM +0100, Lorenzo Stoakes (ARM) wrote:
> somebody who has a

Ignore this incomplete thought :)

--
Cheers, Lorenzo

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

[David Hildenbrand (Arm)]

On 9/13/26 20:48, Lorenzo Stoakes (ARM) wrote:
> (Nguyen - do not send v2 in reply to v1, look across mm and see how things are
> done here).
> 
> somebody who has a
> 
> On Sun, Sep 13, 2026 at 11:17:00AM -0700, Andrew Morton wrote:
>> On Sun, 13 Sep 2026 23:36:44 +0700 Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com> 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@syzkaller.appspotmail.com
>>> 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)
> 
> Also a change log, and that mm doesn't like sending a respin in-reply-to a
> previous version. Which is all consistent with somebody using e.g. openclaw to
> pepper generated patches across the kernel...
> 
>>
>> 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.
> 
> See https://lore.kernel.org/linux-mm/aqbtfms0_2ULBIT7@gremlin/
> 
> I am not really entirely happy with somebody who has sent a flurry of patches
> across disparate subsystems with no previous track record being in charge of a
> potentially backported fix.
> 
> It's David's decision but I think this fix should be taken over by somebody
> else.

IIUC, this patch might be responsible for random CI failures (Mike still
investigating).

So I'd prefer if this is taken over by someone familiar with that code.

@Willy, @Zi, @Lance?


-- 
Cheers,

David

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

[Mike Rapoport]

On Mon, Sep 14, 2026 at 04:19:11PM +0200, David Hildenbrand (Arm) wrote:
> > 
> > See https://lore.kernel.org/linux-mm/aqbtfms0_2ULBIT7@gremlin/
> > 
> > I am not really entirely happy with somebody who has sent a flurry of patches
> > across disparate subsystems with no previous track record being in charge of a
> > potentially backported fix.
> > 
> > It's David's decision but I think this fix should be taken over by somebody
> > else.
> 
> IIUC, this patch might be responsible for random CI failures (Mike still
> investigating).

Checkout of this commit in mm-unstable [ 1be399d378b78 ("khugepaged: hold
invalidate_lock across collapse_file() readahead") as of a few hours back ]
'khugepaged all:file $XFS_DIR' fails once in 20 or so runs on my machine.

With this commit reverted, 400 iterations didn't fail.

> So I'd prefer if this is taken over by someone familiar with that code.
> 
> @Willy, @Zi, @Lance?
> 
> 
> -- 
> Cheers,
> 
> David

-- 
Sincerely yours,
Mike.

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

[Matthew Wilcox]

On Sun, Sep 13, 2026 at 11:36:44PM +0700, Nguyen Ngoc Thang wrote:
> @@ -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) {
> +		filemap_invalidate_lock_shared(mapping);
> +		need_unlock = true;
> +	}

I'm not a fan of all this surplus commentary.  And what happens if e
simultaeneously truncate a shmem file and collapse it at the same time?
I know it doesn't use the invalidate lock, but does it go wrong in some
other way?

>  		} 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);

Not a fan of the overly long line.

				pgoff_t eof;

				eof = DIV_ROUND_UP(i_size_read(mapping->host),
						PAGE_SIZE);

or we could cache mapping->host in a variable called 'inode'.  We use it
in four places, so that might be best.

>  				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.
> +				 */

Again, remove this comment.  This explains why we're making the change
in the way that we are and adds absolutely no value to the reader of the
new file.

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

[Baolin Wang]

On 9/14/26 6:34 AM, Matthew Wilcox wrote:
> On Sun, Sep 13, 2026 at 11:36:44PM +0700, Nguyen Ngoc Thang wrote:
>> @@ -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) {
>> +		filemap_invalidate_lock_shared(mapping);
>> +		need_unlock = true;
>> +	}
> 
> I'm not a fan of all this surplus commentary.  And what happens if e
> simultaeneously truncate a shmem file and collapse it at the same time?
> I know it doesn't use the invalidate lock, but does it go wrong in some
> other way?

IIUC, shmem uses the folio lock to synchronize truncate and collapse. It 
will check whether truncation has occurred after taking the folio lock 
in shmem_get_folio_gfp():
	
	folio_lock(folio);

	/* Has the folio been truncated or swapped out? */
	if (unlikely(folio->mapping != inode->i_mapping)) {
		folio_unlock(folio);
		folio_put(folio);
		goto repeat;
	}

So shmem looks safe here, unless I'm missing something.

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

[Lorenzo Stoakes (ARM)]

On Sun, Sep 13, 2026 at 12:31:32PM -0400, Zi Yan wrote:
> +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())

Em-dash.

> > 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.

Please don't send walls of text.

> >
> > 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>

No Fixes tag?...

Given, AFAICT, that you have no contribution history until last wednesday, and
have emerged from nowhere to sending patches across I think 8 maybe 9 subsystems
all at once, it seems a near-certainty you're using an LLM to generate patches.

Please follow kernel procedure and add an Assisted-by tag disclosing this both
on this patch and all the others, please.

https://docs.kernel.org/process/coding-assistants.html

Also note that you are also required to have an understanding of what the
patches are doing:

https://docs.kernel.org/process/generated-content.html

	"As with the output of any tooling, the result may be incorrect or
	inappropriate. You are expected to understand and to be able to defend
	everything you submit. If you are unable to do so, then do not submit
	the resulting changes."

On that basis I wonder whether it would be best if somebody else could take over
this patch?

> > ---
> >  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
>

--
Cheers, Lorenzo