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

[Hugh Dickins <hughd@google.com>]

On Thu, 23 Oct 2025, 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>

It's taking me too long to understand what truncate_inode_partial_folio()
had become before your changes, to be very sure of your changes to it.

But if your commit does indeed achieve what's intended, then I have
no objection to it applying to shmem/tmpfs as well as other filesystems:
we always hope that a split will succeed, so I don't mind you tightening
up what is done when it fails.

However, a few points that have occurred to me...

If 1/2's exception for shmem/tmpfs huge=always does the simple thing,
of just judging by whether a huge page is already there in the file
(without reference to mount option), which I think is okay: then
this 2/2 will not be doing anything useful on shmem/tmpfs, because
when the split fails, the huge page will remain, and after 2/2's
unmap it will just get remapped by PMD again afterwards, so why
bother to unmap at all in the shmem/tmpfs case?.

But it's arguable whether it would then be worth making an
exception for shmem/tmpfs here in 2/2 - how much do we care about
optimizing failed splits?  At least a comment I guess, but you
might prefer to do it quite differently.

Aside from shmem/tmpfs, it does seem to me that this patch is
doing more work than it needs to (but how many lines of source
do we want to add to avoid doing work in the failed split case?):

The intent is to enable SIGBUS beyond EOF: but the changes are
being applied unnecessarily to hole-punch in addition to truncation.

Does the folio2 part actually need to unmap again?  And if it does,
then what about when its trylock failed?  But it's hole-punch anyway.

And a final nit: I'd delete that WARN_ON(folio_mapped(folio)) myself,
all it could ever achieve is perhaps a very rare syzbot report which
nobody would want to spend time on.

Hugh