mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
@ 2026-09-09  6:23 Zhang Yi
  2026-09-09 12:22 ` Jan Kara
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Zhang Yi @ 2026-09-09  6:23 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-fsdevel, linux-kernel, linux-ext4, akpm, david, ljs, liam,
	vbabka, rppt, surenb, mhocko, hughd, baolin.wang, willy, jack,
	ziy, bfoster, joannelkoong, djwong, yi.zhang, yi.zhang,
	yizhang089, yangerkun, chengzhihao1, wangkefeng.wang, yukuai

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.

In addition, even when both splits succeed, data can still be lost when
the mapping's minimum folio order (min_order) is non-zero. folio_split()
stops at min_order instead of order 0, so the sub-folio containing a
split point stays aligned to 1 << min_order rather than to a page. The
original success path left start at the page-aligned head of the range
and set end to the exact page index of the end edge, neither of which is
a folio boundary in general. Either one could land inside the large
folio at its edge, and the truncate loop would drop that straddling
folio together with its valid out-of-range tail.

For example, a 64K (order-4) folio with min_order = 2 punched from
offset 0 to 36K:

  truncate_inode_pages_range()
    truncate_inode_partial_folio()        # same_folio == true
      1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
      folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
      2nd split of folio2 -> [p8-p11, p12-p15]   # success
      end = p9                           # BUG: p9 inside [p8-p11]
    loop truncates ... p8-p11           # p9-p11's valid tail is lost

Rework the contract so the caller is told the page range to discard:

  - Return true only when a split occurred, false otherwise. This
    clarifies the existing confusing return value semantics.

  - Add pgoff_t *pstart and *pend out-parameters that receive the page
    range fully covered by [lstart, lend] after any split (or none),
    i.e. the pages wholly within the range and safe to discard. They are
    aligned up (pstart) and down (pend) to the mapping's minimum folio
    order so they always fall on a folio boundary.

  - Rename the byte-range parameters start/end to lstart/lend to avoid
    clashing with the new outputs and to separate byte offsets from
    folio indices.

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@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>
---
v1->v2:
 - Export pstart as a new parameter so that the generic and shmem
   truncate paths don't need to recompute the start value from the
   return value. (Brian)
 - When min_order is nonzero, align [pstart, pend] to the inner
   boundaries of the folio to ensure they do not point into the middle
   of a large folio, which could otherwise cause valid data within the
   folio to be incorrectly cleared. (Joanne)

v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/

 mm/internal.h |  4 +--
 mm/shmem.c    | 13 +++-----
 mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
 3 files changed, 67 insertions(+), 38 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 68db5abd0a4c..6e6ad3187378 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -627,8 +627,8 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
 unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
 		pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
 int truncate_inode_folio(struct address_space *mapping, struct folio *folio);
-bool truncate_inode_partial_folio(struct folio *folio, loff_t start,
-		loff_t end);
+bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
+		loff_t lend, pgoff_t *pstart, pgoff_t *pend);
 long mapping_evict_folio(struct address_space *mapping, struct folio *folio);
 unsigned long mapping_try_invalidate(struct address_space *mapping,
 		pgoff_t start, pgoff_t end, unsigned long *nr_failed);
diff --git a/mm/shmem.c b/mm/shmem.c
index 89a1495e55f7..4efbbbba5da5 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1175,11 +1175,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
 	if (folio) {
 		same_folio = lend < folio_next_pos(folio);
 		folio_mark_dirty(folio);
-		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
-			start = folio_next_index(folio);
-			if (same_folio)
-				end = folio->index;
-		}
+		truncate_inode_partial_folio(folio, lstart, lend, &start,
+					     same_folio ? &end : NULL);
 		folio_unlock(folio);
 		folio_put(folio);
 		folio = NULL;
@@ -1189,8 +1186,7 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
 		folio = shmem_get_partial_folio(inode, lend >> PAGE_SHIFT);
 	if (folio) {
 		folio_mark_dirty(folio);
-		if (!truncate_inode_partial_folio(folio, lstart, lend))
-			end = folio->index;
+		truncate_inode_partial_folio(folio, lstart, lend, NULL, &end);
 		folio_unlock(folio);
 		folio_put(folio);
 	}
@@ -1258,7 +1254,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
 
 				if (!folio_test_large(folio)) {
 					truncate_inode_folio(mapping, folio);
-				} else if (truncate_inode_partial_folio(folio, lstart, lend)) {
+				} else if (truncate_inode_partial_folio(folio,
+							lstart, lend, NULL, NULL)) {
 					/*
 					 * If we split a page, reset the loop so
 					 * that we pick up the new sub pages.
diff --git a/mm/truncate.c b/mm/truncate.c
index b58ba940be47..d88a1b159084 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -206,35 +206,41 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
 /*
  * 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
- * folio that's within the [start, end] range, and then split the folio if
+ * folio that's within the [lstart, lend] range, and then split the folio if
  * it's large.  split_page_range() will discard pages which now lie beyond
  * i_size, and we rely on the caller to discard pages which lie within a
  * newly created hole.
  *
- * Returns false if splitting failed so the caller can avoid
- * discarding the entire folio which is stubbornly unsplit.
+ * When @pstart and/or @pend are non-NULL they receive the indexes of the
+ * page range fully covered by [lstart, lend] after any split (or none),
+ * i.e. the range of pages that are wholly within [lstart, lend] and so safe
+ * to discard.
+ *
+ * Return %true if the folio was split, %false otherwise.
  */
-bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
+bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
+				  loff_t lend, pgoff_t *pstart, pgoff_t *pend)
 {
 	loff_t pos = folio_pos(folio);
 	size_t size = folio_size(folio);
 	unsigned int offset, length;
 	struct page *split_at, *split_at2;
+	unsigned long min_nrbytes;
 	unsigned int min_order;
 
-	if (pos < start)
-		offset = start - pos;
+	if (pos < lstart)
+		offset = lstart - pos;
 	else
 		offset = 0;
-	if (pos + size <= (u64)end)
+	if (pos + size <= (u64)lend)
 		length = size - offset;
 	else
-		length = end + 1 - pos - offset;
+		length = lend + 1 - pos - offset;
 
 	folio_wait_writeback(folio);
 	if (length == size) {
 		truncate_inode_folio(folio->mapping, folio);
-		return true;
+		goto no_split;
 	}
 
 	/*
@@ -248,9 +254,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 	if (folio_needs_release(folio))
 		folio_invalidate(folio, offset, length);
 	if (!folio_test_large(folio))
-		return true;
+		goto no_split;
 
 	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,38 +266,67 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 		 * for shmem truncate
 		 */
 		struct folio *folio2;
+		bool tail_isolated = true;
+
+		if (pend)
+			*pend = round_down(pos + offset + length,
+					   min_nrbytes) >> PAGE_SHIFT;
 
 		if (offset + length == size)
-			goto no_split;
-
+			goto split;
+retry:
 		split_at2 = folio_page(folio,
 				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
 		folio2 = page_folio(split_at2);
 
 		if (!folio_try_get(folio2))
-			goto no_split;
+			goto split;
 
 		if (!folio_test_large(folio2))
 			goto out;
 
-		if (!folio_trylock(folio2))
+		if (!folio_trylock(folio2)) {
+			tail_isolated = false;
 			goto out;
+		}
+
+		/*
+		 * split_at2 may no longer belong to folio2 due to concurrent
+		 * split. Retry to find the correct folio in case it's still
+		 * large.
+		 */
+		if (page_folio(split_at2) != folio2) {
+			folio_unlock(folio2);
+			folio_put(folio2);
+			goto retry;
+		}
 
 		/* 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);
+		    folio2->mapping == folio->mapping &&
+		    folio_split_or_unmap(folio2, split_at2, min_order))
+			tail_isolated = false;
 
 		folio_unlock(folio2);
 out:
+		if (!tail_isolated && pend)
+			*pend = folio2->index;
 		folio_put(folio2);
-no_split:
+split:
+		if (pstart)
+			*pstart = round_up(pos + offset,
+					   min_nrbytes) >> PAGE_SHIFT;
 		return true;
 	}
-	if (folio_test_dirty(folio))
-		return false;
-	truncate_inode_folio(folio->mapping, folio);
-	return true;
+	if (!folio_test_dirty(folio))
+		truncate_inode_folio(folio->mapping, folio);
+no_split:
+	if (pstart)
+		*pstart = offset ? folio_next_index(folio) : folio->index;
+	if (pend)
+		*pend = (pos + size > (u64)lend) ? folio->index :
+						   folio_next_index(folio);
+	return false;
 }
 
 /*
@@ -413,11 +449,8 @@ void truncate_inode_pages_range(struct address_space *mapping,
 	folio = __filemap_get_folio(mapping, lstart >> PAGE_SHIFT, FGP_LOCK, 0);
 	if (!IS_ERR(folio)) {
 		same_folio = lend < folio_next_pos(folio);
-		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
-			start = folio_next_index(folio);
-			if (same_folio)
-				end = folio->index;
-		}
+		truncate_inode_partial_folio(folio, lstart, lend, &start,
+					     same_folio ? &end : NULL);
 		folio_unlock(folio);
 		folio_put(folio);
 		folio = NULL;
@@ -427,8 +460,7 @@ void truncate_inode_pages_range(struct address_space *mapping,
 		folio = __filemap_get_folio(mapping, lend >> PAGE_SHIFT,
 						FGP_LOCK, 0);
 		if (!IS_ERR(folio)) {
-			if (!truncate_inode_partial_folio(folio, lstart, lend))
-				end = folio->index;
+			truncate_inode_partial_folio(folio, lstart, lend, NULL, &end);
 			folio_unlock(folio);
 			folio_put(folio);
 		}
-- 
2.52.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09  6:23 [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios Zhang Yi
@ 2026-09-09 12:22 ` Jan Kara
  2026-09-09 18:29 ` Joanne Koong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2026-09-09 12:22 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, bfoster, joannelkoong, djwong, yi.zhang,
	yizhang089, yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Wed 09-09-26 14:23:39, 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.
> 
> In addition, even when both splits succeed, data can still be lost when
> the mapping's minimum folio order (min_order) is non-zero. folio_split()
> stops at min_order instead of order 0, so the sub-folio containing a
> split point stays aligned to 1 << min_order rather than to a page. The
> original success path left start at the page-aligned head of the range
> and set end to the exact page index of the end edge, neither of which is
> a folio boundary in general. Either one could land inside the large
> folio at its edge, and the truncate loop would drop that straddling
> folio together with its valid out-of-range tail.
> 
> For example, a 64K (order-4) folio with min_order = 2 punched from
> offset 0 to 36K:
> 
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()        # same_folio == true
>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>       end = p9                           # BUG: p9 inside [p8-p11]
>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
> 
> Rework the contract so the caller is told the page range to discard:
> 
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.
> 
>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
>     range fully covered by [lstart, lend] after any split (or none),
>     i.e. the pages wholly within the range and safe to discard. They are
>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>     order so they always fall on a folio boundary.
> 
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new outputs and to separate byte offsets from
>     folio indices.
> 
> 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@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>

Looks good to me. Thanks! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> v1->v2:
>  - Export pstart as a new parameter so that the generic and shmem
>    truncate paths don't need to recompute the start value from the
>    return value. (Brian)
>  - When min_order is nonzero, align [pstart, pend] to the inner
>    boundaries of the folio to ensure they do not point into the middle
>    of a large folio, which could otherwise cause valid data within the
>    folio to be incorrectly cleared. (Joanne)
> 
> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/
> 
>  mm/internal.h |  4 +--
>  mm/shmem.c    | 13 +++-----
>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>  3 files changed, 67 insertions(+), 38 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 68db5abd0a4c..6e6ad3187378 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -627,8 +627,8 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>  unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>  		pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
>  int truncate_inode_folio(struct address_space *mapping, struct folio *folio);
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start,
> -		loff_t end);
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +		loff_t lend, pgoff_t *pstart, pgoff_t *pend);
>  long mapping_evict_folio(struct address_space *mapping, struct folio *folio);
>  unsigned long mapping_try_invalidate(struct address_space *mapping,
>  		pgoff_t start, pgoff_t end, unsigned long *nr_failed);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 89a1495e55f7..4efbbbba5da5 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1175,11 +1175,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  	if (folio) {
>  		same_folio = lend < folio_next_pos(folio);
>  		folio_mark_dirty(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
> -			start = folio_next_index(folio);
> -			if (same_folio)
> -				end = folio->index;
> -		}
> +		truncate_inode_partial_folio(folio, lstart, lend, &start,
> +					     same_folio ? &end : NULL);
>  		folio_unlock(folio);
>  		folio_put(folio);
>  		folio = NULL;
> @@ -1189,8 +1186,7 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  		folio = shmem_get_partial_folio(inode, lend >> PAGE_SHIFT);
>  	if (folio) {
>  		folio_mark_dirty(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend))
> -			end = folio->index;
> +		truncate_inode_partial_folio(folio, lstart, lend, NULL, &end);
>  		folio_unlock(folio);
>  		folio_put(folio);
>  	}
> @@ -1258,7 +1254,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  
>  				if (!folio_test_large(folio)) {
>  					truncate_inode_folio(mapping, folio);
> -				} else if (truncate_inode_partial_folio(folio, lstart, lend)) {
> +				} else if (truncate_inode_partial_folio(folio,
> +							lstart, lend, NULL, NULL)) {
>  					/*
>  					 * If we split a page, reset the loop so
>  					 * that we pick up the new sub pages.
> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be47..d88a1b159084 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -206,35 +206,41 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>  /*
>   * 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
> - * folio that's within the [start, end] range, and then split the folio if
> + * folio that's within the [lstart, lend] range, and then split the folio if
>   * it's large.  split_page_range() will discard pages which now lie beyond
>   * i_size, and we rely on the caller to discard pages which lie within a
>   * newly created hole.
>   *
> - * Returns false if splitting failed so the caller can avoid
> - * discarding the entire folio which is stubbornly unsplit.
> + * When @pstart and/or @pend are non-NULL they receive the indexes of the
> + * page range fully covered by [lstart, lend] after any split (or none),
> + * i.e. the range of pages that are wholly within [lstart, lend] and so safe
> + * to discard.
> + *
> + * Return %true if the folio was split, %false otherwise.
>   */
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +				  loff_t lend, pgoff_t *pstart, pgoff_t *pend)
>  {
>  	loff_t pos = folio_pos(folio);
>  	size_t size = folio_size(folio);
>  	unsigned int offset, length;
>  	struct page *split_at, *split_at2;
> +	unsigned long min_nrbytes;
>  	unsigned int min_order;
>  
> -	if (pos < start)
> -		offset = start - pos;
> +	if (pos < lstart)
> +		offset = lstart - pos;
>  	else
>  		offset = 0;
> -	if (pos + size <= (u64)end)
> +	if (pos + size <= (u64)lend)
>  		length = size - offset;
>  	else
> -		length = end + 1 - pos - offset;
> +		length = lend + 1 - pos - offset;
>  
>  	folio_wait_writeback(folio);
>  	if (length == size) {
>  		truncate_inode_folio(folio->mapping, folio);
> -		return true;
> +		goto no_split;
>  	}
>  
>  	/*
> @@ -248,9 +254,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  	if (folio_needs_release(folio))
>  		folio_invalidate(folio, offset, length);
>  	if (!folio_test_large(folio))
> -		return true;
> +		goto no_split;
>  
>  	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,38 +266,67 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		bool tail_isolated = true;
> +
> +		if (pend)
> +			*pend = round_down(pos + offset + length,
> +					   min_nrbytes) >> PAGE_SHIFT;
>  
>  		if (offset + length == size)
> -			goto no_split;
> -
> +			goto split;
> +retry:
>  		split_at2 = folio_page(folio,
>  				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
>  		folio2 = page_folio(split_at2);
>  
>  		if (!folio_try_get(folio2))
> -			goto no_split;
> +			goto split;
>  
>  		if (!folio_test_large(folio2))
>  			goto out;
>  
> -		if (!folio_trylock(folio2))
> +		if (!folio_trylock(folio2)) {
> +			tail_isolated = false;
>  			goto out;
> +		}
> +
> +		/*
> +		 * split_at2 may no longer belong to folio2 due to concurrent
> +		 * split. Retry to find the correct folio in case it's still
> +		 * large.
> +		 */
> +		if (page_folio(split_at2) != folio2) {
> +			folio_unlock(folio2);
> +			folio_put(folio2);
> +			goto retry;
> +		}
>  
>  		/* 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);
> +		    folio2->mapping == folio->mapping &&
> +		    folio_split_or_unmap(folio2, split_at2, min_order))
> +			tail_isolated = false;
>  
>  		folio_unlock(folio2);
>  out:
> +		if (!tail_isolated && pend)
> +			*pend = folio2->index;
>  		folio_put(folio2);
> -no_split:
> +split:
> +		if (pstart)
> +			*pstart = round_up(pos + offset,
> +					   min_nrbytes) >> PAGE_SHIFT;
>  		return true;
>  	}
> -	if (folio_test_dirty(folio))
> -		return false;
> -	truncate_inode_folio(folio->mapping, folio);
> -	return true;
> +	if (!folio_test_dirty(folio))
> +		truncate_inode_folio(folio->mapping, folio);
> +no_split:
> +	if (pstart)
> +		*pstart = offset ? folio_next_index(folio) : folio->index;
> +	if (pend)
> +		*pend = (pos + size > (u64)lend) ? folio->index :
> +						   folio_next_index(folio);
> +	return false;
>  }
>  
>  /*
> @@ -413,11 +449,8 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  	folio = __filemap_get_folio(mapping, lstart >> PAGE_SHIFT, FGP_LOCK, 0);
>  	if (!IS_ERR(folio)) {
>  		same_folio = lend < folio_next_pos(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
> -			start = folio_next_index(folio);
> -			if (same_folio)
> -				end = folio->index;
> -		}
> +		truncate_inode_partial_folio(folio, lstart, lend, &start,
> +					     same_folio ? &end : NULL);
>  		folio_unlock(folio);
>  		folio_put(folio);
>  		folio = NULL;
> @@ -427,8 +460,7 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  		folio = __filemap_get_folio(mapping, lend >> PAGE_SHIFT,
>  						FGP_LOCK, 0);
>  		if (!IS_ERR(folio)) {
> -			if (!truncate_inode_partial_folio(folio, lstart, lend))
> -				end = folio->index;
> +			truncate_inode_partial_folio(folio, lstart, lend, NULL, &end);
>  			folio_unlock(folio);
>  			folio_put(folio);
>  		}
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09  6:23 [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios Zhang Yi
  2026-09-09 12:22 ` Jan Kara
