Re: [External] : Re: [PATCH v4 2/4] btrfs: redeclare btrfs_stale_devices arg1 to dev_t

[Anand Jain <anand.jain@oracle.com>]

On 11/01/2022 16:30, Nikolay Borisov wrote:
> 
> 
> On 11.01.22 г. 6:51, Anand Jain wrote:
>>
>>>> @@ -604,14 +599,14 @@ static int btrfs_free_stale_devices(const char
>>>> *path,
>>>>                         &fs_devices->devices, dev_list) {
>>>>                if (skip_device && skip_device == device)
>>>>                    continue;
>>>> -            if (path && !device->name)
>>>> +            if (devt && !device->name)
>>>
>>> This check is now rendered obsolete since ->name is used iff we have
>>> passed a patch to match against it, but since your series removes the
>>> path altogether having device->name becomes obsolete, hence it can be
>>> removed.
>>
>> We have it to check for the missing device. Device->name == '\0' is one
>> of the ways coded to identify a missing device. It helps to fail early
>> instead of failing inside device_matched() at lookup_bdev().
> 
> In this case shouldn't the check be just for if (!device->name) rather
> than also checking for the presence of devt? Also a comment is warranted
> that we are skipping missing devices.

I think you missed the point that %devt is an argument there? It implies 
and frees the device with that matching %devt.
IMO it is straightforward that if %devt present then skips the devices 
without a name.
I will add the comment.

Thanks, Anand