[v3,1/3] mm/truncate: fix data loss when splitting straddling large folios fails

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.

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

  - 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.

  - Adjust the ordering of the validate check when splitting folio2.
    folio2->index is only reliable after the reference count and lock
    have been successfully acquired, since it may have been split
    concurrently, or freed and recycled to an unrelated mapping. On any
    failure to obtain a reliable end position, fall back to
    folio->index, which is safe but leaves the sub-folios split off at
    the offset edge in the page cache.

  - Rename the byte-range parameters start/end to lstart/lend to better
    express their semantics.

Callers in truncate_inode_pages_range() and shmem_undo_range() pass
&pstart for the folio at the start edge and &pend for the folio at the
end edge, so the truncate loop drops exactly the fully covered pages and
never touches a straddling folio that still holds valid out-of-range
data.

Suggested-by: Brian Foster <bfoster@redhat.com>
Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 mm/internal.h |  4 +--
 mm/shmem.c    | 13 +++-----
 mm/truncate.c | 89 ++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 71 insertions(+), 35 deletions(-)

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] mm: data loss in hole punch due to ABA in speculative folio lookup
- [High] mm: memory leak in hole punch due to skipped successfully split order-0 folios
--

commit 2e1b0dbca7f3fd72aa6840729db47e2498ff1268
Author: Zhang Yi <yi.zhang@huawei.com>

mm/truncate: fix data loss when splitting straddling large folios fails

This commit modifies truncate_inode_partial_folio to return out-parameters
pstart and pend indicating the fully covered page range safe to discard
after any split. This is intended to fix a data loss issue when splitting
a straddling large folio fails.

Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/

> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be474..bec6d881d0225 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> 
> [ ... ]
> 
> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		pgoff_t end, aligned_end = (pos + offset + length) >>
> +					   PAGE_SHIFT;
>  
> -		if (offset + length == size)
> -			goto no_split;
> +		if (pstart)
> +			*pstart = round_up(pos + offset, PAGE_SIZE) >>
> +				  PAGE_SHIFT;
> +
> +		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);
>  
> +		/*
> +		 * 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 no_split;
> -
> -		if (!folio_test_large(folio2))
>  			goto out;
>  
> +		if (folio2->mapping != folio->mapping ||
> +		    !folio_test_large(folio2))
> +			goto out_put;

[Severity: High]
Could this lead to a memory leak if the first split successfully shatters the
folio to order-0 pages?

If the hole punch starts in the middle of a large folio and the first split
successfully shatters it into order-0 folios (which happens when min_order
is 0), the tail page folio2 becomes an order-0 folio.

The speculative pre-lock check validates folio2 here:

    if (folio2->mapping != folio->mapping ||
        !folio_test_large(folio2))
        goto out_put;

Because folio2 is order-0, !folio_test_large(folio2) evaluates to true.
Execution jumps to out_put without updating end to aligned_end, leaving end
initialized to folio->index.

When pend is returned as folio->index to the caller
(truncate_inode_pages_range), it is used as the upper bound for the
truncation loop. Because end is less than or equal to start, the loop
terminates immediately, skipping the pages within the hole punch range
and leaving them permanently leaked in the page cache.

> +
> +		if (!folio_trylock(folio2))
> -			goto out;
> +			goto out_put;
> +
> +		if (page_folio(split_at2) != folio2) {
> +			folio_unlock(folio2);
> +			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);
> +		/* 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;

[Severity: Critical]
Does this speculative lookup handle the reallocation race correctly if the
folio is reallocated to a different file offset?

After the first split, the tail pages are added to the page cache and
unlocked. During speculative lookup, the tail page could be concurrently
reclaimed, freed, and reallocated as a new large folio in the same mapping
but at a completely different file offset.

The validation checks earlier and the page_folio check here will all pass
because the physical page split_at2 is part of the new allocation.

However, the file index of the folio is not verified. If this unrelated large
folio fails to split, the fallback path assigns its index to end here:

    if (folio_split_or_unmap(folio2, split_at2, min_order))
        end = folio2->index;

This unrelated file offset is then returned to the caller via pend, which
can cause the hole punch loop to truncate an unrelated segment of the file.

Should we verify that the file index of folio2 matches the expected index
(e.g. folio2->index + folio_page_idx(folio2, split_at2) == expected_index)
before using it?

>  
>  		folio_unlock(folio2);
> -out:
> +out_put:
>  		folio_put(folio2);
> -no_split:
> +out:
> +		if (pend)
> +			*pend = end;
>  		return true;
>  	}
>  	if (folio_test_dirty(folio))

On Wed 16-09-26 17:24:48, 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.
> 
> Rework the contract so the caller is told the page range to discard:
> 
>   - 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.
> 
>   - Adjust the ordering of the validate check when splitting folio2.
>     folio2->index is only reliable after the reference count and lock
>     have been successfully acquired, since it may have been split
>     concurrently, or freed and recycled to an unrelated mapping. On any
>     failure to obtain a reliable end position, fall back to
>     folio->index, which is safe but leaves the sub-folios split off at
>     the offset edge in the page cache.
> 
>   - Rename the byte-range parameters start/end to lstart/lend to better
>     express their semantics.
> 
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
> &pstart for the folio at the start edge and &pend for the folio at the
> end edge, so the truncate loop drops exactly the fully covered pages and
> never touches a straddling folio that still holds valid out-of-range
> data.
> 
> Suggested-by: Brian Foster <bfoster@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>

The changes mostly look good to me but I have some confusion around the
folio2 splitting below.

> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		pgoff_t end, aligned_end = (pos + offset + length) >>
> +					   PAGE_SHIFT;
>  
> -		if (offset + length == size)
> -			goto no_split;
> +		if (pstart)
> +			*pstart = round_up(pos + offset, PAGE_SIZE) >>
> +				  PAGE_SHIFT;
> +
> +		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);
>  
> +		/*
> +		 * 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 no_split;
> -
> -		if (!folio_test_large(folio2))
>  			goto out;
>  
> +		if (folio2->mapping != folio->mapping ||
> +		    !folio_test_large(folio2))
> +			goto out_put;
> +
>  		if (!folio_trylock(folio2))
> -			goto out;
> +			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);
> +		if (page_folio(split_at2) != folio2) {
> +			folio_unlock(folio2);
> +			goto out_put;
> +		}
> +		if (!folio_test_large(folio2)) {
> +			end = aligned_end;
> +			folio_unlock(folio2);
> +			goto out_put;
> +		}

So I always found this folio2 lookup and revalidation somewhat suspicious
and now that we're digging into it I'll ask: As you note in the changelog,
by the time we compute split_at2 the page can be already freed and reused
because it was split off from the original 'folio'. It can be for example a
slab page or anything else. So is it guaranteed that page_folio() actually
returns something sensible? What guarantees we properly detect the "reuse
for something else" case in all possible cases for which the page can be
reused? I understand this is mostly a preexisting issue so maybe these
questions are more for MM guys than you...

So for me as an filesystem guy I'd appreciate some comment in this code
explaining why grabbing folio2 this way is actually safe. The really safe
way of getting to folio2 would be to use
  __filemap_get_folio(mapping, (offset+length) >> PAGE_SHIFT, FGP_LOCK, 0)
but I suppose we don't use the mapping lookup as it is more expensive?

								Honza

> +
> +		/* 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:
> +out_put:
>  		folio_put(folio2);
> -no_split:
> +out:
> +		if (pend)
> +			*pend = end;
>  		return true;
>  	}
>  	if (folio_test_dirty(folio))
> @@ -413,11 +455,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 +466,8 @@ 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
>

On Wed, Sep 16, 2026 at 05:24:48PM +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.
> 
> Rework the contract so the caller is told the page range to discard:
> 
>   - 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.
> 
>   - Adjust the ordering of the validate check when splitting folio2.
>     folio2->index is only reliable after the reference count and lock
>     have been successfully acquired, since it may have been split
>     concurrently, or freed and recycled to an unrelated mapping. On any
>     failure to obtain a reliable end position, fall back to
>     folio->index, which is safe but leaves the sub-folios split off at
>     the offset edge in the page cache.
> 
>   - Rename the byte-range parameters start/end to lstart/lend to better
>     express their semantics.
> 
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
> &pstart for the folio at the start edge and &pend for the folio at the
> end edge, so the truncate loop drops exactly the fully covered pages and
> never touches a straddling folio that still holds valid out-of-range
> data.
> 
> Suggested-by: Brian Foster <bfoster@redhat.com>
> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  mm/internal.h |  4 +--
>  mm/shmem.c    | 13 +++-----
>  mm/truncate.c | 89 ++++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 71 insertions(+), 35 deletions(-)
> 
...
> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be47..bec6d881d022 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
...
> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		pgoff_t end, aligned_end = (pos + offset + length) >>
> +					   PAGE_SHIFT;
>  
> -		if (offset + length == size)
> -			goto no_split;
> +		if (pstart)
> +			*pstart = round_up(pos + offset, PAGE_SIZE) >>
> +				  PAGE_SHIFT;
> +
> +		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);
>  
> +		/*
> +		 * 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 no_split;
> -
> -		if (!folio_test_large(folio2))
>  			goto out;
>  
> +		if (folio2->mapping != folio->mapping ||
> +		    !folio_test_large(folio2))
> +			goto out_put;
> +

Hi Zhang,

The only thing that sticks out to me in this version is the
!folio_test_large() check above. If we split (or something happens) such
that folio2 is no longer large, wouldn't it be more likely to catch it
here and fail to update end like we do in the same check a bit further
down after the folio lock? Maybe I'm missing something here, but it
seems a little odd for the same condition to return two possible states
like this. Otherwise I like the changes you've made and the rest of the
patch LGTM.

Brian

>  		if (!folio_trylock(folio2))
> -			goto out;
> +			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);
> +		if (page_folio(split_at2) != folio2) {
> +			folio_unlock(folio2);
> +			goto out_put;
> +		}
> +		if (!folio_test_large(folio2)) {
> +			end = aligned_end;
> +			folio_unlock(folio2);
> +			goto out_put;
> +		}
> +
> +		/* 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:
> +out_put:
>  		folio_put(folio2);
> -no_split:
> +out:
> +		if (pend)
> +			*pend = end;
>  		return true;
>  	}
>  	if (folio_test_dirty(folio))
> @@ -413,11 +455,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 +466,8 @@ 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
>

On Wed Sep 16, 2026 at 8:02 AM EDT, Jan Kara wrote:
> On Wed 16-09-26 17:24:48, 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.
>> 
>> Rework the contract so the caller is told the page range to discard:
>> 
>>   - 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.
>> 
>>   - Adjust the ordering of the validate check when splitting folio2.
>>     folio2->index is only reliable after the reference count and lock
>>     have been successfully acquired, since it may have been split
>>     concurrently, or freed and recycled to an unrelated mapping. On any
>>     failure to obtain a reliable end position, fall back to
>>     folio->index, which is safe but leaves the sub-folios split off at
>>     the offset edge in the page cache.
>> 
>>   - Rename the byte-range parameters start/end to lstart/lend to better
>>     express their semantics.
>> 
>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
>> &pstart for the folio at the start edge and &pend for the folio at the
>> end edge, so the truncate loop drops exactly the fully covered pages and
>> never touches a straddling folio that still holds valid out-of-range
>> data.
>> 
>> Suggested-by: Brian Foster <bfoster@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>
>
> The changes mostly look good to me but I have some confusion around the
> folio2 splitting below.
>
>> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>  		 * for shmem truncate
>>  		 */
>>  		struct folio *folio2;
>> +		pgoff_t end, aligned_end = (pos + offset + length) >>
>> +					   PAGE_SHIFT;
>>  
>> -		if (offset + length == size)
>> -			goto no_split;
>> +		if (pstart)
>> +			*pstart = round_up(pos + offset, PAGE_SIZE) >>
>> +				  PAGE_SHIFT;
>> +
>> +		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);
>>  
>> +		/*
>> +		 * 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 no_split;
>> -
>> -		if (!folio_test_large(folio2))
>>  			goto out;
>>  
>> +		if (folio2->mapping != folio->mapping ||
>> +		    !folio_test_large(folio2))
>> +			goto out_put;
>> +
>>  		if (!folio_trylock(folio2))
>> -			goto out;
>> +			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);
>> +		if (page_folio(split_at2) != folio2) {
>> +			folio_unlock(folio2);
>> +			goto out_put;
>> +		}
>> +		if (!folio_test_large(folio2)) {
>> +			end = aligned_end;
>> +			folio_unlock(folio2);
>> +			goto out_put;
>> +		}
>
> So I always found this folio2 lookup and revalidation somewhat suspicious
> and now that we're digging into it I'll ask: As you note in the changelog,
> by the time we compute split_at2 the page can be already freed and reused
> because it was split off from the original 'folio'. It can be for example a
> slab page or anything else. So is it guaranteed that page_folio() actually

For a slab or unrelated page, folio->mapping check rejects them.

> returns something sensible? What guarantees we properly detect the "reuse

No, but folio_try_get() prevents non ref'd folios.

> for something else" case in all possible cases for which the page can be
> reused? I understand this is mostly a preexisting issue so maybe these
> questions are more for MM guys than you...

I agree with you that trying to split a unlocked and not ref'd folio2 is
flaky. And it almost does a __filemap_get_folio() like you proposed
below.

>
> So for me as an filesystem guy I'd appreciate some comment in this code
> explaining why grabbing folio2 this way is actually safe. The really safe
> way of getting to folio2 would be to use
>   __filemap_get_folio(mapping, (offset+length) >> PAGE_SHIFT, FGP_LOCK, 0)
> but I suppose we don't use the mapping lookup as it is more expensive?

__filemap_get_folio() is a better version of the existing folio2 finding
code. When I wrote the code, I never thought about using
__filemap_get_folio(), but I probably should have done it.

BTW,
__filemap_get_folio(mapping, folio->index + (offset+length) >> PAGE_SHIFT,
		FGP_LOCK | FGP_NOWAIT, 0)
might be better:
1. folio->index is needed to get the right folio2,
2. nowait can us nonblocking.


>> +
>> +		/* 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:
>> +out_put:
>>  		folio_put(folio2);
>> -no_split:
>> +out:
>> +		if (pend)
>> +			*pend = end;
>>  		return true;
>>  	}
>>  	if (folio_test_dirty(folio))
>> @@ -413,11 +455,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 +466,8 @@ 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
>>

On 9/17/2026 2:03 AM, Zi Yan wrote:
> On Wed Sep 16, 2026 at 8:02 AM EDT, Jan Kara wrote:
>> On Wed 16-09-26 17:24:48, 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.
>>>
>>> Rework the contract so the caller is told the page range to discard:
>>>
>>>    - 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.
>>>
>>>    - Adjust the ordering of the validate check when splitting folio2.
>>>      folio2->index is only reliable after the reference count and lock
>>>      have been successfully acquired, since it may have been split
>>>      concurrently, or freed and recycled to an unrelated mapping. On any
>>>      failure to obtain a reliable end position, fall back to
>>>      folio->index, which is safe but leaves the sub-folios split off at
>>>      the offset edge in the page cache.
>>>
>>>    - Rename the byte-range parameters start/end to lstart/lend to better
>>>      express their semantics.
>>>
>>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
>>> &pstart for the folio at the start edge and &pend for the folio at the
>>> end edge, so the truncate loop drops exactly the fully covered pages and
>>> never touches a straddling folio that still holds valid out-of-range
>>> data.
>>>
>>> Suggested-by: Brian Foster <bfoster@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>
>>
>> The changes mostly look good to me but I have some confusion around the
>> folio2 splitting below.
>>
>>> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>>   		 * for shmem truncate
>>>   		 */
>>>   		struct folio *folio2;
>>> +		pgoff_t end, aligned_end = (pos + offset + length) >>
>>> +					   PAGE_SHIFT;
>>>   
>>> -		if (offset + length == size)
>>> -			goto no_split;
>>> +		if (pstart)
>>> +			*pstart = round_up(pos + offset, PAGE_SIZE) >>
>>> +				  PAGE_SHIFT;
>>> +
>>> +		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);
>>>   
>>> +		/*
>>> +		 * 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 no_split;
>>> -
>>> -		if (!folio_test_large(folio2))
>>>   			goto out;
>>>   
>>> +		if (folio2->mapping != folio->mapping ||
>>> +		    !folio_test_large(folio2))
>>> +			goto out_put;
>>> +
>>>   		if (!folio_trylock(folio2))
>>> -			goto out;
>>> +			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);
>>> +		if (page_folio(split_at2) != folio2) {
>>> +			folio_unlock(folio2);
>>> +			goto out_put;
>>> +		}
>>> +		if (!folio_test_large(folio2)) {
>>> +			end = aligned_end;
>>> +			folio_unlock(folio2);
>>> +			goto out_put;
>>> +		}
>>
>> So I always found this folio2 lookup and revalidation somewhat suspicious
>> and now that we're digging into it I'll ask: As you note in the changelog,
>> by the time we compute split_at2 the page can be already freed and reused
>> because it was split off from the original 'folio'. It can be for example a
>> slab page or anything else. So is it guaranteed that page_folio() actually
> 
> For a slab or unrelated page, folio->mapping check rejects them.
> 
>> returns something sensible? What guarantees we properly detect the "reuse
> 
> No, but folio_try_get() prevents non ref'd folios.
> 
>> for something else" case in all possible cases for which the page can be
>> reused? I understand this is mostly a preexisting issue so maybe these
>> questions are more for MM guys than you...
> 
> I agree with you that trying to split a unlocked and not ref'd folio2 is
> flaky. And it almost does a __filemap_get_folio() like you proposed
> below.
> 

I think this is indeed a pre-existing issue, as Sashiko pointed out:

 > [Severity: Critical]
 > Does this speculative lookup handle the reallocation race correctly 
if the
 > folio is reallocated to a different file offset?
 >
 > After the first split, the tail pages are added to the page cache and
 > unlocked. During speculative lookup, the tail page could be concurrently
 > reclaimed, freed, and reallocated as a new large folio in the same 
mapping
 > but at a completely different file offset.
 >
 > The validation checks earlier and the page_folio check here will all pass
 > because the physical page split_at2 is part of the new allocation.
 >
 > However, the file index of the folio is not verified. If this 
unrelated large
 > folio fails to split, the fallback path assigns its index to end here:
 >
 >     if (folio_split_or_unmap(folio2, split_at2, min_order))
 >         end = folio2->index;
 >
 > This unrelated file offset is then returned to the caller via pend, which
 > can cause the hole punch loop to truncate an unrelated segment of the 
file.
 >
 > Should we verify that the file index of folio2 matches the expected index
 > (e.g. folio2->index + folio_page_idx(folio2, split_at2) == 
expected_index)
 > before using it?

the current mapping check on the folio2 and the folio !=
page_folio(split_at) check in __folio_split() are not able to catch this
problem, which could then cause folio2 to be split at the wrong
position. Does that sound right?

>>
>> So for me as an filesystem guy I'd appreciate some comment in this code
>> explaining why grabbing folio2 this way is actually safe. The really safe
>> way of getting to folio2 would be to use
>>    __filemap_get_folio(mapping, (offset+length) >> PAGE_SHIFT, FGP_LOCK, 0)
>> but I suppose we don't use the mapping lookup as it is more expensive?
> 
> __filemap_get_folio() is a better version of the existing folio2 finding
> code. When I wrote the code, I never thought about using
> __filemap_get_folio(), but I probably should have done it.
> 
> BTW,
> __filemap_get_folio(mapping, folio->index + (offset+length) >> PAGE_SHIFT,
> 		FGP_LOCK | FGP_NOWAIT, 0)
> might be better:
> 1. folio->index is needed to get the right folio2,
> 2. nowait can us nonblocking.
> 
> 

Following your suggestion, I reworked my existing patch to use
__filemap_get_folio(), and it ends up looking roughly like the
following(not tested yet). What do you think?

Besides, if the issue Sashiko pointed out does indeed exist, I can
extract the __filemap_get_folio() change into a separate patch to fix it
on its own.

Thanks,
Yi.

diff --git a/mm/truncate.c b/mm/truncate.c
index 23c90f00b530..b5e5bdf5f9ef 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -279,51 +279,46 @@ bool truncate_inode_partial_folio(struct folio 
*folio, loff_t lstart,
  			*pstart = round_up(pos + offset,
  					   min_nrbytes) >> PAGE_SHIFT;

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

  		/*
-		 * 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.
+		 * After the first split at the start edge, the folio at the
+		 * end edge may be freed and reused concurrently.
+		 * __filemap_get_folio() looks up the straddler at
+		 * aligned_end and returns it locked and ref'd with the
+		 * mapping validated.
  		 */
