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

[Zhang Yi]

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.