[PATCH] btrfs: add device scanned-by process name in the scan message

[PATCH] btrfs: add device scanned-by process name in the scan message

[Anand Jain]

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(-)

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)) {
 		/*
-- 
1.8.3.1

Re: [PATCH] btrfs: add device scanned-by process name in the scan message

[Nikolay Borisov]

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)) {
>  		/*
> 

Re: [PATCH] btrfs: add device scanned-by process name in the scan message

[Anand Jain]

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

Re: [PATCH] btrfs: add device scanned-by process name in the scan message

[David Sterba]

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.

Re: [PATCH] btrfs: add device scanned-by process name in the scan message

[David Sterba]

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.

Re: [PATCH] btrfs: add device scanned-by process name in the scan message

[David Sterba]

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.

Re: [PATCH] btrfs: add device scanned-by process name in the scan message

[Anand Jain]

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