Re: [PATCH 2/3] btrfs: Fix two misuses of folio_shift()

[David Sterba <dsterba@suse.cz>]

On Tue, Jan 21, 2025 at 05:51:40PM +1030, Qu Wenruo wrote:
> 
> 
> 在 2025/1/21 16:10, Matthew Wilcox (Oracle) 写道:
> > It is meaningless to shift a byte count by folio_shift().  The folio index
> > is in units of PAGE_SIZE, not folio_size().  We can use folio_contains()
> > to make this work for arbitrary-order folios, so remove the assertion
> > that the folios are of order 0.
> > 
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > ---
> >   fs/btrfs/extent_io.c | 8 +++-----
> >   1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > index 289ecb8ce217..c9b0ee841501 100644
> > --- a/fs/btrfs/extent_io.c
> > +++ b/fs/btrfs/extent_io.c
> > @@ -523,8 +523,6 @@ static void end_bbio_data_read(struct btrfs_bio *bbio)
> >   		u64 end;
> >   		u32 len;
> >   
> > -		/* For now only order 0 folios are supported for data. */
> > -		ASSERT(folio_order(folio) == 0);
> 
> I'd prefer to keep this ASSERT(), as all the btrfs_folio_*() helpers can 
> only handle page sized folio for now.
> 
> >   		btrfs_debug(fs_info,
> >   			"%s: bi_sector=%llu, err=%d, mirror=%u",
> >   			__func__, bio->bi_iter.bi_sector, bio->bi_status,
> > @@ -552,7 +550,6 @@ static void end_bbio_data_read(struct btrfs_bio *bbio)
> >   
> >   		if (likely(uptodate)) {
> >   			loff_t i_size = i_size_read(inode);
> > -			pgoff_t end_index = i_size >> folio_shift(folio);
> >   
> >   			/*
> >   			 * Zero out the remaining part if this range straddles
> > @@ -563,7 +560,8 @@ static void end_bbio_data_read(struct btrfs_bio *bbio)
> >   			 *
> >   			 * NOTE: i_size is exclusive while end is inclusive.
> >   			 */
> > -			if (folio_index(folio) == end_index && i_size <= end) {
> > +			if (folio_contains(folio, i_size >> PAGE_SHIFT) &&
> 
> Although folio_contains() is already a pretty good improvement, can we 
> have a bytenr/i_sized based solution for fs usages?

Good point, something like folio_contains_offset() that does the correct
shifts.