mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
@ 2026-09-13 10:11 Nguyen Ngoc Thang
  2026-09-13 16:12 ` Lance Yang
  2026-09-13 16:31 ` Zi Yan
  0 siblings, 2 replies; 14+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-13 10:11 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes
  Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Usama Arif, linux-mm,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
  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
  1 sibling, 1 reply; 14+ messages in thread
From: Lance Yang @ 2026-09-13 16:12 UTC (permalink / raw)
  To: ngocthang2710.1999
  Cc: akpm, david, ljs, ziy, baolin.wang, liam, nico.pache,
	ryan.roberts, dev.jain, baohua, lance.yang, usama.arif, linux-mm,
	linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 16:12 ` Lance Yang
@ 2026-09-13 16:16   ` Lance Yang
  0 siblings, 0 replies; 14+ messages in thread
From: Lance Yang @ 2026-09-13 16:16 UTC (permalink / raw)
  To: ngocthang2710.1999
  Cc: akpm, david, ljs, ziy, baolin.wang, liam, nico.pache,
	ryan.roberts, dev.jain, baohua, usama.arif, linux-mm,
	linux-kernel, stable



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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
  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:31 ` Zi Yan
  2026-09-13 16:36   ` [PATCH v2] " Nguyen Ngoc Thang
  2026-09-13 18:43   ` [PATCH] " Lorenzo Stoakes (ARM)
  1 sibling, 2 replies; 14+ messages in thread
From: Zi Yan @ 2026-09-13 16:31 UTC (permalink / raw)
  To: Nguyen Ngoc Thang, Andrew Morton, David Hildenbrand,
	Lorenzo Stoakes, Matthew Wilcox, Pedro Falcato
  Cc: Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Usama Arif, linux-mm,
	linux-kernel

+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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 16:31 ` Zi Yan
@ 2026-09-13 16:36   ` Nguyen Ngoc Thang
  2026-09-13 18:17     ` Andrew Morton
  2026-09-13 22:34     ` Matthew Wilcox
  2026-09-13 18:43   ` [PATCH] " Lorenzo Stoakes (ARM)
  1 sibling, 2 replies; 14+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-13 16:36 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes
  Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Usama Arif, Matthew Wilcox,
	linux-mm, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  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 22:34     ` Matthew Wilcox
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2026-09-13 18:17 UTC (permalink / raw)
  To: Nguyen Ngoc Thang
  Cc: David Hildenbrand, Lorenzo Stoakes, Zi Yan, Baolin Wang,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Usama Arif, Matthew Wilcox, linux-mm, linux-kernel,
	stable, Jan Kara, Hugh Dickins

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?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 16:31 ` Zi Yan
  2026-09-13 16:36   ` [PATCH v2] " Nguyen Ngoc Thang
@ 2026-09-13 18:43   ` Lorenzo Stoakes (ARM)
  1 sibling, 0 replies; 14+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-13 18:43 UTC (permalink / raw)
  To: Nguyen Ngoc Thang
  Cc: Zi Yan, Andrew Morton, David Hildenbrand, Matthew Wilcox,
	Pedro Falcato, Baolin Wang, Liam R . Howlett, Nico Pache,
	Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
	linux-mm, linux-kernel

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  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-14 14:19         ` David Hildenbrand (Arm)
  0 siblings, 2 replies; 14+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-13 18:48 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Nguyen Ngoc Thang, David Hildenbrand, Zi Yan, Baolin Wang,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Usama Arif, Matthew Wilcox, linux-mm, linux-kernel,
	stable, Jan Kara, Hugh Dickins

(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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 18:48       ` Lorenzo Stoakes (ARM)
@ 2026-09-13 18:49         ` Lorenzo Stoakes (ARM)
  2026-09-14 14:19         ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 14+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-13 18:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Nguyen Ngoc Thang, David Hildenbrand, Zi Yan, Baolin Wang,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Usama Arif, Matthew Wilcox, linux-mm, linux-kernel,
	stable, Jan Kara, Hugh Dickins

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

Ignore this incomplete thought :)

--
Cheers, Lorenzo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 16:36   ` [PATCH v2] " Nguyen Ngoc Thang
  2026-09-13 18:17     ` Andrew Morton
@ 2026-09-13 22:34     ` Matthew Wilcox
  2026-09-14  3:14       ` Baolin Wang
  1 sibling, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2026-09-13 22:34 UTC (permalink / raw)
  To: Nguyen Ngoc Thang
  Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan,
	Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Usama Arif, linux-mm,
	linux-kernel, stable

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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 22:34     ` Matthew Wilcox
@ 2026-09-14  3:14       ` Baolin Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Baolin Wang @ 2026-09-14  3:14 UTC (permalink / raw)
  To: Matthew Wilcox, Nguyen Ngoc Thang
  Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Usama Arif, linux-mm, linux-kernel, stable



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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-13 18:48       ` Lorenzo Stoakes (ARM)
  2026-09-13 18:49         ` Lorenzo Stoakes (ARM)
@ 2026-09-14 14:19         ` David Hildenbrand (Arm)
  2026-09-14 16:58           ` Mike Rapoport
  1 sibling, 1 reply; 14+ messages in thread
From: David Hildenbrand (Arm) @ 2026-09-14 14:19 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM), Andrew Morton
  Cc: Nguyen Ngoc Thang, Zi Yan, Baolin Wang, Liam R . Howlett,
	Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
	Usama Arif, Matthew Wilcox, linux-mm, linux-kernel, stable,
	Jan Kara, Hugh Dickins, Mike Rapoport

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead
  2026-09-14 14:19         ` David Hildenbrand (Arm)
@ 2026-09-14 16:58           ` Mike Rapoport
  0 siblings, 0 replies; 14+ messages in thread
From: Mike Rapoport @ 2026-09-14 16:58 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Lorenzo Stoakes (ARM),
	Andrew Morton, Nguyen Ngoc Thang, Zi Yan, Baolin Wang,
	Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
	Lance Yang, Usama Arif, Matthew Wilcox, linux-mm, linux-kernel,
	stable, Jan Kara, Hugh Dickins

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] khugepaged: hold invalidate_lock across collapse_file() readahead
@ 2026-09-13 10:08 Nguyen Ngoc Thang
  0 siblings, 0 replies; 14+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-13 10:08 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes
  Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
	Dev Jain, Barry Song, Lance Yang, Usama Arif, linux-mm,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-09-14 16:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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-14 14:19         ` David Hildenbrand (Arm)
2026-09-14 16:58           ` Mike Rapoport
2026-09-13 22:34     ` Matthew Wilcox
2026-09-14  3:14       ` Baolin Wang
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®