Re: [PATCH RFC] btrfs: allow extent buffer helpers to skip cross-page handling

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

On 2023/11/23 00:16, David Sterba wrote:
> On Thu, Nov 16, 2023 at 03:49:06PM +1030, Qu Wenruo wrote:
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -80,8 +80,16 @@ static void csum_tree_block(struct extent_buffer *buf, u8 *result)
>>   	char *kaddr;
>>   	int i;
>>
>> +	memset(result, 0, BTRFS_CSUM_SIZE);
>>   	shash->tfm = fs_info->csum_shash;
>>   	crypto_shash_init(shash);
>> +
>> +	if (buf->addr) {
>> +		crypto_shash_digest(shash, buf->addr + offset_in_page(buf->start) + BTRFS_CSUM_SIZE,
>> +				    buf->len - BTRFS_CSUM_SIZE, result);
>> +		return;
>> +	}
>
> This duplicates the address and size
>> +
>>   	kaddr = page_address(buf->pages[0]) + offset_in_page(buf->start);
>>   	crypto_shash_update(shash, kaddr + BTRFS_CSUM_SIZE,
>>   			    first_page_part - BTRFS_CSUM_SIZE);
>> @@ -90,7 +98,6 @@ static void csum_tree_block(struct extent_buffer *buf, u8 *result)
>>   		kaddr = page_address(buf->pages[i]);
>>   		crypto_shash_update(shash, kaddr, PAGE_SIZE);
>>   	}
>> -	memset(result, 0, BTRFS_CSUM_SIZE);
>>   	crypto_shash_final(shash, result);
>
> I'd like to have only one code doing the crypto_shash_ calls, so I'm
> suggesting this as the final code (the diff is not clear);

This looks good to me, mind to update it inside your branch?

Thanks,
Qu
>
>   74 static void csum_tree_block(struct extent_buffer *buf, u8 *result)
>   75 {
>   76         struct btrfs_fs_info *fs_info = buf->fs_info;
>   77         int num_pages;
>   78         u32 first_page_part;
>   79         SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>   80         char *kaddr;
>   81         int i;
>   82
>   83         shash->tfm = fs_info->csum_shash;
>   84         crypto_shash_init(shash);
>   85
>   86         if (buf->addr) {
>   87                 /* Pages are contiguous, handle it as one big page. */
>   88                 kaddr = buf->addr;
>   89                 first_page_part = fs_info->nodesize;
>   90                 num_pages = 1;
>   91         } else {
>   92                 kaddr = page_address(buf->pages[0]);
>   93                 first_page_part = min_t(u32, PAGE_SIZE, fs_info->nodesize);
>   94                 num_pages = num_extent_pages(buf);
>   95         }
>   96         kaddr += offset_in_page(buf->start) + BTRFS_CSUM_SIZE;
>   97         first_page_part -= BTRFS_CSUM_SIZE;
>   98
>   99         crypto_shash_update(shash, kaddr + BTRFS_CSUM_SIZE,
> 100                             first_page_part - BTRFS_CSUM_SIZE);
> 101
> 102         for (i = 1; i < num_pages && INLINE_EXTENT_BUFFER_PAGES > 1; i++) {
> 103                 kaddr = page_address(buf->pages[i]);
> 104                 crypto_shash_update(shash, kaddr, PAGE_SIZE);
> 105         }
> 106         memset(result, 0, BTRFS_CSUM_SIZE);
> 107         crypto_shash_final(shash, result);
> 108 }
>