Re: [PATCH 04/10] btrfs: split btrfs_submit_data_bio

[Qu Wenruo <wqu@suse.com>]

On 2022/5/5 06:41, Qu Wenruo wrote:
> 
> 
> On 2022/5/4 22:08, Christoph Hellwig wrote:
>> On Wed, May 04, 2022 at 08:38:23PM +0800, Qu Wenruo wrote:
>>>> +    struct inode *inode = tree->private_data;
>>>
>>> I guess we shouldn't use the extent_io_tree for bio->bi_private at all
>>> if we're just tring to grab an inode.
>>>
>>> In fact, for all submit_one_bio() callers, we are handling buffered
>>> read/write, thus we can grab inode using
>>> bio_first_page_all(bio)->mapping->host.
>>>
>>> No need for such weird io_tree based dance.
>>
>> Yes, we can eventully.  Not for this series, though.
> 
> Looking forward to the new cleanups on various weird private member usage.
>>
>>>> -    if (is_data_inode(tree->private_data))
>>>> -        btrfs_submit_data_bio(tree->private_data, bio, mirror_num,
>>>> -                        compress_type);
>>>> +    if (!is_data_inode(tree->private_data))
>>>> +        btrfs_submit_metadata_bio(inode, bio, mirror_num);
>>>
>>> Can we just call btrfs_submit_metadata_bio() and return?
>>>
>>> Every time I see an "if () else if ()", I can't stop thinking if we have
>>> some corner cases not taken into consideration.
>>
>> I generally agree with you, but for this case I think it is pretty
>> simple.  But a few more series down the road these helpers will change
>> a bit anyway, so we can revisit it.
>>
> OK, that sounds good.
> 
> Just a little worried about how many series there still are...
> 
> Thanks,
> Qu
> 

Oh, forgot the tag.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu