Re: [PATCH 2/2] mm/truncate: Unmap large folio on split failure

[Kiryl Shutsemau <kirill@shutemov.name>]

On Tue, Oct 21, 2025 at 02:33:39PM +0200, David Hildenbrand wrote:
> On 21.10.25 08:35, Kiryl Shutsemau wrote:
> > From: Kiryl Shutsemau <kas@kernel.org>
> > 
> > Accesses within VMA, but beyond i_size rounded up to PAGE_SIZE are
> > supposed to generate SIGBUS.
> > 
> > This behavior might not be respected on truncation.
> > 
> > During truncation, the kernel splits a large folio in order to reclaim
> > memory. As a side effect, it unmaps the folio and destroys PMD mappings
> > of the folio. The folio will be refaulted as PTEs and SIGBUS semantics
> > are preserved.
> > 
> > However, if the split fails, PMD mappings are preserved and the user
> > will not receive SIGBUS on any accesses within the PMD.
> > 
> > Unmap the folio on split failure. It will lead to refault as PTEs and
> > preserve SIGBUS semantics.
> > 
> > Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
> > ---
> >   mm/truncate.c | 29 ++++++++++++++++++++++++++---
> >   1 file changed, 26 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index 91eb92a5ce4f..cdb698b5f7fa 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -177,6 +177,28 @@ int truncate_inode_folio(struct address_space *mapping, struct folio *folio)
> >   	return 0;
> >   }
> > +static int try_folio_split_or_unmap(struct folio *folio, struct page *split_at)
> > +{
> > +	enum ttu_flags ttu_flags =
> > +		TTU_RMAP_LOCKED |
> > +		TTU_SYNC |
> > +		TTU_BATCH_FLUSH |
> 
> I recall that this flag interacts with try_to_unmap_flush() /
> try_to_unmap_flush_dirty().
> 
> See unmap_folio() as one example.
> 
> If so, aren't we missing such a call or is the flush implied already
> somehow?

My bad. TTU_RMAP_LOCKED also should not be there.

Will fix.

> > +		TTU_SPLIT_HUGE_PMD |
> > +		TTU_IGNORE_MLOCK;
> > +	int ret;
> > +
> > +	ret = try_folio_split(folio, split_at, NULL);
> > +
> > +	/*
> > +	 * If the split fails, unmap the folio, so it will be refaulted
> > +	 * with PTEs to respect SIGBUS semantics.
> > +	 */
> > +	if (ret)
> > +		try_to_unmap(folio, ttu_flags);
> 
> Just wondering: do we want to check whether the folio is now actually
> completely unmapped through !folio_mapped() and try to handle if it isn't
> (maybe just warn? Don't know)
> 
> We usually check after try_to_unmap() whether we actually found all mappings
> (see unmap_poisoned_folio()). I recall some corner cases where unmapping
> could fail, but I don't remember whether that's specific to anonymous pages
> only.

I will add WARN_ON(folio_mapped(folio)).

> 
> > +
> > +	return ret;
> > +}
> > +
> >   /*
> >    * Handle partial folios.  The folio may be entirely within the
> >    * range if a split has raced with us.  If not, we zero the part of the
> > @@ -224,7 +246,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> >   		return true;
> >   	split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
> > -	if (!try_folio_split(folio, split_at, NULL)) {
> > +	if (!try_folio_split_or_unmap(folio, split_at)) {
> >   		/*
> >   		 * try to split at offset + length to make sure folios within
> >   		 * the range can be dropped, especially to avoid memory waste
> > @@ -249,12 +271,13 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> >   			goto out;
> >   		/*
> > +		 * Split the folio.
> 
> I'd drop that. It's not particularly helpful given that we call
> try_folio_split_or_unmap() and mention further above "try to split at
> offset".

Okay.

> Nothing else jumped at me!

Thanks for the review!

-- 
  Kiryl Shutsemau / Kirill A. Shutemov