Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
From: Zhang Yi
Date: Sat Sep 05 2026 - 05:16:13 EST
On 9/5/2026 3:31 AM, Zi Yan wrote:
On Thu Sep 3, 2026 at 7:50 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() -> iomap_release_folio() returns false 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.
Thank you for the analysis.
Rework the contract so the caller is told where to stop instead of
silently truncating the straddler:
- Return true only when a split occurred, false otherwise. This
clarifies the existing confusing return value semantics.
Should we do "return false" for not split case as a minmal fix first?
Something like below. A second patch can optimize on top of it. Let me
know if I miss anything.
BTW, Claude also mentioned that if min_order > 0 and end is not aligned
to 1UL << min_order, there could be some issue. So
ret = !folio_split_or_unmap(folio2, split_at2, min_order);
should be
unsigned long idx2 = PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE;
ret = !folio_split_or_unmap(folio2, split_at2, min_order) &&
IS_ALIGNED(idx2, 1UL << min_order);
?
Hi Zi Yan,
Thanks for the minimal fix. I agree with the core observation — the
tail is only really isolated when the second split and the boundary
alignment both cooperate. However, I'd like to point out a trade-off:
the "return false to make the caller skip the folio" mechanism leads
to an incorrect 'start' in truncate_inode_pages_range() and over-keeps
the in-range sub-folios.
The problem is that the false return value was designed for the case
where the folio is unsplit. Look at the caller:
if (!truncate_inode_partial_folio(folio, lstart, lend)) {
start = folio_next_index(folio);
if (same_folio)
end = folio->index;
}
start = folio_next_index(folio); end = folio->index only makes sense
when folio is still the whole folio. But after the first split
succeeds, the caller's folio reference has already been transferred to
the sub-folio containing split_at, so folio is no longer the whole
folio.
Concretely, take the commit's example: a 4-page order-2 folio
[p0 p1 p2 p3], punched from offset 0 into the middle of p3,
min_order == 0:
[p0 p1 p2 p3] --1st split @p0--> [p0] [p1] [p2-p3]
folio now points to [p0]
folio2 = [p2-p3] # p2 zeroed, p3 tail valid
2nd split of [p2-p3] fails # e.g. -EBUSY, or can't lock
With your fix, ret becomes false, so the caller runs:
start = folio_next_index([p0]) = 1;
end = folio->index = 0;
The truncate loop then does while (index < end) -> 1 < 0 -> nothing,
and [p0] and [p1] are left in the page cache, even though they are
fully inside the punched range and should have been dropped.
To be fair, this will not lead to any data-corruption problem because
p0 and p1 are already zeroed. So as a minimal fix to stop the data
loss, it is acceptable. But it still leaves the in-range sub-folios
behind and wastes memory, which somewhat reduces the benefit of
splitting the folio. So I don't think change the return value alone can
solve this problem.
What do you think?
Thanks,
Yi.
From 564fd753071be9d59d9e45e4609a812bb81f778a Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@xxxxxxxxxx>
Date: Fri, 4 Sep 2026 15:25:06 -0400
Subject: [PATCH] fix unsuccessful folio2 split
Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
---
mm/truncate.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/mm/truncate.c b/mm/truncate.c
index b58ba940be474..2c575f5e61889 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -259,6 +259,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
* for shmem truncate
*/
struct folio *folio2;
+ bool ret = true;
if (offset + length == size)
goto no_split;
@@ -273,19 +274,23 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
if (!folio_test_large(folio2))
goto out;
- if (!folio_trylock(folio2))
+ if (!folio_trylock(folio2)) {
+ ret = false;
goto out;
+ }
/* make sure folio2 is large and does not change its mapping */
if (folio_test_large(folio2) &&
folio2->mapping == folio->mapping)
- folio_split_or_unmap(folio2, split_at2, min_order);
+ ret = !folio_split_or_unmap(folio2, split_at2, min_order);
+ else
+ ret = false;
folio_unlock(folio2);
out:
folio_put(folio2);
no_split:
- return true;
+ return ret;
}
if (folio_test_dirty(folio))
return false;