Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()

On 9/5/2026 8:41 PM, Zi Yan wrote:

On Sat Sep 5, 2026 at 5:15 AM EDT, Zhang Yi wrote:

quoted

On 9/5/2026 3:31 AM, Zi Yan wrote:

quoted

On Thu Sep 3, 2026 at 7:50 AM EDT, Zhang Yi wrote:

quoted

From: Zhang Yi <yi.zhang@huawei.com>

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.

quoted

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?

Understood.

Alternatives are changing folio_split() to make sure folio2 is split.
 From weakest guarantee to strongest guarantee:

1. make folio_split() return with folio2 locked: but others can still
put a ref on folio2 to fail the subsequent folio2 split.

2. make folio_split() accept two split_at, folio_split() does both folio
and folio2 split internally: but if folio2 split require an xa_node and
the allocation fails, folio2 split can still fail.

3. make folio_split() accept two split_at and preallocate two xa_node
upfront: this should guarantee folio_split() to either split both folio
and folio2 or split nothing, but it specializes folio_split() for
truncate.

I'm afraid even guaranteeing a successful folio2 split is not enough
here. As Joanne pointed out [1], when min_order is not 0, even if folio2
is successfully split and returns true, the folio remains a large folio.
In the outer loop, 'end' still cannot point to the head of the split
folio2, so the entire folio is still incorrectly dropped as a whole,
losing the valid data at the tail.

[1] 
https://lore.kernel.org/linux-mm/CAJnrk1bQYUe6+1ryyJur5EEnZYrC+_5AYsy=OWzVRgD4202y1g@mail.gmail.com/ (local)

I guess for now it might be better to make truncate to handle the
folio2-not-split situation.

Yeah, agree.

Thanks,
Yi.