@ 2026-09-09 18:29 ` Joanne Koong
  2026-09-11  8:25   ` Zhang Yi
  2026-09-09 19:18 ` Zi Yan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Joanne Koong @ 2026-09-09 18:29 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, bfoster, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Tue, Sep 8, 2026 at 11:31 PM Zhang Yi <yi.zhang@huaweicloud.com> 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.
>
> In addition, even when both splits succeed, data can still be lost when
> the mapping's minimum folio order (min_order) is non-zero. folio_split()
> stops at min_order instead of order 0, so the sub-folio containing a
> split point stays aligned to 1 << min_order rather than to a page. The
> original success path left start at the page-aligned head of the range
> and set end to the exact page index of the end edge, neither of which is
> a folio boundary in general. Either one could land inside the large
> folio at its edge, and the truncate loop would drop that straddling
> folio together with its valid out-of-range tail.
>
> For example, a 64K (order-4) folio with min_order = 2 punched from
> offset 0 to 36K:
>
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()        # same_folio == true
>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>       end = p9                           # BUG: p9 inside [p8-p11]
>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
>
> Rework the contract so the caller is told the page range to discard:
>
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.
>
>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
>     range fully covered by [lstart, lend] after any split (or none),
>     i.e. the pages wholly within the range and safe to discard. They are
>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>     order so they always fall on a folio boundary.
>
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new outputs and to separate byte offsets from
>     folio indices.
>
> 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@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>

Reviewed-by: Joanne Koong <joannelkoong@gmail.com>

> ---
> v1->v2:
>  - Export pstart as a new parameter so that the generic and shmem
>    truncate paths don't need to recompute the start value from the
>    return value. (Brian)
>  - When min_order is nonzero, align [pstart, pend] to the inner
>    boundaries of the folio to ensure they do not point into the middle
>    of a large folio, which could otherwise cause valid data within the
>    folio to be incorrectly cleared. (Joanne)
>
> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/
>
>  mm/internal.h |  4 +--
>  mm/shmem.c    | 13 +++-----
>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>  3 files changed, 67 insertions(+), 38 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 68db5abd0a4c..6e6ad3187378 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -627,8 +627,8 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>  unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>                 pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
>  int truncate_inode_folio(struct address_space *mapping, struct folio *folio);
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start,
> -               loff_t end);
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +               loff_t lend, pgoff_t *pstart, pgoff_t *pend);
>  long mapping_evict_folio(struct address_space *mapping, struct folio *folio);
>  unsigned long mapping_try_invalidate(struct address_space *mapping,
>                 pgoff_t start, pgoff_t end, unsigned long *nr_failed);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 89a1495e55f7..4efbbbba5da5 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1175,11 +1175,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>         if (folio) {
>                 same_folio = lend < folio_next_pos(folio);
>                 folio_mark_dirty(folio);
> -               if (!truncate_inode_partial_folio(folio, lstart, lend)) {
> -                       start = folio_next_index(folio);
> -                       if (same_folio)
> -                               end = folio->index;
> -               }
> +               truncate_inode_partial_folio(folio, lstart, lend, &start,
> +                                            same_folio ? &end : NULL);
>                 folio_unlock(folio);
>                 folio_put(folio);
>                 folio = NULL;
> @@ -1189,8 +1186,7 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>                 folio = shmem_get_partial_folio(inode, lend >> PAGE_SHIFT);
>         if (folio) {
>                 folio_mark_dirty(folio);
> -               if (!truncate_inode_partial_folio(folio, lstart, lend))
> -                       end = folio->index;
> +               truncate_inode_partial_folio(folio, lstart, lend, NULL, &end);
>                 folio_unlock(folio);
>                 folio_put(folio);
>         }
> @@ -1258,7 +1254,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>
>                                 if (!folio_test_large(folio)) {
>                                         truncate_inode_folio(mapping, folio);
> -                               } else if (truncate_inode_partial_folio(folio, lstart, lend)) {
> +                               } else if (truncate_inode_partial_folio(folio,
> +                                                       lstart, lend, NULL, NULL)) {
>                                         /*
>                                          * If we split a page, reset the loop so
>                                          * that we pick up the new sub pages.
> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be47..d88a1b159084 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -206,35 +206,41 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>  /*
>   * 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
> - * folio that's within the [start, end] range, and then split the folio if
> + * folio that's within the [lstart, lend] range, and then split the folio if
>   * it's large.  split_page_range() will discard pages which now lie beyond
>   * i_size, and we rely on the caller to discard pages which lie within a
>   * newly created hole.
>   *
> - * Returns false if splitting failed so the caller can avoid
> - * discarding the entire folio which is stubbornly unsplit.
> + * When @pstart and/or @pend are non-NULL they receive the indexes of the
> + * page range fully covered by [lstart, lend] after any split (or none),
> + * i.e. the range of pages that are wholly within [lstart, lend] and so safe
> + * to discard.

I think with themin_order > 0 case, pages wholly within [lstart, lend]
can still be excluded, so maybe worth editing this to reflect that.

Thanks,
Joanne

> + *
> + * Return %true if the folio was split, %false otherwise.
>   */
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +                                 loff_t lend, pgoff_t *pstart, pgoff_t *pend)
>  {
>         loff_t pos = folio_pos(folio);
>         size_t size = folio_size(folio);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09  6:23 [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios Zhang Yi
  2026-09-09 12:22 ` Jan Kara
  2026-09-09 18:29 ` Joanne Koong
@ 2026-09-09 19:18 ` Zi Yan
  2026-09-11  9:26   ` Zhang Yi
  2026-09-09 23:14 ` Andrew Morton
  2026-09-10 16:26 ` Brian Foster
  4 siblings, 1 reply; 11+ messages in thread
From: Zi Yan @ 2026-09-09 19:18 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, bfoster, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On 9 Sep 2026, at 2:23, 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 fixing it.

>
> In addition, even when both splits succeed, data can still be lost when
> the mapping's minimum folio order (min_order) is non-zero. folio_split()
> stops at min_order instead of order 0, so the sub-folio containing a
> split point stays aligned to 1 << min_order rather than to a page. The
> original success path left start at the page-aligned head of the range
> and set end to the exact page index of the end edge, neither of which is
> a folio boundary in general. Either one could land inside the large
> folio at its edge, and the truncate loop would drop that straddling
> folio together with its valid out-of-range tail.
>
> For example, a 64K (order-4) folio with min_order = 2 punched from
> offset 0 to 36K:
>
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()        # same_folio == true
>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>       end = p9                           # BUG: p9 inside [p8-p11]
>     loop truncates ... p8-p11           # p9-p11's valid tail is lost

Should min_order > 0 fix come as a separate patch first? min_order>0
was introduced by commit e220917fa5077 ("mm: split a folio in minimum
folio order chunks") and it is before folio_split(). The old
split_huge_page_to_list_to_order() can cause a similar issue?


<snip>

>     i.e. the pages wholly within the range and safe to discard. They are
>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>     order so they always fall on a folio boundary.

Basically, to fix min_order issue alone, truncate_inode_partial_folio()
needs to align pstart and pend to 1UL << min_order?



Best Regards,
Yan, Zi

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09  6:23 [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios Zhang Yi
                   ` (2 preceding siblings ...)
  2026-09-09 19:18 ` Zi Yan
@ 2026-09-09 23:14 ` Andrew Morton
  2026-09-10  7:18   ` Zhang Yi
  2026-09-10 16:26 ` Brian Foster
  4 siblings, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2026-09-09 23:14 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, david, ljs,
	liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang, willy,
	jack, ziy, bfoster, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Wed,  9 Sep 2026 14:23:39 +0800 Zhang Yi <yi.zhang@huaweicloud.com> 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.