-		end = folio->index;
-		if (!folio_try_get(folio2))
+		folio2 = __filemap_get_folio(folio->mapping, aligned_end,
+					     FGP_LOCK | FGP_NOWAIT, 0);
+		if (IS_ERR(folio2)) {
+			/*
+			 * No sub-folio straddles the boundary when aligned_end
+			 * is empty so discarding up to it is safe.  Otherwise
+			 * the straddler is locked by someone else and we
+			 * cannot obtain a reliable end position, so we fall
+			 * back to folio->index.  This is safe but may leave
+			 * pages split off at the offset edge lingering in the
+			 * page cache this round.
+			 */
+			if (PTR_ERR(folio2) != -ENOENT)
+				end = folio->index;
  			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);
-			goto out_put;
-		}
-		if (!folio_test_large(folio2)) {
-			end = aligned_end;
-			folio_unlock(folio2);
-			goto out_put;
  		}

-		/* Split failed: back off to the head of the straddler */
+		/* Already at the minimum order, nothing to split */
+		if (folio_order(folio2) == min_order)
+			goto out_put;
+
+		split_at2 = folio_page(folio2, (aligned_end - folio2->index));
+
+		/* Split failed, keep the straddler intact */
  		if (folio_split_or_unmap(folio2, split_at2, min_order))
  			end = folio2->index;
