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

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

在 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.

That folio_shift is assigned inside the first patch:
(Although in the first patch, there is a bug that it should assign
folio_shift, not sectorsize_bits again)

+	fs_info->folio_size = PAGE_SIZE;
+	fs_info->folio_shift = PAGE_SHIFT;
+	if (sectorsize > PAGE_SIZE) {
+		/* For future multi-page sectorsize support */
+		fs_info->folio_size = sectorsize;
+		fs_info->folio_shift  = fs_info->sectorsize_bits;
+	} else {
+		fs_info->folio_size = PAGE_SIZE;
+		fs_info->folio_shift = PAGE_SHIFT;
+	}

>
>>   	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.
Mind to explain why the folio->index would be garbage?

Thanks,
Qu

>
>