So is there any way of being hit by these issues in 7.2 and earlier?

>  mm/internal.h |  4 +--
>  mm/shmem.c    | 13 +++-----
>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>  3 files changed, 67 insertions(+), 38 deletions(-)

I hope not, because that's quite a lump of code to be backporting.

Thanks.  I'll queue it for testing and shall await your response to Zi
Yan's questions.

I see you've received Sashiko's feedback.  Please let us know if
there's any validity to it.

	https://sashiko.dev/#/patchset/20260909062339.473816-1-yi.zhang@huaweicloud.com



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09 23:14 ` Andrew Morton
@ 2026-09-10  7:18   ` Zhang Yi
  0 siblings, 0 replies; 11+ messages in thread
From: Zhang Yi @ 2026-09-10  7:18 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, david, ljs,
	liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang, willy,
	jack, ziy, bfoster, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On 9/10/2026 7:14 AM, Andrew Morton wrote:
> On Wed,  9 Sep 2026 14:23:39 +0800 Zhang Yi <yi.zhang@huaweicloud.com> 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.
> 
> So is there any way of being hit by these issues in 7.2 and earlier?
> 
>>  mm/internal.h |  4 +--
>>  mm/shmem.c    | 13 +++-----
>>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>>  3 files changed, 67 insertions(+), 38 deletions(-)
> 
> I hope not, because that's quite a lump of code to be backporting.

