Thread (12 messages) flat view 12 messages, 6 authors, 1d ago

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

From: "Zi Yan" <ziy@nvidia.com>
Date: 2026-09-04 19:31:23
Also in: linux-fsdevel, linux-mm, lkml
Subsystem: memory management, page cache, the rest · Maintainers: Andrew Morton, Matthew Wilcox, Jan Kara, Linus Torvalds

On Thu Sep 3, 2026 at 7:50 AM EDT, Zhang Yi wrote:
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.
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);

?

From 564fd753071be9d59d9e45e4609a812bb81f778a Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 4 Sep 2026 15:25:06 -0400
Subject: [PATCH] fix unsuccessful folio2 split

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 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;
-- 
2.53.0


>
>   - Add an optional out-parameter pgoff_t *end, set to the index of the
>     folio that contains @lend and must be kept by the caller's loop. It
>     is only written when the folio actually straddles @lend. On the
>     success path it defaults to the page index of the end edge and is
>     refined to folio2->index when the second split fails to isolate the
>     tail.
>
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new @end output and to separate byte offsets from
>     folio indices.
>
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
> only when the folio straddles lend. After all, no caller discards a
> straddling folio anymore, the in-range cleanly-split sub-folios below it
> are still dropped.
>
> Suggested-by: Brian Foster <bfoster@redhat.com>
> 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@huawei.com>
> ---
>  mm/internal.h |  4 ++--
>  mm/shmem.c    | 12 +++++------
>  mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
>  3 files changed, 41 insertions(+), 32 deletions(-)
>


-- 
Best Regards,
Yan, Zi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help