-		else
-			end = aligned_end;

-		folio_unlock(folio2);
  out_put:
+		folio_unlock(folio2);
  		folio_put(folio2);
  out:
  		if (pend)

On 9/17/2026 1:38 AM, Brian Foster wrote:
> On Wed, Sep 16, 2026 at 05:24:48PM +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.
>>
>> Rework the contract so the caller is told the page range to discard:
>>
>>    - 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.
>>
>>    - Adjust the ordering of the validate check when splitting folio2.
>>      folio2->index is only reliable after the reference count and lock
>>      have been successfully acquired, since it may have been split
>>      concurrently, or freed and recycled to an unrelated mapping. On any
>>      failure to obtain a reliable end position, fall back to
>>      folio->index, which is safe but leaves the sub-folios split off at
>>      the offset edge in the page cache.
>>
>>    - Rename the byte-range parameters start/end to lstart/lend to better
>>      express their semantics.
>>
>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
>> &pstart for the folio at the start edge and &pend for the folio at the
>> end edge, so the truncate loop drops exactly the fully covered pages and
>> never touches a straddling folio that still holds valid out-of-range
>> data.
>>
>> Suggested-by: Brian Foster <bfoster@redhat.com>
>> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
>> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> ---
>>   mm/internal.h |  4 +--
>>   mm/shmem.c    | 13 +++-----
>>   mm/truncate.c | 89 ++++++++++++++++++++++++++++++++++++---------------
>>   3 files changed, 71 insertions(+), 35 deletions(-)
>>
> ...
>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index b58ba940be47..bec6d881d022 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
> ...
>> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>   		 * for shmem truncate
>>   		 */
>>   		struct folio *folio2;
>> +		pgoff_t end, aligned_end = (pos + offset + length) >>
>> +					   PAGE_SHIFT;
>>   
>> -		if (offset + length == size)
>> -			goto no_split;
>> +		if (pstart)
>> +			*pstart = round_up(pos + offset, PAGE_SIZE) >>
>> +				  PAGE_SHIFT;
>> +
>> +		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);
>>   
>> +		/*
>> +		 * 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 no_split;
>> -
>> -		if (!folio_test_large(folio2))
>>   			goto out;
>>   
>> +		if (folio2->mapping != folio->mapping ||
>> +		    !folio_test_large(folio2))
>> +			goto out_put;
>> +
> 
> Hi Zhang,
> 
> The only thing that sticks out to me in this version is the
> !folio_test_large() check above. If we split (or something happens) such
> that folio2 is no longer large, wouldn't it be more likely to catch it
> here and fail to update end like we do in the same check a bit further
> down after the folio lock? Maybe I'm missing something here, but it
> seems a little odd for the same condition to return two possible states
> like this. Otherwise I like the changes you've made and the rest of the
> patch LGTM.
> 
> Brian

Yes, you're right! That check should be dropped, I got myself
confused with all these complicated checks. Fortunately Jan and Zi Yan
suggested using __filemap_get_folio(), and with that I think we can
get rid of all these convoluted checks.

Thanks,
Yi.

> 
>>   		if (!folio_trylock(folio2))
>> -			goto out;
>> +			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);
>> +		if (page_folio(split_at2) != folio2) {
>> +			folio_unlock(folio2);
>> +			goto out_put;
>> +		}
>> +		if (!folio_test_large(folio2)) {
>> +			end = aligned_end;
>> +			folio_unlock(folio2);
>> +			goto out_put;
>> +		}
>> +
>> +		/* 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:
>> +out_put:
>>   		folio_put(folio2);
>> -no_split:
>> +out:
>> +		if (pend)
>> +			*pend = end;
>>   		return true;
>>   	}
>>   	if (folio_test_dirty(folio))
>> @@ -413,11 +455,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 +466,8 @@ 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
>>
>

On 9/17/2026 7:38 PM, Zhang Yi wrote:
> On 9/17/2026 2:03 AM, Zi Yan wrote:
>> On Wed Sep 16, 2026 at 8:02 AM EDT, Jan Kara wrote:
>>> On Wed 16-09-26 17:24:48, 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.
>>>>
>>>> Rework the contract so the caller is told the page range to discard:
>>>>
>>>>    - 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.
>>>>
>>>>    - Adjust the ordering of the validate check when splitting folio2.
>>>>      folio2->index is only reliable after the reference count and lock
>>>>      have been successfully acquired, since it may have been split
>>>>      concurrently, or freed and recycled to an unrelated mapping. On any
>>>>      failure to obtain a reliable end position, fall back to
>>>>      folio->index, which is safe but leaves the sub-folios split off at
>>>>      the offset edge in the page cache.
>>>>
>>>>    - Rename the byte-range parameters start/end to lstart/lend to better
>>>>      express their semantics.
>>>>
>>>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
>>>> &pstart for the folio at the start edge and &pend for the folio at the
>>>> end edge, so the truncate loop drops exactly the fully covered pages and
>>>> never touches a straddling folio that still holds valid out-of-range
>>>> data.
>>>>
>>>> Suggested-by: Brian Foster <bfoster@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>
>>>
>>> The changes mostly look good to me but I have some confusion around the
>>> folio2 splitting below.
>>>
>>>> @@ -259,32 +271,62 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>>>            * for shmem truncate
>>>>            */
>>>>           struct folio *folio2;
>>>> +        pgoff_t end, aligned_end = (pos + offset + length) >>
>>>> +                       PAGE_SHIFT;
>>>>   -        if (offset + length == size)
>>>> -            goto no_split;
>>>> +        if (pstart)
>>>> +            *pstart = round_up(pos + offset, PAGE_SIZE) >>
>>>> +                  PAGE_SHIFT;
>>>> +
>>>> +        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);
>>>>   +        /*
>>>> +         * 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 no_split;
>>>> -
>>>> -        if (!folio_test_large(folio2))
>>>>               goto out;
>>>>   +        if (folio2->mapping != folio->mapping ||
>>>> +            !folio_test_large(folio2))
>>>> +            goto out_put;
>>>> +
>>>>           if (!folio_trylock(folio2))
>>>> -            goto out;
>>>> +            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);
>>>> +        if (page_folio(split_at2) != folio2) {
>>>> +            folio_unlock(folio2);
>>>> +            goto out_put;
>>>> +        }
>>>> +        if (!folio_test_large(folio2)) {
>>>> +            end = aligned_end;
>>>> +            folio_unlock(folio2);
>>>> +            goto out_put;
>>>> +        }
>>>
>>> So I always found this folio2 lookup and revalidation somewhat suspicious
>>> and now that we're digging into it I'll ask: As you note in the changelog,
>>> by the time we compute split_at2 the page can be already freed and reused
>>> because it was split off from the original 'folio'. It can be for example a
>>> slab page or anything else. So is it guaranteed that page_folio() actually
>>
>> For a slab or unrelated page, folio->mapping check rejects them.
>>
>>> returns something sensible? What guarantees we properly detect the "reuse
>>
>> No, but folio_try_get() prevents non ref'd folios.
>>
>>> for something else" case in all possible cases for which the page can be
>>> reused? I understand this is mostly a preexisting issue so maybe these
>>> questions are more for MM guys than you...
>>
>> I agree with you that trying to split a unlocked and not ref'd folio2 is
>> flaky. And it almost does a __filemap_get_folio() like you proposed
>> below.
>>
> 
> I think this is indeed a pre-existing issue, as Sashiko pointed out:
> 
>> [Severity: Critical]
>> Does this speculative lookup handle the reallocation race correctly if the
>> folio is reallocated to a different file offset?
>>
>> After the first split, the tail pages are added to the page cache and
>> unlocked. During speculative lookup, the tail page could be concurrently
>> reclaimed, freed, and reallocated as a new large folio in the same mapping
>> but at a completely different file offset.
>>
>> The validation checks earlier and the page_folio check here will all pass
>> because the physical page split_at2 is part of the new allocation.
>>
>> However, the file index of the folio is not verified. If this unrelated large
>> folio fails to split, the fallback path assigns its index to end here:
>>
>>     if (folio_split_or_unmap(folio2, split_at2, min_order))
>>         end = folio2->index;
>>
>> This unrelated file offset is then returned to the caller via pend, which
>> can cause the hole punch loop to truncate an unrelated segment of the file.
>>
>> Should we verify that the file index of folio2 matches the expected index
>> (e.g. folio2->index + folio_page_idx(folio2, split_at2) == expected_index)
>> before using it?
> 
> the current mapping check on the folio2 and the folio !=
> page_folio(split_at) check in __folio_split() are not able to catch this
> problem, which could then cause folio2 to be split at the wrong
> position. Does that sound right?
> 
>>>
>>> So for me as an filesystem guy I'd appreciate some comment in this code
>>> explaining why grabbing folio2 this way is actually safe. The really safe
>>> way of getting to folio2 would be to use
>>>    __filemap_get_folio(mapping, (offset+length) >> PAGE_SHIFT, FGP_LOCK, 0)
>>> but I suppose we don't use the mapping lookup as it is more expensive?
>>
>> __filemap_get_folio() is a better version of the existing folio2 finding
>> code. When I wrote the code, I never thought about using
>> __filemap_get_folio(), but I probably should have done it.
>>
>> BTW,
>> __filemap_get_folio(mapping, folio->index + (offset+length) >> PAGE_SHIFT,
>>         FGP_LOCK | FGP_NOWAIT, 0)
>> might be better:
>> 1. folio->index is needed to get the right folio2,
>> 2. nowait can us nonblocking.
>>
>>
> 
> Following your suggestion, I reworked my existing patch to use
> __filemap_get_folio(), and it ends up looking roughly like the
> following(not tested yet). What do you think?
> 
> Besides, if the issue Sashiko pointed out does indeed exist, I can
> extract the __filemap_get_folio() change into a separate patch to fix it
> on its own.
> 
> Thanks,
> Yi.
> 

Sorry, the previous diff format was incorrect, reposting it.

diff --git a/mm/truncate.c b/mm/truncate.c
index 23c90f00b530..b5e5bdf5f9ef 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -279,51 +279,46 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
 			*pstart = round_up(pos + offset,
 					   min_nrbytes) >> PAGE_SHIFT;

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

 		/*
-		 * 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.
+		 * After the first split at the start edge, the folio at the
+		 * end edge may be freed and reused concurrently.
+		 * __filemap_get_folio() looks up the straddler at
+		 * aligned_end and returns it locked and ref'd with the
+		 * mapping validated.
 		 */
