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

[David Hildenbrand <david@redhat.com>]

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?

> +		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.

> +
> +	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".

Nothing else jumped at me!

-- 
Cheers

David / dhildenb