Re: [PATCH 1/3] btrfs: volumes: Refactor device holes gathering into a separate function

Johannes Thumshirn
2019-11-07T09:45:12Z

On 07/11/2019 10:33, Qu Wenruo wrote:
>
>
> On 2019/11/7 下午5:20, Johannes Thumshirn wrote:
>> On 07/11/2019 07:27, Qu Wenruo wrote:
>>> +static int gather_dev_holes(struct btrfs_fs_info *info,
>>> + struct btrfs_device_info *devices_info,
>>> + int *index, struct list_head *list,
>>> + int max_nr_devs, u64 stripe_size, int dev_stripes)
>>> +{
>>
>> Hi Qu,
>>
>> What do you think of
>>
>> static int gather_dev_holes(struct btrfs_fs_info *info,
>> int *index, u64 stripe_size,
>> int dev_stripes)
>> {
>> struct btrfs_fs_devices *fs_devices = info->fs_devices;
>> struct btrfs_device *device;
>> int ret;
>> int ndevs = 0;
>>
>> list_for_each_entry(device, &fs_devices->alloc_list,
>> dev_alloc_list) {
>>
>> [...]
>>
>>
>> btrfs_device_info can be derived from btrfs_fs_info, and *list and
>> max_nr_devs can be derived from btrfs_device_info.
>>
>> This reduces the number of arguments by 3 and we don't need to pass that
>> odd 'struct list_head *list' which isn't really clear from the type what
>> it is referring to.
>
> The objective of this refactor is to accept different list, not only
> fs_devices->alloc_list, but also later fs_devices->missing_list.
>
> So I'm afraid it's not that easy to reduce the number of paramters.

hmm this makes the api of gather_dev_holes() even more intransparent.

Looking at patch 3/3 I see why you're doing it though.

--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@suse.de +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850