-		end = folio->index;
-		if (!folio_try_get(folio2))
+		folio2 = __filemap_get_folio(folio->mapping, aligned_end,
+					     FGP_LOCK | FGP_NOWAIT, 0);
+		if (IS_ERR(folio2)) {
+			/*
+			 * No sub-folio straddles the boundary when aligned_end
+			 * is empty so discarding up to it is safe.  Otherwise
+			 * the straddler is locked by someone else and we
+			 * cannot obtain a reliable end position, so we fall
+			 * back to folio->index.  This is safe but may leave
+			 * pages split off at the offset edge lingering in the
+			 * page cache this round.
+			 */
+			if (PTR_ERR(folio2) != -ENOENT)
+				end = folio->index;
 			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);
-			goto out_put;
-		}
-		if (!folio_test_large(folio2)) {
-			end = aligned_end;
-			folio_unlock(folio2);
-			goto out_put;
 		}

-		/* Split failed: back off to the head of the straddler */
+		/* Already at the minimum order, nothing to split */
+		if (folio_order(folio2) == min_order)
+			goto out_put;
+
+		split_at2 = folio_page(folio2, (aligned_end - folio2->index));
+
+		/* Split failed, keep the straddler intact */
 		if (folio_split_or_unmap(folio2, split_at2, min_order))
 			end = folio2->index;
