Re: [PATCH RFC 2/2] btrfs: defrag: prepare defrag for larger data folio size

[Matthew Wilcox <willy@infradead.org>]

On Fri, Feb 16, 2024 at 09:37:01AM +1030, Qu Wenruo wrote:
> 在 2024/2/16 06:53, Matthew Wilcox 写道:
> > On Wed, Jan 24, 2024 at 02:29:08PM +1030, Qu Wenruo wrote:
> > > Although we have migrated defrag to use the folio interface, we can
> > > still further enhance it for the future larger data folio size.
> > 
> > This patch is wrong.  Please drop it.
> > 
> > >   {
> > >   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
> > >   	struct extent_changeset *data_reserved = NULL;
> > >   	const u64 start = target->start;
> > >   	const u64 len = target->len;
> > > -	unsigned long last_index = (start + len - 1) >> PAGE_SHIFT;
> > > -	unsigned long start_index = start >> PAGE_SHIFT;
> > > +	unsigned long last_index = (start + len - 1) >> fs_info->folio_shift;
> > > +	unsigned long start_index = start >> fs_info->folio_shift;
> > 
> > indices are always in multiples of PAGE_SIZE.
> 
> So is the fs_info->folio_shift. It would always be >= PAGE_SHIFT.

No, you don't understand.  folio->index * PAGE_SIZE == byte offset of folio
in the file.  What you've done here breaks that.

> > >   	unsigned long first_index = folios[0]->index;
> > 
> > ... so if you've shifted a file position by some "folio_shift" and then
> > subtracted it from folio->index, you have garbage.
> 
> For the future larger folio support, all folio would be in the size of
> sectorsize.

Yes, folios are always an integer multiple of sector size.  But their
_index_ is expressed as a multiple of the page size.