| Message ID | 1570012248-16099-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add device scanned-by process name in the scan message | expand |
On 2.10.19 г. 13:30 ч., Anand Jain wrote: > Its very helpful if we had logged the device scanner process name > to debug the race condition between the systemd-udevd scan and the > user initiated device forget command. > > This patch adds scanned-by process name to the scan message. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Same effect can be achieved (for debugging purposes) if you have used ftrace on device_list_add without needing to patch the kernel. I'm somewhat indifferent whether this will be merged or not but I personally don't see much value in it. > --- > fs/btrfs/volumes.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 788271649726..2c4c89bfafd1 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1011,11 +1011,11 @@ static noinline struct btrfs_device *device_list_add(const char *path, > *new_device_added = true; > > if (disk_super->label[0]) > - pr_info("BTRFS: device label %s devid %llu transid %llu %s\n", > - disk_super->label, devid, found_transid, path); > + pr_info("BTRFS: device label %s devid %llu transid %llu %s scanned-by %s\n", > + disk_super->label, devid, found_transid, path, current->comm); > else > - pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s\n", > - disk_super->fsid, devid, found_transid, path); > + pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s scanned-by %s\n", > + disk_super->fsid, devid, found_transid, path, current->comm); > > } else if (!device->name || strcmp(device->name->str, path)) { > /* >
On 10/2/19 6:39 PM, Nikolay Borisov wrote: > > > On 2.10.19 г. 13:30 ч., Anand Jain wrote: >> Its very helpful if we had logged the device scanner process name >> to debug the race condition between the systemd-udevd scan and the >> user initiated device forget command. >> >> This patch adds scanned-by process name to the scan message. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> > > Same effect can be achieved (for debugging purposes) if you have used > ftrace on device_list_add without needing to patch the kernel. > > I'm somewhat indifferent whether this will be merged or not but I > personally don't see much value in it. Its always good to provide a completely cooked log messages. Half cooked ideas or the log messages leads to more questions than answers. Thanks, Anand
On Wed, Oct 02, 2019 at 06:30:48PM +0800, Anand Jain wrote: > Its very helpful if we had logged the device scanner process name > to debug the race condition between the systemd-udevd scan and the > user initiated device forget command. > > This patch adds scanned-by process name to the scan message. Then PID should be printed as well, otherwise ok. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/volumes.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 788271649726..2c4c89bfafd1 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1011,11 +1011,11 @@ static noinline struct btrfs_device *device_list_add(const char *path, > *new_device_added = true; > > if (disk_super->label[0]) > - pr_info("BTRFS: device label %s devid %llu transid %llu %s\n", > - disk_super->label, devid, found_transid, path); > + pr_info("BTRFS: device label %s devid %llu transid %llu %s scanned-by %s\n", > + disk_super->label, devid, found_transid, path, current->comm); > else > - pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s\n", > - disk_super->fsid, devid, found_transid, path); > + pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s scanned-by %s\n", pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s scanned by %s(%d)\n", I'm not sure if %di si the right specifier for pid, the format name(pid) can be seen in other messages.
On Wed, Oct 02, 2019 at 01:39:03PM +0300, Nikolay Borisov wrote: > > > On 2.10.19 г. 13:30 ч., Anand Jain wrote: > > Its very helpful if we had logged the device scanner process name > > to debug the race condition between the systemd-udevd scan and the > > user initiated device forget command. > > > > This patch adds scanned-by process name to the scan message. > > > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > > Same effect can be achieved (for debugging purposes) if you have used > ftrace on device_list_add without needing to patch the kernel. For reproducible issues adding a debugging hooks is fine but races can be tricky and device scanning depends on the state of the system and other processes so I understand the need to document what happens for post-mortem analysis. > I'm somewhat indifferent whether this will be merged or not but I > personally don't see much value in it. We have messages for many administrative tasks, either requested by users or by developers so I don't object in principle against adding more.
On Wed, Oct 02, 2019 at 06:30:48PM +0800, Anand Jain wrote: > Its very helpful if we had logged the device scanner process name > to debug the race condition between the systemd-udevd scan and the > user initiated device forget command. > > This patch adds scanned-by process name to the scan message. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Added to misc-next with the updated message.
On 3/10/19 9:24 PM, David Sterba wrote: > On Wed, Oct 02, 2019 at 06:30:48PM +0800, Anand Jain wrote: >> Its very helpful if we had logged the device scanner process name >> to debug the race condition between the systemd-udevd scan and the >> user initiated device forget command. >> >> This patch adds scanned-by process name to the scan message. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> > > Added to misc-next with the updated message. > oops I had v2 with pid. sorry for the delay I got stuck with a debugging. Sending it now. Thanks, Anand
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 788271649726..2c4c89bfafd1 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1011,11 +1011,11 @@ static noinline struct btrfs_device *device_list_add(const char *path, *new_device_added = true; if (disk_super->label[0]) - pr_info("BTRFS: device label %s devid %llu transid %llu %s\n", - disk_super->label, devid, found_transid, path); + pr_info("BTRFS: device label %s devid %llu transid %llu %s scanned-by %s\n", + disk_super->label, devid, found_transid, path, current->comm); else - pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s\n", - disk_super->fsid, devid, found_transid, path); + pr_info("BTRFS: device fsid %pU devid %llu transid %llu %s scanned-by %s\n", + disk_super->fsid, devid, found_transid, path, current->comm); } else if (!device->name || strcmp(device->name->str, path)) { /*
Its very helpful if we had logged the device scanner process name to debug the race condition between the systemd-udevd scan and the user initiated device forget command. This patch adds scanned-by process name to the scan message. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)