Re: [PATCH v2 1/5] mm/memory-failure: keep the folio, not the poisoned subpage, locked across split

[Kiryl Shutsemau]

On Tue, Jul 14, 2026 at 03:01:46PM +0200, David Hildenbrand (Arm) wrote:
> On 7/14/26 14:23, Kiryl Shutsemau wrote:
> > From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
> > 
> > try_to_split_thp_page() locked the poisoned page and passed it to
> > split_huge_page_to_order(), which returns that very page locked to the
> > caller.  For a tail page that means __folio_split() runs with @lock_at
> > pointing into the middle of the folio.
> > 
> > __folio_split() dereferences the mapping after the split completes
> > (shmem_uncharge(), i_mmap_unlock_read()).  The only thing keeping the
> > inode alive across that is the locked @lock_at folio: while it stays in
> > the page cache, eviction cannot complete.
> > 
> > But a tail @lock_at can lie beyond EOF -- e.g. part of a shmem THP that
> > reaches past i_size while the file is being truncated.  The split then
> > drops it from the page cache yet still returns it locked, so the pin is
> > gone and a racing final iput() can evict and RCU-free the inode while
> > __folio_split() is still running:
> > 
> >   BUG: KASAN: slab-use-after-free in __up_read+0x634/0x790
> >    i_mmap_unlock_read include/linux/fs.h:537 [inline]
> >    __folio_split+0x732/0x1640 mm/huge_memory.c:4100
> >    try_to_split_thp_page+0xab/0x390 mm/memory-failure.c:1675
> >    memory_failure+0x1394/0x26e0 mm/memory-failure.c:2470
> > 
> >   Freed by task 4601:
> >    shmem_free_in_core_inode+0x54/0xb0 mm/shmem.c:5177
> >    evict+0x57f/0xac0 fs/inode.c:870
> > 
> > Split the folio as a folio, via split_folio_to_order(), so the head is
> > the anchor left locked.  The head is piece 0, which the beyond-EOF drop
> > loop never removes (it starts at folio_next(folio)), so the split always
> > leaves it in the page cache and the inode stays pinned for the whole of
> > __folio_split().  memory_failure() and soft offline re-lock the poisoned
> > subpage's folio themselves after the split, so they do not depend on it
> > being returned locked.
> > 
> > Reported-by: Hao Zhang <zhanghao1@xxxxxxxxxx>
> > Closes: https://lore.kernel.org/linux-mm/20260710071344.GA106129@zh-pc
> > Fixes: baa355fd3314 ("thp: file pages support for split_huge_page()")
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> > ---
> >  mm/memory-failure.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index 51508a55c405..68d42cbed458 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -1657,11 +1657,18 @@ static int identify_page_state(unsigned long pfn, struct page *p,
> >  static int try_to_split_thp_page(struct page *page, unsigned int new_order,
> >  		bool release)
> >  {
> > +	struct folio *folio = page_folio(page);
> >  	int ret;
> >  
> > -	lock_page(page);
> > -	ret = split_huge_page_to_order(page, new_order);
> > -	unlock_page(page);
> > +	/*
> > +	 * Lock and split at the head, not the poisoned subpage: __folio_split()
> > +	 * keeps the anchor folio locked and needs it to stay in the page cache
> > +	 * to pin the inode. A tail beyond EOF would be dropped yet returned
> > +	 * locked, losing that pin. The caller re-locks @page afterwards.
> > +	 */
> > +	folio_lock(folio);
> > +	ret = split_folio_to_order(folio, new_order);
> > +	folio_unlock(folio);
> 
> With a non-uniform split it would actually make a difference: we'd want to split
> such that we the other folio pages minimal.
> 
>  split_folio_to_order() always seems to end up in
> __split_huge_page_to_list_to_order() where we do a SPLIT_TYPE_UNIFORM.
> 
> I recall discussing with Zi and Willy that in the future we'd want to convert
> more places to do a non-uniform split.
> 
> So I'm afraid that would just re-introduce the problem then.

Right. Non-uniform split can be useful.

But my patch is completely broken because code expects the pin to be on the
@page, not on the head. put_page() few lines down can explode already.

So the fix does not belong in memory_failure(). It belongs in
__folio_split(), and it is really just 2/5: refuse the split with -EBUSY
when @lock_at is at or beyond the sampled EOF.

The safety then sits in __folio_split() regardless of caller or split type,
which should also cover your non-uniform worry.

The behavioural change is that memory_failure() reports a beyond-EOF
poisoned tail as unsplit (MF_FAILED) instead of recovered, and kills the
mappers instead of splitting the page off. What we give up is salvaging
the folio's healthy pages and the clean unmap -- both low value for a page
that is beyond EOF and getting truncated away. Containment is unaffected:
PageHWPoison is set before the split and free_pages_prepare() keeps a
poisoned page out of the buddy allocator, so the bad page never comes back
regardless.

The alternative, if we would rather keep the recovered outcome, is to leave
@lock_at as the poisoned page and move i_mmap_unlock_read() (and
shmem_uncharge()) ahead of the after-split unlock loop, so every mapping
dereference happens while @folio -- the head, within EOF -- still pins the
inode. That is close to Hao's original patch. It works, but it rests on "no
mapping dereference after the unlock loop", which is its own fragility.

I lean towards the -EBUSY guard.

Comments?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov