mm/khugepaged.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-)
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.0On 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.
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.
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?
(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
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
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.
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
© 2016 - 2026 Red Hat, Inc.