-		else
-			end = aligned_end;

-		folio_unlock(folio2);
 out_put:
+		folio_unlock(folio2);
 		folio_put(folio2);
 out:
 		if (pend)

On Thu 17-09-26 20:03:57, Zhang Yi wrote:
> Sorry, the previous diff format was incorrect, reposting it.
> 
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 23c90f00b530..b5e5bdf5f9ef 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -279,51 +279,46 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>  			*pstart = round_up(pos + offset,
>  					   min_nrbytes) >> PAGE_SHIFT;
> 
> -		if (offset + length == size) {
> -			end = aligned_end;
> +		end = aligned_end;
> +		if (offset + length == size)
>  			goto out;
> -		}
> -
> -		split_at2 = folio_page(folio,
> -				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
> -		folio2 = page_folio(split_at2);
> 
>  		/*
> -		 * 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.
> +		 * After the first split at the start edge, the folio at the
> +		 * end edge may be freed and reused concurrently.
> +		 * __filemap_get_folio() looks up the straddler at
> +		 * aligned_end and returns it locked and ref'd with the
> +		 * mapping validated.
>  		 */
> -		end = folio->index;
> -		if (!folio_try_get(folio2))
> +		folio2 = __filemap_get_folio(folio->mapping, aligned_end,
> +					     FGP_LOCK | FGP_NOWAIT, 0);
> +		if (IS_ERR(folio2)) {
> +			/*
> +			 * No sub-folio straddles the boundary when aligned_end
> +			 * is empty so discarding up to it is safe.  Otherwise
> +			 * the straddler is locked by someone else and we
> +			 * cannot obtain a reliable end position, so we fall
> +			 * back to folio->index.  This is safe but may leave
> +			 * pages split off at the offset edge lingering in the
> +			 * page cache this round.
> +			 */
> +			if (PTR_ERR(folio2) != -ENOENT)
> +				end = folio->index;
>  			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);
> -			goto out_put;
> -		}
> -		if (!folio_test_large(folio2)) {
> -			end = aligned_end;
> -			folio_unlock(folio2);
> -			goto out_put;
>  		}
> 
> -		/* Split failed: back off to the head of the straddler */
> +		/* Already at the minimum order, nothing to split */
> +		if (folio_order(folio2) == min_order)
> +			goto out_put;
> +
> +		split_at2 = folio_page(folio2, (aligned_end - folio2->index));
> +
> +		/* Split failed, keep the straddler intact */
>  		if (folio_split_or_unmap(folio2, split_at2, min_order))
>  			end = folio2->index;
> -		else
> -			end = aligned_end;
> 
> -		folio_unlock(folio2);
>  out_put:
> +		folio_unlock(folio2);
>  		folio_put(folio2);
>  out:
>  		if (pend)

