Re: [PATCH 2/3] btrfs: use folio_contains() for EOF detection

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2025/4/8 04:09, David Sterba 写道:
> On Fri, Apr 04, 2025 at 12:17:41PM +1030, Qu Wenruo wrote:
>> Currently we use the following pattern to detect if the folio contains
>> the end of a file:
>>
>> 	if (folio->index == end_index)
>> 		folio_zero_range();
>>
>> But that only works if the folio is page sized.
>>
>> For the following case, it will not work and leave the range beyond EOF
>> uninitialized:
>>
>>    The page size is 4K, and the fs block size is also 4K.
>>
>> 	16K        20K       24K
>>          |          |     |   |
>> 	                 |
>>                           EOF at 22K
>>
>> And we have a large folio sized 8K at file offset 16K.
>>
>> In that case, the old "folio->index == end_index" will not work, thus
>> we the range [22K, 24K) will not be zeroed out.
>>
>> Fix the following call sites which use the above pattern:
>>
>> - add_ra_bio_pages()
>>
>> - extent_writepage()
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/compression.c | 2 +-
>>   fs/btrfs/extent_io.c   | 6 +++---
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
>> index cb954f9bc332..7aa63681f92a 100644
>> --- a/fs/btrfs/compression.c
>> +++ b/fs/btrfs/compression.c
>> @@ -523,7 +523,7 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>>   		free_extent_map(em);
>>   		unlock_extent(tree, cur, page_end, NULL);
>>
>> -		if (folio->index == end_index) {
>> +		if (folio_contains(folio, end_index)) {
>>   			size_t zero_offset = offset_in_folio(folio, isize);
>>
>>   			if (zero_offset) {
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 013268f70621..f0d51f6ed951 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -221,7 +221,7 @@ static void __process_folios_contig(struct address_space *mapping,
>>   }
>>
>>   static noinline void unlock_delalloc_folio(const struct inode *inode,
>> -					   const struct folio *locked_folio,
>> +					   struct folio *locked_folio,
>
> I'm not happy to see removing const from the parameters as it's quite
> tedious to find them. Here it's not necessary as it's still not changing
> the folio, only required because folio API is not const-clean,
> folio_contains() in particular.
>

Yes, I'm not happy with that either, and I'm planning to constify the
parameters for those helpers in a dedicated series.

Thanks,
Qu