Re: [PATCH] btrfs: improve messages when devices rescanned

[Nikolay Borisov <nborisov@suse.com>]

On 3.09.20 г. 12:41 ч., Anand Jain wrote:
> On 3/9/20 4:16 pm, Nikolay Borisov wrote:
>>
>>
>> On 3.09.20 г. 10:27 ч., Anand Jain wrote:
>>> Systems booting without the initramfs seems to scan an unusual kind
>>> of device path. And at a later time, the device is updated to the
>>> correct path. We generally print the process name and PID of the process
>>> scanning the device but we don't capture the same information if the
>>> device path is rescanned with a different pathname.
>>>
>>> But the current message is too long, so drop the unwanted words and add
>>> process name and PID.
>>>
>>> While at this also update the duplicate device warning to include the
>>> process name and PID.
>>>
>>> Reported-by: https://bugzilla.kernel.org/show_bug.cgi?id=89721
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   fs/btrfs/volumes.c | 14 ++++++++------
>>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index dc81646b13c0..c386ad722ae1 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -942,16 +942,18 @@ static noinline struct btrfs_device
>>> *device_list_add(const char *path,
>>>                   bdput(path_bdev);
>>>                   mutex_unlock(&fs_devices->device_list_mutex);
>>>                   btrfs_warn_in_rcu(device->fs_info,
>>> -            "duplicate device fsid:devid for %pU:%llu old:%s new:%s",
>>> -                    disk_super->fsid, devid,
>>> -                    rcu_str_deref(device->name), path);
>>> +    "duplicate device %s devid %llu generation %llu scanned by %s
>>> (%d)",
>>> +                          path, devid, found_transid,
>>> +                          current->comm,
>>> +                          task_pid_nr(current));
>>>                   return ERR_PTR(-EEXIST);
>>>               }
>>>               bdput(path_bdev);
>>>               btrfs_info_in_rcu(device->fs_info,
>>> -                "device fsid %pU devid %llu moved old:%s new:%s",
>>> -                disk_super->fsid, devid,
>>> -                rcu_str_deref(device->name), path);
>>> +                "device path %s changed to %s by %s (pid %d)",
>>> +                      rcu_str_deref(device->name),
>>> +                      path, current->comm,
>>> +                      task_pid_nr(current));
>>
>> This 2nd messages is misleading, it's not the process calling
>> device_list_add which have changed the path per-se but rather it sees
>> the changed path. It's not possible to know why it changed in this
>> context. The idea here is "
>>
>> "Process %pid saw different dev path %new_dev_path for dev %old_path"
> 
>   Hm. How about we stick to the usual scanned by. That is..
> 
>   "device path %s changed to %s scanned by %s (pid %d)",

works for me.

> 
> Thanks, Anand
> 
>>
>>>           }
>>>             name = rcu_string_strdup(path, GFP_NOFS);
>>>
>