FWIW this looks much more understandable to me. And since this is a
preexisting issue I agree this should be a separate patch.

								Honza

On 9/21/2026 5:05 PM, Jan Kara wrote:
> On Thu 17-09-26 20:03:57, Zhang Yi wrote:
>> Sorry, the previous diff format was incorrect, reposting it.
>>
>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index 23c90f00b530..b5e5bdf5f9ef 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
>> @@ -279,51 +279,46 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>>  			*pstart = round_up(pos + offset,
>>  					   min_nrbytes) >> PAGE_SHIFT;
>>
>> -		if (offset + length == size) {
>> -			end = aligned_end;
>> +		end = aligned_end;
>> +		if (offset + length == size)
>>  			goto out;
>> -		}
>> -
>> -		split_at2 = folio_page(folio,
>> -				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
>> -		folio2 = page_folio(split_at2);
>>
>>  		/*
>> -		 * 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.
>> +		 * After the first split at the start edge, the folio at the
>> +		 * end edge may be freed and reused concurrently.
>> +		 * __filemap_get_folio() looks up the straddler at
>> +		 * aligned_end and returns it locked and ref'd with the
>> +		 * mapping validated.
>>  		 */
>> -		end = folio->index;
>> -		if (!folio_try_get(folio2))
>> +		folio2 = __filemap_get_folio(folio->mapping, aligned_end,
>> +					     FGP_LOCK | FGP_NOWAIT, 0);
>> +		if (IS_ERR(folio2)) {
>> +			/*
>> +			 * No sub-folio straddles the boundary when aligned_end
>> +			 * is empty so discarding up to it is safe.  Otherwise
>> +			 * the straddler is locked by someone else and we
>> +			 * cannot obtain a reliable end position, so we fall
>> +			 * back to folio->index.  This is safe but may leave
>> +			 * pages split off at the offset edge lingering in the
>> +			 * page cache this round.
>> +			 */
>> +			if (PTR_ERR(folio2) != -ENOENT)
>> +				end = folio->index;
>>  			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);
>> -			goto out_put;
>> -		}
>> -		if (!folio_test_large(folio2)) {
>> -			end = aligned_end;
>> -			folio_unlock(folio2);
>> -			goto out_put;
>>  		}
>>
>> -		/* Split failed: back off to the head of the straddler */
>> +		/* Already at the minimum order, nothing to split */
>> +		if (folio_order(folio2) == min_order)
>> +			goto out_put;
>> +
>> +		split_at2 = folio_page(folio2, (aligned_end - folio2->index));
>> +
>> +		/* Split failed, keep the straddler intact */
>>  		if (folio_split_or_unmap(folio2, split_at2, min_order))
>>  			end = folio2->index;
>> -		else
>> -			end = aligned_end;
>>
>> -		folio_unlock(folio2);
>>  out_put:
>> +		folio_unlock(folio2);
>>  		folio_put(folio2);
>>  out:
>>  		if (pend)
> 
> FWIW this looks much more understandable to me. And since this is a
> preexisting issue I agree this should be a separate patch.
> 
> 								Honza

Thank you for confirming. I will send out the next iteration after
testing.

Thanks,
Yi.