Re: [PATCH AUTOSEL 6.14 099/642] btrfs: prevent inline data extents read from touching blocks beyond its range

[Sasha Levin <sashal@kernel.org>]

On Tue, May 06, 2025 at 03:19:13PM +0200, David Sterba wrote:
>On Mon, May 05, 2025 at 06:05:15PM -0400, Sasha Levin wrote:
>> From: Qu Wenruo <wqu@suse.com>
>>
>> [ Upstream commit 1a5b5668d711d3d1ef447446beab920826decec3 ]
>>
>> Currently reading an inline data extent will zero out the remaining
>> range in the page.
>>
>> This is not yet causing problems even for block size < page size
>> (subpage) cases because:
>>
>> 1) An inline data extent always starts at file offset 0
>>    Meaning at page read, we always read the inline extent first, before
>>    any other blocks in the page. Then later blocks are properly read out
>>    and re-fill the zeroed out ranges.
>>
>> 2) Currently btrfs will read out the whole page if a buffered write is
>>    not page aligned
>>    So a page is either fully uptodate at buffered write time (covers the
>>    whole page), or we will read out the whole page first.
>>    Meaning there is nothing to lose for such an inline extent read.
>>
>> But it's still not ideal:
>>
>> - We're zeroing out the page twice
>>   Once done by read_inline_extent()/uncompress_inline(), once done by
>>   btrfs_do_readpage() for ranges beyond i_size.
>>
>> - We're touching blocks that don't belong to the inline extent
>>   In the incoming patches, we can have a partial uptodate folio, of
>>   which some dirty blocks can exist while the page is not fully uptodate:
>>
>>   The page size is 16K and block size is 4K:
>>
>>   0         4K        8K        12K        16K
>>   |         |         |/////////|          |
>>
>>   And range [8K, 12K) is dirtied by a buffered write, the remaining
>>   blocks are not uptodate.
>>
>>   If range [0, 4K) contains an inline data extent, and we try to read
>>   the whole page, the current behavior will overwrite range [8K, 12K)
>>   with zero and cause data loss.
>>
>> So to make the behavior more consistent and in preparation for future
>> changes, limit the inline data extents read to only zero out the range
>> inside the first block, not the whole page.
>>
>> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> Signed-off-by: David Sterba <dsterba@suse.com>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>This is not a stable dependency and the patch is not fixing anything
>but a preparation so this does not make much sense for stable backports,
>please drop it. Thanks.

Will do, thanks!

-- 
Thanks,
Sasha