I took a rough look at a few existing filesystems and had an LLM help
put things together.

Actually triggering it needs all of:
(1) a large folio straddling the punch/truncate boundary, so the
    in-range part is zeroed and the out-of-range part holds valid data;
(2) that folio dirty, so the out-of-range tail exists only in the
    pagecache;
(3) no filesystem private data on the folio (or ->release_folio()
    returning success), so the first folio_split() in
    truncate_inode_partial_folio() can succeed.

Filesystems that enable large folios fall into three groups, none of
which can currently satisfy all three:

- buffer_head / private-based (ext4, btrfs, ...): a dirty folio
  always carries fs private state (buffer_head, extent state), so
  ->release_folio() returns false and folio_split() aborts with -EBUSY
  before the first split — condition (3) fails.
- netfs (afs, nfs, smb, ...): AP_RELEASE_ALWAYS is set but
  netfs_release_folio() returns false on a dirty folio — same effect,
  condition (3) fails.
- iomap (xfs, zonefs, erofs): these can have dirty large folios
  without private data, but none reach the unsafe window — xfs always
  runs filemap_write_and_wait_range() (xfs_flush_unmap_range) before
  punching, so the boundary folio is clean; zonefs has no punch-hole;
  erofs is read-only. With the tail already on disk (clean), discarding
  the straddling folio loses nothing.

So on 7.2 and earlier I cannot find a reachable trigger path. It only
becomes reachable on the upcoming ext4 + iomap buffered-I/O path, which
for the first time combines large folios + no private data on
fully-dirtied folios + missing pre-punch writeback.

> 
> Thanks.  I'll queue it for testing and shall await your response to Zi
> Yan's questions.
> 
> I see you've received Sashiko's feedback.  Please let us know if
> there's any validity to it.
> 
> 	https://sashiko.dev/#/patchset/20260909062339.473816-1-yi.zhang@huaweicloud.com
> 

I took an initial look at Sashiko's review report. I think the
issue is valid — the current fix can get the wrong folio2 index
when there's a concurrent split. I need to think more about how
to fix it properly.

