Re: [PATCH v4 3/4] mm/truncate: fix data loss when splitting straddling large folios fails

[Zhang Yi]

On 9/23/2026 1:27 AM, Zi Yan wrote:
> On Tue Sep 22, 2026 at 7:07 AM EDT, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>>
>> truncate_inode_partial_folio() splits a large folio so that the caller's
>> truncate loop can drop the in-range sub-folios while keeping the
>> out-of-range tail. The first split at the punch start edge is
>> non-uniform, which leaves the sub-folio at the truncation end edge as
>> large as possible, this means it may still straddle the range, holding
>> both zeroed in-range and valid out-of-range data. The function then
>> attempts a second split at offset + length to isolate that tail.
>>
>> If the second split fails the straddling sub-folio stays merged. The
>> function returned true unconditionally on all exit paths of the success
>> block, telling the caller it was fully handled. The caller kept its
>> default end and the truncate loop truncated every sub-folio below it,
>> including the merged straddler, discarding the valid out-of-range tail.
>>
>> For example, a 4-page order-2 folio punched from offset 0 to the middle
>> of the last page:
>>
>>   truncate_inode_pages_range()
>>     truncate_inode_partial_folio()      # same_folio == true
>>       1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
>>       folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
>>       2nd split of folio2 fails / cannot lock
>>       return true                       # BUG: caller keeps default end
>>     end = 3
>>     loop truncates p0, p1, p2-3        # p3's valid tail is lost
>>
>> This became reachable after commit 7460b470a131 ("mm/truncate: use
>> folio_split() in truncate operation") replaced the atomic split_folio()
>> with folio_split(), whose non-uniform split can partially split a folio
>> and leave the end edge merged.
>>
>> It has gone unnoticed because a dirty large folio normally carries the
>> filesystem's private data, for example buffer_head, so
>> filemap_release_folio() fails on a dirty folio and folio_split() aborts
>> with -EBUSY before any split, leaving the straddler safely unsplit. The
>> bug is only reachable on paths that produce dirty large folios without
>> filesystem private data, and it was caught on the upcoming ext4 iomap
>> buffered I/O path when no ifs is attached.
>>
>> Rework the contract so the caller is told the folio range to discard:
>>
>>   - Add pgoff_t *pstart and *pend out-parameters that receive the folio
>>     range fully covered by [lstart, lend] after any split (or none),
>>     aligned inwards to min_order, i.e. the folios wholly within the
>>     range and safe to discard.
>>
>>   - Report a reliable end position to the caller. The straddler is
>>     looked up at an index aligned inwards to the mapping minimum folio
>>     order, and *pend is set to that boundary on success. If nothing
>>     covers the boundary, discarding up to it stays safe. If the
>>     straddler is locked by someone else, fall back to folio->index.
>>     This best-effort fallback may leave the in-range sub-folios to a
>>     later pass but never discards the out-of-range tail. If the
>>     straddler cannot be split, fall back to folio2->index so the caller
>>     keeps the out-of-range tail.
>>
>>   - Rename the byte-range parameters start/end to lstart/lend to better
>>     express their semantics.
>>
>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
>> &pstart for the folio at the start edge and &pend for the folio at the
>> end edge, so the truncate loop drops exactly the fully covered pages and
>> never touches a straddling folio that still holds valid out-of-range
>> data.
>>
>> Suggested-by: Brian Foster <bfoster@xxxxxxxxxx>
>> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
>> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
>> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
>> ---
>>  mm/internal.h |  4 +--
>>  mm/shmem.c    | 13 +++-----
>>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>>  3 files changed, 68 insertions(+), 37 deletions(-)
> 
> <snip>
>>
>> @@ -251,6 +264,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> 
> Add more context:
> 
> 	if (!folio_test_large(folio))
>>  		return true;
>>  
>>  	min_order = mapping_min_folio_order(folio->mapping);
>> +	min_nrbytes = mapping_min_folio_nrbytes(folio->mapping);
>>  	split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
>>  	if (!folio_split_or_unmap(folio, split_at, min_order)) {
>>  		/*
>> @@ -259,34 +273,57 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>  		 * for shmem truncate
>>  		 */
>>  		struct folio *folio2;
>> -		pgoff_t end_idx;
>> +		pgoff_t end, aligned_end = round_down(pos + offset + length,
>> +						min_nrbytes) >> PAGE_SHIFT;
> 
> <snip>
> 
>> +		/* Already at the minimum order, nothing to split */
>> +		if (folio_order(folio2) == min_order)
>> +			goto out_put;
> 
> In the above, folio_test_large() is used to determine whether folio
> needs to be split or not, but here folio_order() == min_order is used.
> Should the above "if (!folio_test_large(folio))" be changed to use
> min_order check to match the check here?
> 

Yeah, this makes sense to me.

Thanks,
Yi.