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

[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

[David Hildenbrand (Arm)]

On 9/14/26 18:58, Mike Rapoport wrote:
> 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.#

Okay, Andrew please drop it for now.

-- 
Cheers,

David

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.

So what's the status of this? Any volunteers? Otherwise I'll polish this up.

-- 
Cheers,

David

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

[Lance Yang]

On 2026/10/15 20:26, David Hildenbrand (Arm) wrote:
> 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.
> 
> So what's the status of this? Any volunteers? Otherwise I'll polish this up.

I can take this over if no one else volunteers :D

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

[David Hildenbrand (Arm)]

On 10/15/26 15:17, Lance Yang wrote:
> 
> 
> On 2026/10/15 20:26, David Hildenbrand (Arm) wrote:
>> 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
>>>
>>>
>>> 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...
>>>
>>>
>>> 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.
>>
>> So what's the status of this? Any volunteers? Otherwise I'll polish this up.
> 
> I can take this over if no one else volunteers :D
> 

Thanks!

-- 
Cheers,

David

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.