Thanks,
Yi.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09  6:23 [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios Zhang Yi
                   ` (3 preceding siblings ...)
  2026-09-09 23:14 ` Andrew Morton
@ 2026-09-10 16:26 ` Brian Foster
  2026-09-11  7:34   ` Zhang Yi
  4 siblings, 1 reply; 11+ messages in thread
From: Brian Foster @ 2026-09-10 16:26 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Wed, Sep 09, 2026 at 02:23:39PM +0800, 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.
> 
> In addition, even when both splits succeed, data can still be lost when
> the mapping's minimum folio order (min_order) is non-zero. folio_split()
> stops at min_order instead of order 0, so the sub-folio containing a
> split point stays aligned to 1 << min_order rather than to a page. The
> original success path left start at the page-aligned head of the range
> and set end to the exact page index of the end edge, neither of which is
> a folio boundary in general. Either one could land inside the large
> folio at its edge, and the truncate loop would drop that straddling
> folio together with its valid out-of-range tail.
> 
> For example, a 64K (order-4) folio with min_order = 2 punched from
> offset 0 to 36K:
> 
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()        # same_folio == true
>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>       end = p9                           # BUG: p9 inside [p8-p11]
>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
> 
> Rework the contract so the caller is told the page range to discard:
> 
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.
> 
>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
>     range fully covered by [lstart, lend] after any split (or none),
>     i.e. the pages wholly within the range and safe to discard. They are
>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>     order so they always fall on a folio boundary.
> 
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new outputs and to separate byte offsets from
>     folio indices.
> 
> 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@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>
> ---
> v1->v2:
>  - Export pstart as a new parameter so that the generic and shmem
>    truncate paths don't need to recompute the start value from the
>    return value. (Brian)
>  - When min_order is nonzero, align [pstart, pend] to the inner
>    boundaries of the folio to ensure they do not point into the middle
>    of a large folio, which could otherwise cause valid data within the
>    folio to be incorrectly cleared. (Joanne)
> 
> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/
> 

Hi Zhang,

Thanks for the tweaks. I still found some of the logic circuitous as I
read through it so I spent some time playing with this just to
experiment with cleaning it up a bit. I ended up removing a couple of
the labels, lifting the pstart/pend assignment to a default init/case,
and reshuffling the split case pstart/pend assignments in a way that I
think also elides the need for the boolean or using folio2. (I'm curious
if this happens to address the Sashiko feedback as well..?)

Note that this is completely untested and needs further review. Since
the current patch looked mostly Ok to me functionally (though I do agree
with the comment about possibly splitting up into smaller changes) and
has other reviews, I'm just posting this as an FYI. Here's a diff of the
changes I made on top of this patch (Assisted-by: LLM, fwiw). Feel free
to use some, all or none of it. Thanks!

Brian

--- 8< ---

diff --git a/mm/truncate.c b/mm/truncate.c
index d88a1b159084..8da16d7e6763 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -237,10 +237,16 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 	else
 		length = lend + 1 - pos - offset;
 
+	if (pstart)
+		*pstart = offset ? folio_next_index(folio) : folio->index;
+	if (pend)
+		*pend = (pos + size > (u64)lend) ? folio->index :
+						   folio_next_index(folio);
+
 	folio_wait_writeback(folio);
 	if (length == size) {
 		truncate_inode_folio(folio->mapping, folio);
-		goto no_split;
+		return false;
 	}
 
 	/*
@@ -254,7 +260,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 	if (folio_needs_release(folio))
 		folio_invalidate(folio, offset, length);
 	if (!folio_test_large(folio))
-		goto no_split;
+		return false;
 
 	min_order = mapping_min_folio_order(folio->mapping);
 	min_nrbytes = mapping_min_folio_nrbytes(folio->mapping);
@@ -266,29 +272,30 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 		 * for shmem truncate
 		 */
 		struct folio *folio2;
-		bool tail_isolated = true;
 
-		if (pend)
-			*pend = round_down(pos + offset + length,
+		if (pstart)
+			*pstart = round_up(pos + offset,
 					   min_nrbytes) >> PAGE_SHIFT;
 
-		if (offset + length == size)
-			goto split;
+		if (offset + length == size) {
+			if (pend)
+				*pend = round_down(pos + offset + length,
+						   min_nrbytes) >> PAGE_SHIFT;
+			return true;
+		}
 retry:
 		split_at2 = folio_page(folio,
 				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
 		folio2 = page_folio(split_at2);
 
 		if (!folio_try_get(folio2))
-			goto split;
+			return true;
 
 		if (!folio_test_large(folio2))
 			goto out;
 
-		if (!folio_trylock(folio2)) {
-			tail_isolated = false;
+		if (!folio_trylock(folio2))
 			goto out;
-		}
 
 		/*
 		 * split_at2 may no longer belong to folio2 due to concurrent
@@ -304,28 +311,18 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 		/* 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))
-			tail_isolated = false;
+		    !folio_split_or_unmap(folio2, split_at2, min_order) &&
+		    pend)
+			*pend = round_down(pos + offset + length,
+					   min_nrbytes) >> PAGE_SHIFT;
 
 		folio_unlock(folio2);
 out:
-		if (!tail_isolated && pend)
-			*pend = folio2->index;
 		folio_put(folio2);
-split:
-		if (pstart)
-			*pstart = round_up(pos + offset,
-					   min_nrbytes) >> PAGE_SHIFT;
 		return true;
 	}
 	if (!folio_test_dirty(folio))
 		truncate_inode_folio(folio->mapping, folio);
-no_split:
-	if (pstart)
-		*pstart = offset ? folio_next_index(folio) : folio->index;
-	if (pend)
-		*pend = (pos + size > (u64)lend) ? folio->index :
-						   folio_next_index(folio);
 	return false;
 }


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-10 16:26 ` Brian Foster
@ 2026-09-11  7:34   ` Zhang Yi
  2026-09-11 14:57     ` Brian Foster
  0 siblings, 1 reply; 11+ messages in thread
From: Zhang Yi @ 2026-09-11  7:34 UTC (permalink / raw)
  To: Brian Foster
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On 9/11/2026 12:26 AM, Brian Foster wrote:
> On Wed, Sep 09, 2026 at 02:23:39PM +0800, 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.
>>
>> In addition, even when both splits succeed, data can still be lost when
>> the mapping's minimum folio order (min_order) is non-zero. folio_split()
>> stops at min_order instead of order 0, so the sub-folio containing a
>> split point stays aligned to 1 << min_order rather than to a page. The
>> original success path left start at the page-aligned head of the range
>> and set end to the exact page index of the end edge, neither of which is
>> a folio boundary in general. Either one could land inside the large
>> folio at its edge, and the truncate loop would drop that straddling
>> folio together with its valid out-of-range tail.
>>
>> For example, a 64K (order-4) folio with min_order = 2 punched from
>> offset 0 to 36K:
>>
>>   truncate_inode_pages_range()
>>     truncate_inode_partial_folio()        # same_folio == true
>>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>>       end = p9                           # BUG: p9 inside [p8-p11]
>>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
>>
>> Rework the contract so the caller is told the page range to discard:
>>
>>   - Return true only when a split occurred, false otherwise. This
>>     clarifies the existing confusing return value semantics.
>>
>>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
>>     range fully covered by [lstart, lend] after any split (or none),
>>     i.e. the pages wholly within the range and safe to discard. They are
>>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>>     order so they always fall on a folio boundary.
>>
>>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>>     clashing with the new outputs and to separate byte offsets from
>>     folio indices.
>>
>> 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@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>
>> ---
>> v1->v2:
>>  - Export pstart as a new parameter so that the generic and shmem
>>    truncate paths don't need to recompute the start value from the
>>    return value. (Brian)
>>  - When min_order is nonzero, align [pstart, pend] to the inner
>>    boundaries of the folio to ensure they do not point into the middle
>>    of a large folio, which could otherwise cause valid data within the
>>    folio to be incorrectly cleared. (Joanne)
>>
>> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/
>>
> 
> Hi Zhang,
> 
> Thanks for the tweaks. I still found some of the logic circuitous as I
> read through it so I spent some time playing with this just to
> experiment with cleaning it up a bit. I ended up removing a couple of
> the labels, lifting the pstart/pend assignment to a default init/case,
> and reshuffling the split case pstart/pend assignments in a way that I
> think also elides the need for the boolean or using folio2. (I'm curious
> if this happens to address the Sashiko feedback as well..?)
> 
> Note that this is completely untested and needs further review. Since
> the current patch looked mostly Ok to me functionally (though I do agree
> with the comment about possibly splitting up into smaller changes) and
> has other reviews, I'm just posting this as an FYI. Here's a diff of the
> changes I made on top of this patch (Assisted-by: LLM, fwiw). Feel free
> to use some, all or none of it. Thanks!
> 
> Brian

Hi Brian,

Thanks a lot for spending time on this, and for posting the diff. I
agree with most of it.

Lifting the default pstart/pend assignment to the top and turning the
early exits into plain returns removed the need for the no_split: /
split: labels and the tail_isolated boolean, which does make the
function easier to follow.

One thing I was wondering about: in your diff, when setting pend while
trying to split folio2, you completely dropped the use of folio2->index.
With folio2->index gone, an unaligned end means any failed split falls
back to folio->index. I assume that is mainly because its value is not
reliable under concurrency, which is exactly what sashiko pointed out.
However, it does mean we lose a bit of precision though, I mean the
sub-folios that were successfully split off at the pstart edge end up
left behind in the page cache. Please see a small example below.

> 
> --- 8< ---
> 
> diff --git a/mm/truncate.c b/mm/truncate.c
> index d88a1b159084..8da16d7e6763 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -237,10 +237,16 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  	else
>  		length = lend + 1 - pos - offset;
>  
> +	if (pstart)
> +		*pstart = offset ? folio_next_index(folio) : folio->index;
> +	if (pend)
> +		*pend = (pos + size > (u64)lend) ? folio->index :
> +						   folio_next_index(folio);
> +
>  	folio_wait_writeback(folio);
>  	if (length == size) {
>  		truncate_inode_folio(folio->mapping, folio);
> -		goto no_split;
> +		return false;
>  	}
>  
>  	/*
> @@ -254,7 +260,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  	if (folio_needs_release(folio))
>  		folio_invalidate(folio, offset, length);
>  	if (!folio_test_large(folio))
> -		goto no_split;
> +		return false;
>  
>  	min_order = mapping_min_folio_order(folio->mapping);
>  	min_nrbytes = mapping_min_folio_nrbytes(folio->mapping);
> @@ -266,29 +272,30 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> -		bool tail_isolated = true;
>  
> -		if (pend)
> -			*pend = round_down(pos + offset + length,
> +		if (pstart)
> +			*pstart = round_up(pos + offset,
>  					   min_nrbytes) >> PAGE_SHIFT;
>  
> -		if (offset + length == size)
> -			goto split;
> +		if (offset + length == size) {
> +			if (pend)
> +				*pend = round_down(pos + offset + length,
> +						   min_nrbytes) >> PAGE_SHIFT;
> +			return true;
> +		}
>  retry:
>  		split_at2 = folio_page(folio,
>  				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
>  		folio2 = page_folio(split_at2);
>  
>  		if (!folio_try_get(folio2))
> -			goto split;
> +			return true;
>  
>  		if (!folio_test_large(folio2))
>  			goto out;
>  
> -		if (!folio_trylock(folio2)) {
> -			tail_isolated = false;
> +		if (!folio_trylock(folio2))
>  			goto out;
> -		}
>  
>  		/*
>  		 * split_at2 may no longer belong to folio2 due to concurrent
> @@ -304,28 +311,18 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  		/* 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))
> -			tail_isolated = false;
> +		    !folio_split_or_unmap(folio2, split_at2, min_order) &&
> +		    pend)
> +			*pend = round_down(pos + offset + length,
> +					   min_nrbytes) >> PAGE_SHIFT;

[...]

Assume a 4-page order-2 folio [p0 p1 p2 p3], punched from offset 0 into
the middle of p3, with 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  # folio_split_or_unmap() returns failure.
		
As a result, both pstart and pend are p0, so the sub-folios p0 and p1
are left behind.

I agree that under concurrency we really can't get a valid end position
since folio2 is not trustworthy(e,g., failed to get the folio2 or failed
to lock the folio2...). But once we have successfully got a reference to
and locked folio2, its index should be valid. So I've ended up with
something like the below (This hasn't been tested yet, just wanted to see
everyone's opinions), let me know what you and the other reviewers think.


diff --git a/mm/truncate.c b/mm/truncate.c
index d88a1b159084..b551e8253de7 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -237,10 +237,16 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 	else
 		length = lend + 1 - pos - offset;

+	if (pstart)
+		*pstart = offset ? folio_next_index(folio) : folio->index;
+	if (pend)
+		*pend = (pos + size > (u64)lend) ? folio->index :
+						   folio_next_index(folio);
+
 	folio_wait_writeback(folio);
 	if (length == size) {
 		truncate_inode_folio(folio->mapping, folio);
-		goto no_split;
+		return false;
 	}

 	/*
@@ -254,7 +260,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 	if (folio_needs_release(folio))
 		folio_invalidate(folio, offset, length);
 	if (!folio_test_large(folio))
-		goto no_split;
+		return false;

 	min_order = mapping_min_folio_order(folio->mapping);
 	min_nrbytes = mapping_min_folio_nrbytes(folio->mapping);
@@ -266,66 +272,65 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 		 * for shmem truncate
 		 */
 		struct folio *folio2;
-		bool tail_isolated = true;
+		pgoff_t end, aligned_end = round_down(pos + offset + length,
+						min_nrbytes) >> PAGE_SHIFT;

-		if (pend)
-			*pend = round_down(pos + offset + length,
+		if (pstart)
+			*pstart = round_up(pos + offset,
 					   min_nrbytes) >> PAGE_SHIFT;

-		if (offset + length == size)
-			goto split;
-retry:
+		if (offset + length == size) {
+			end = aligned_end;
+			goto out;
+		}
+
 		split_at2 = folio_page(folio,
 				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
 		folio2 = page_folio(split_at2);

-		if (!folio_try_get(folio2))
-			goto split;
-
-		if (!folio_test_large(folio2))
-			goto out;
-
-		if (!folio_trylock(folio2)) {
-			tail_isolated = false;
-			goto out;
-		}
-
 		/*
-		 * split_at2 may no longer belong to folio2 due to concurrent
-		 * split. Retry to find the correct folio in case it's still
-		 * large.
+		 * folio2 may become stale due to a concurrent split or
+		 * freeing, so validate it before and after taking its lock.
+		 * If it fails, we can't get an accurate end position and fall
+		 * back to folio->index, which may leave sub-folios split off
+		 * at the offset edge in the page cache this round.
 		 */
+		end = folio->index;
+		if (!folio_try_get(folio2))
+			goto out;
+		if (folio2->mapping != folio->mapping ||
+		    !folio_test_large(folio2))
+			goto out_put;
+
+		if (!folio_trylock(folio2))
+			goto out_put;
+
 		if (page_folio(split_at2) != folio2) {
 			folio_unlock(folio2);
-			folio_put(folio2);
-			goto retry;
+			goto out_put;
+		}
+		if (!folio_test_large(folio2)) {
+			end = aligned_end;
+			folio_unlock(folio2);
+			goto out_put;
 		}

-		/* 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))
-			tail_isolated = false;
+		/* Split failed: back off to the head of the straddler */
+		if (folio_split_or_unmap(folio2, split_at2, min_order))
+			end = folio2->index;
+		else
+			end = aligned_end;

 		folio_unlock(folio2);
-out:
-		if (!tail_isolated && pend)
-			*pend = folio2->index;
+out_put:
 		folio_put(folio2);
-split:
-		if (pstart)
-			*pstart = round_up(pos + offset,
-					   min_nrbytes) >> PAGE_SHIFT;
+out:
+		if (pend)
+			*pend = end;
 		return true;
 	}
 	if (!folio_test_dirty(folio))
 		truncate_inode_folio(folio->mapping, folio);
-no_split:
-	if (pstart)
-		*pstart = offset ? folio_next_index(folio) : folio->index;
-	if (pend)
-		*pend = (pos + size > (u64)lend) ? folio->index :
-						   folio_next_index(folio);
 	return false;
 }



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09 18:29 ` Joanne Koong
@ 2026-09-11  8:25   ` Zhang Yi
  0 siblings, 0 replies; 11+ messages in thread
From: Zhang Yi @ 2026-09-11  8:25 UTC (permalink / raw)
  To: Joanne Koong
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, bfoster, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On 9/10/2026 2:29 AM, Joanne Koong wrote:
> On Tue, Sep 8, 2026 at 11:31 PM Zhang Yi <yi.zhang@huaweicloud.com> 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.
>>
>> In addition, even when both splits succeed, data can still be lost when
>> the mapping's minimum folio order (min_order) is non-zero. folio_split()
>> stops at min_order instead of order 0, so the sub-folio containing a
>> split point stays aligned to 1 << min_order rather than to a page. The
>> original success path left start at the page-aligned head of the range
>> and set end to the exact page index of the end edge, neither of which is
>> a folio boundary in general. Either one could land inside the large
>> folio at its edge, and the truncate loop would drop that straddling
>> folio together with its valid out-of-range tail.
>>
>> For example, a 64K (order-4) folio with min_order = 2 punched from
>> offset 0 to 36K:
>>
>>   truncate_inode_pages_range()
>>     truncate_inode_partial_folio()        # same_folio == true
>>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>>       end = p9                           # BUG: p9 inside [p8-p11]
>>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
>>
>> Rework the contract so the caller is told the page range to discard:
>>
>>   - Return true only when a split occurred, false otherwise. This
>>     clarifies the existing confusing return value semantics.
>>
>>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
>>     range fully covered by [lstart, lend] after any split (or none),
>>     i.e. the pages wholly within the range and safe to discard. They are
>>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>>     order so they always fall on a folio boundary.
>>
>>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>>     clashing with the new outputs and to separate byte offsets from
>>     folio indices.
>>
>> 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@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>
> 
> Reviewed-by: Joanne Koong <joannelkoong@gmail.com>
> 
>> ---
>> v1->v2:
>>  - Export pstart as a new parameter so that the generic and shmem
>>    truncate paths don't need to recompute the start value from the
>>    return value. (Brian)
>>  - When min_order is nonzero, align [pstart, pend] to the inner
>>    boundaries of the folio to ensure they do not point into the middle
>>    of a large folio, which could otherwise cause valid data within the
>>    folio to be incorrectly cleared. (Joanne)
>>
>> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/
>>
>>  mm/internal.h |  4 +--
>>  mm/shmem.c    | 13 +++-----
>>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>>  3 files changed, 67 insertions(+), 38 deletions(-)
>>

[...]

>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index b58ba940be47..d88a1b159084 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
>> @@ -206,35 +206,41 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>>  /*
>>   * 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
>> - * folio that's within the [start, end] range, and then split the folio if
>> + * folio that's within the [lstart, lend] range, and then split the folio if
>>   * it's large.  split_page_range() will discard pages which now lie beyond
>>   * i_size, and we rely on the caller to discard pages which lie within a
>>   * newly created hole.
>>   *
>> - * Returns false if splitting failed so the caller can avoid
>> - * discarding the entire folio which is stubbornly unsplit.
>> + * When @pstart and/or @pend are non-NULL they receive the indexes of the
>> + * page range fully covered by [lstart, lend] after any split (or none),
>> + * i.e. the range of pages that are wholly within [lstart, lend] and so safe
>> + * to discard.
> 
> I think with themin_order > 0 case, pages wholly within [lstart, lend]
> can still be excluded, so maybe worth editing this to reflect that.
> 
> Thanks,
> Joanne

Good point, I will fix this in next iteration.

Thanks,
Yi.

> 
>> + *
>> + * Return %true if the folio was split, %false otherwise.
>>   */
>> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>> +                                 loff_t lend, pgoff_t *pstart, pgoff_t *pend)
>>  {
>>         loff_t pos = folio_pos(folio);
>>         size_t size = folio_size(folio);


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-09 19:18 ` Zi Yan
@ 2026-09-11  9:26   ` Zhang Yi
  0 siblings, 0 replies; 11+ messages in thread
From: Zhang Yi @ 2026-09-11  9:26 UTC (permalink / raw)
  To: Zi Yan
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, bfoster, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On 9/10/2026 3:18 AM, Zi Yan wrote:
> On 9 Sep 2026, at 2:23, 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 fixing it.
> 
>>
>> In addition, even when both splits succeed, data can still be lost when
>> the mapping's minimum folio order (min_order) is non-zero. folio_split()
>> stops at min_order instead of order 0, so the sub-folio containing a
>> split point stays aligned to 1 << min_order rather than to a page. The
>> original success path left start at the page-aligned head of the range
>> and set end to the exact page index of the end edge, neither of which is
>> a folio boundary in general. Either one could land inside the large
>> folio at its edge, and the truncate loop would drop that straddling
>> folio together with its valid out-of-range tail.
>>
>> For example, a 64K (order-4) folio with min_order = 2 punched from
>> offset 0 to 36K:
>>
>>   truncate_inode_pages_range()
>>     truncate_inode_partial_folio()        # same_folio == true
>>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
>>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
>>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
>>       end = p9                           # BUG: p9 inside [p8-p11]
>>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
> 
> Should min_order > 0 fix come as a separate patch first? min_order>0
> was introduced by commit e220917fa5077 ("mm: split a folio in minimum
> folio order chunks") and it is before folio_split(). The old
> split_huge_page_to_list_to_order() can cause a similar issue?
> 

Indeed, the min_order issue is a separate problem, and we should fix
it in a separate patch.

> 
> <snip>
> 
>>     i.e. the pages wholly within the range and safe to discard. They are
>>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>>     order so they always fall on a folio boundary.
> 
> Basically, to fix min_order issue alone, truncate_inode_partial_folio()
> needs to align pstart and pend to 1UL << min_order?
> 
> 

Yes, fixing the min_order issue separately amounts to aligning pstart
and pend to 1UL << min_order, so they need to be exported first.
Otherwise the same alignment would have to be done in the two callers,
truncate_inode_pages_range() / shmem_undo_range(). I can rework
truncate_inode_partial_folio() to export these two parameters first, and
then split out a separate patch for the min_order fix.

Thanks,
Yi.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
  2026-09-11  7:34   ` Zhang Yi
@ 2026-09-11 14:57     ` Brian Foster
  0 siblings, 0 replies; 11+ messages in thread
From: Brian Foster @ 2026-09-11 14:57 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, joannelkoong, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Fri, Sep 11, 2026 at 03:34:48PM +0800, Zhang Yi wrote:
> On 9/11/2026 12:26 AM, Brian Foster wrote:
> > On Wed, Sep 09, 2026 at 02:23:39PM +0800, 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.
> >>
> >> In addition, even when both splits succeed, data can still be lost when
> >> the mapping's minimum folio order (min_order) is non-zero. folio_split()
> >> stops at min_order instead of order 0, so the sub-folio containing a
> >> split point stays aligned to 1 << min_order rather than to a page. The
> >> original success path left start at the page-aligned head of the range
> >> and set end to the exact page index of the end edge, neither of which is
> >> a folio boundary in general. Either one could land inside the large
> >> folio at its edge, and the truncate loop would drop that straddling
> >> folio together with its valid out-of-range tail.
> >>
> >> For example, a 64K (order-4) folio with min_order = 2 punched from
> >> offset 0 to 36K:
> >>
> >>   truncate_inode_pages_range()
> >>     truncate_inode_partial_folio()        # same_folio == true
> >>       1st split at p0 -> [p0-p3, p4-p7, p8-p15]  # non-uniform, min_order
> >>       folio2 = p8-p15 # straddles: p8 in range, p9-p15 tail valid
> >>       2nd split of folio2 -> [p8-p11, p12-p15]   # success
> >>       end = p9                           # BUG: p9 inside [p8-p11]
> >>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
> >>
> >> Rework the contract so the caller is told the page range to discard:
> >>
> >>   - Return true only when a split occurred, false otherwise. This
> >>     clarifies the existing confusing return value semantics.
> >>
> >>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
> >>     range fully covered by [lstart, lend] after any split (or none),
> >>     i.e. the pages wholly within the range and safe to discard. They are
> >>     aligned up (pstart) and down (pend) to the mapping's minimum folio
> >>     order so they always fall on a folio boundary.
> >>
> >>   - Rename the byte-range parameters start/end to lstart/lend to avoid
> >>     clashing with the new outputs and to separate byte offsets from
> >>     folio indices.
> >>
> >> 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@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>
> >> ---
> >> v1->v2:
> >>  - Export pstart as a new parameter so that the generic and shmem
> >>    truncate paths don't need to recompute the start value from the
> >>    return value. (Brian)
> >>  - When min_order is nonzero, align [pstart, pend] to the inner
> >>    boundaries of the folio to ensure they do not point into the middle
> >>    of a large folio, which could otherwise cause valid data within the
> >>    folio to be incorrectly cleared. (Joanne)
> >>
> >> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@huaweicloud.com/
> >>
> > 
> > Hi Zhang,
> > 
> > Thanks for the tweaks. I still found some of the logic circuitous as I
> > read through it so I spent some time playing with this just to
> > experiment with cleaning it up a bit. I ended up removing a couple of
> > the labels, lifting the pstart/pend assignment to a default init/case,
> > and reshuffling the split case pstart/pend assignments in a way that I
> > think also elides the need for the boolean or using folio2. (I'm curious
> > if this happens to address the Sashiko feedback as well..?)
> > 
> > Note that this is completely untested and needs further review. Since
> > the current patch looked mostly Ok to me functionally (though I do agree
> > with the comment about possibly splitting up into smaller changes) and
> > has other reviews, I'm just posting this as an FYI. Here's a diff of the
> > changes I made on top of this patch (Assisted-by: LLM, fwiw). Feel free
> > to use some, all or none of it. Thanks!
> > 
> > Brian
> 
> Hi Brian,
> 
> Thanks a lot for spending time on this, and for posting the diff. I
> agree with most of it.
> 
> Lifting the default pstart/pend assignment to the top and turning the
> early exits into plain returns removed the need for the no_split: /
> split: labels and the tail_isolated boolean, which does make the
> function easier to follow.
> 
> One thing I was wondering about: in your diff, when setting pend while
> trying to split folio2, you completely dropped the use of folio2->index.
> With folio2->index gone, an unaligned end means any failed split falls
> back to folio->index. I assume that is mainly because its value is not
> reliable under concurrency, which is exactly what sashiko pointed out.
> However, it does mean we lose a bit of precision though, I mean the
> sub-folios that were successfully split off at the pstart edge end up
> left behind in the page cache. Please see a small example below.
> 

Yeah, it's possible this went a little too far on the simplification
side. I was thinking of just being conservative and saying it's probably
safer to err on the side of leaving folios around in the failure case
vs. being too aggressive and losing data like the issue this is trying
to fix.

I won't be able to take a closer look at this until next week, but on a
quick glance what you describe below sounds reasonable to me. Thanks!

Brian

> > 
> > --- 8< ---
> > 
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index d88a1b159084..8da16d7e6763 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -237,10 +237,16 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> >  	else
> >  		length = lend + 1 - pos - offset;
> >  
> > +	if (pstart)
> > +		*pstart = offset ? folio_next_index(folio) : folio->index;
> > +	if (pend)
> > +		*pend = (pos + size > (u64)lend) ? folio->index :
> > +						   folio_next_index(folio);
> > +
> >  	folio_wait_writeback(folio);
> >  	if (length == size) {
> >  		truncate_inode_folio(folio->mapping, folio);
> > -		goto no_split;
> > +		return false;
> >  	}
> >  
> >  	/*
> > @@ -254,7 +260,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> >  	if (folio_needs_release(folio))
> >  		folio_invalidate(folio, offset, length);
> >  	if (!folio_test_large(folio))
> > -		goto no_split;
> > +		return false;
> >  
> >  	min_order = mapping_min_folio_order(folio->mapping);
> >  	min_nrbytes = mapping_min_folio_nrbytes(folio->mapping);
> > @@ -266,29 +272,30 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> >  		 * for shmem truncate
> >  		 */
> >  		struct folio *folio2;
> > -		bool tail_isolated = true;
> >  
> > -		if (pend)
> > -			*pend = round_down(pos + offset + length,
> > +		if (pstart)
> > +			*pstart = round_up(pos + offset,
> >  					   min_nrbytes) >> PAGE_SHIFT;
> >  
> > -		if (offset + length == size)
> > -			goto split;
> > +		if (offset + length == size) {
> > +			if (pend)
> > +				*pend = round_down(pos + offset + length,
> > +						   min_nrbytes) >> PAGE_SHIFT;
> > +			return true;
> > +		}
> >  retry:
> >  		split_at2 = folio_page(folio,
> >  				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
> >  		folio2 = page_folio(split_at2);
> >  
> >  		if (!folio_try_get(folio2))
> > -			goto split;
> > +			return true;
> >  
> >  		if (!folio_test_large(folio2))
> >  			goto out;
> >  
> > -		if (!folio_trylock(folio2)) {
> > -			tail_isolated = false;
> > +		if (!folio_trylock(folio2))
> >  			goto out;
> > -		}
> >  
> >  		/*
> >  		 * split_at2 may no longer belong to folio2 due to concurrent
> > @@ -304,28 +311,18 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> >  		/* 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))
> > -			tail_isolated = false;
> > +		    !folio_split_or_unmap(folio2, split_at2, min_order) &&
> > +		    pend)
> > +			*pend = round_down(pos + offset + length,
> > +					   min_nrbytes) >> PAGE_SHIFT;
> 
> [...]
> 
> Assume a 4-page order-2 folio [p0 p1 p2 p3], punched from offset 0 into
> the middle of p3, with 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  # folio_split_or_unmap() returns failure.
> 		
> As a result, both pstart and pend are p0, so the sub-folios p0 and p1
> are left behind.
> 
> I agree that under concurrency we really can't get a valid end position
> since folio2 is not trustworthy(e,g., failed to get the folio2 or failed
> to lock the folio2...). But once we have successfully got a reference to
> and locked folio2, its index should be valid. So I've ended up with
> something like the below (This hasn't been tested yet, just wanted to see
> everyone's opinions), let me know what you and the other reviewers think.
> 
> 
> diff --git a/mm/truncate.c b/mm/truncate.c
> index d88a1b159084..b551e8253de7 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -237,10 +237,16 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  	else
>  		length = lend + 1 - pos - offset;
> 
> +	if (pstart)
> +		*pstart = offset ? folio_next_index(folio) : folio->index;
> +	if (pend)
> +		*pend = (pos + size > (u64)lend) ? folio->index :
> +						   folio_next_index(folio);
> +
>  	folio_wait_writeback(folio);
>  	if (length == size) {
>  		truncate_inode_folio(folio->mapping, folio);
> -		goto no_split;
> +		return false;
>  	}
> 
>  	/*
> @@ -254,7 +260,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  	if (folio_needs_release(folio))
>  		folio_invalidate(folio, offset, length);
>  	if (!folio_test_large(folio))
> -		goto no_split;
> +		return false;
> 
>  	min_order = mapping_min_folio_order(folio->mapping);
>  	min_nrbytes = mapping_min_folio_nrbytes(folio->mapping);
> @@ -266,66 +272,65 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> -		bool tail_isolated = true;
> +		pgoff_t end, aligned_end = round_down(pos + offset + length,
> +						min_nrbytes) >> PAGE_SHIFT;
> 
> -		if (pend)
> -			*pend = round_down(pos + offset + length,
> +		if (pstart)
> +			*pstart = round_up(pos + offset,
>  					   min_nrbytes) >> PAGE_SHIFT;
> 
> -		if (offset + length == size)
> -			goto split;
> -retry:
> +		if (offset + length == size) {
> +			end = aligned_end;
> +			goto out;
> +		}
> +
>  		split_at2 = folio_page(folio,
>  				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
>  		folio2 = page_folio(split_at2);
> 
> -		if (!folio_try_get(folio2))
> -			goto split;
> -
> -		if (!folio_test_large(folio2))
> -			goto out;
> -
> -		if (!folio_trylock(folio2)) {
> -			tail_isolated = false;
> -			goto out;
> -		}
> -
>  		/*
> -		 * split_at2 may no longer belong to folio2 due to concurrent
> -		 * split. Retry to find the correct folio in case it's still
> -		 * large.
> +		 * folio2 may become stale due to a concurrent split or
> +		 * freeing, so validate it before and after taking its lock.
> +		 * If it fails, we can't get an accurate end position and fall
> +		 * back to folio->index, which may leave sub-folios split off
> +		 * at the offset edge in the page cache this round.
>  		 */
> +		end = folio->index;
> +		if (!folio_try_get(folio2))
> +			goto out;
> +		if (folio2->mapping != folio->mapping ||
> +		    !folio_test_large(folio2))
> +			goto out_put;
> +
> +		if (!folio_trylock(folio2))
> +			goto out_put;
> +
>  		if (page_folio(split_at2) != folio2) {
>  			folio_unlock(folio2);
> -			folio_put(folio2);
> -			goto retry;
> +			goto out_put;
> +		}
> +		if (!folio_test_large(folio2)) {
> +			end = aligned_end;
> +			folio_unlock(folio2);
> +			goto out_put;
>  		}
> 
> -		/* 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))
> -			tail_isolated = false;
> +		/* Split failed: back off to the head of the straddler */
> +		if (folio_split_or_unmap(folio2, split_at2, min_order))
> +			end = folio2->index;
> +		else
> +			end = aligned_end;
> 
>  		folio_unlock(folio2);
> -out:
> -		if (!tail_isolated && pend)
> -			*pend = folio2->index;
> +out_put:
>  		folio_put(folio2);
> -split:
> -		if (pstart)
> -			*pstart = round_up(pos + offset,
> -					   min_nrbytes) >> PAGE_SHIFT;
> +out:
> +		if (pend)
> +			*pend = end;
>  		return true;
>  	}
>  	if (!folio_test_dirty(folio))
>  		truncate_inode_folio(folio->mapping, folio);
> -no_split:
> -	if (pstart)
> -		*pstart = offset ? folio_next_index(folio) : folio->index;
> -	if (pend)
> -		*pend = (pos + size > (u64)lend) ? folio->index :
> -						   folio_next_index(folio);
>  	return false;
>  }
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-11 14:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-09  6:23 [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios Zhang Yi
2026-09-09 12:22 ` Jan Kara
2026-09-09 18:29 ` Joanne Koong
2026-09-11  8:25   ` Zhang Yi
2026-09-09 19:18 ` Zi Yan
2026-09-11  9:26   ` Zhang Yi
2026-09-09 23:14 ` Andrew Morton
2026-09-10  7:18   ` Zhang Yi
2026-09-10 16:26 ` Brian Foster
2026-09-11  7:34   ` Zhang Yi
2026-09-11 14:57     ` Brian Foster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®