| Message ID | 77fc0b0c7c88b14c734b646d1969ccf45a063146.1599118052.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: improve messages when devices rescanned | expand |
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" > } > > name = rcu_string_strdup(path, GFP_NOFS); >
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)", Thanks, Anand > >> } >> >> name = rcu_string_strdup(path, GFP_NOFS); >>
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); >>> >
On Thu, Sep 03, 2020 at 12:42:09PM +0300, Nikolay Borisov wrote: > > > 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. The device id should still be there, the filesystem is referred to by the device name as printed by btrfs_info* so its uuid can be dropped from the message itself.
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)); } name = rcu_string_strdup(path, GFP_NOFS);
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(-)