[PATCH] btrfs: include device major and minor numbers in the device scan notice

[PATCH] btrfs: include device major and minor numbers in the device scan notice

[Anand Jain]

To better debug issues surrounding device scans, include the device's
major and minor numbers in the device scan notice for btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 32312f0de2bb..6db37615a3e5 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -824,13 +824,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 
 		if (disk_super->label[0])
 			pr_info(
-	"BTRFS: device label %s devid %llu transid %llu %s scanned by %s (%d)\n",
+"BTRFS: device label %s devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",
 				disk_super->label, devid, found_transid, path,
+				MAJOR(path_devt), MINOR(path_devt),
 				current->comm, task_pid_nr(current));
 		else
 			pr_info(
-	"BTRFS: device fsid %pU devid %llu transid %llu %s scanned by %s (%d)\n",
+"BTRFS: device fsid %pU devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",
 				disk_super->fsid, devid, found_transid, path,
+				MAJOR(path_devt), MINOR(path_devt),
 				current->comm, task_pid_nr(current));
 
 	} else if (!device->name || strcmp(device->name->str, path)) {
-- 
2.38.1

Re: [PATCH] btrfs: include device major and minor numbers in the device scan notice

[Filipe Manana]

On Fri, Feb 23, 2024 at 11:27 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> To better debug issues surrounding device scans, include the device's
> major and minor numbers in the device scan notice for btrfs.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 32312f0de2bb..6db37615a3e5 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -824,13 +824,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>
>                 if (disk_super->label[0])
>                         pr_info(
> -       "BTRFS: device label %s devid %llu transid %llu %s scanned by %s (%d)\n",
> +"BTRFS: device label %s devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",

Can we please leave a space before the opening parentheses?
So that it's consistent with the rest of the message and more readable
(I believe it's also more formal English).


>                                 disk_super->label, devid, found_transid, path,
> +                               MAJOR(path_devt), MINOR(path_devt),
>                                 current->comm, task_pid_nr(current));
>                 else
>                         pr_info(
> -       "BTRFS: device fsid %pU devid %llu transid %llu %s scanned by %s (%d)\n",
> +"BTRFS: device fsid %pU devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",

Same here.

With that adjusted:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

>                                 disk_super->fsid, devid, found_transid, path,
> +                               MAJOR(path_devt), MINOR(path_devt),
>                                 current->comm, task_pid_nr(current));
>
>         } else if (!device->name || strcmp(device->name->str, path)) {
> --
> 2.38.1
>
>

Re: [PATCH] btrfs: include device major and minor numbers in the device scan notice

[Anand Jain]

On 2/23/24 21:16, Filipe Manana wrote:
> On Fri, Feb 23, 2024 at 11:27 AM Anand Jain <anand.jain@oracle.com> wrote:
>>
>> To better debug issues surrounding device scans, include the device's
>> major and minor numbers in the device scan notice for btrfs.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/volumes.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 32312f0de2bb..6db37615a3e5 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -824,13 +824,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>>
>>                  if (disk_super->label[0])
>>                          pr_info(
>> -       "BTRFS: device label %s devid %llu transid %llu %s scanned by %s (%d)\n",
>> +"BTRFS: device label %s devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",
> 
> Can we please leave a space before the opening parentheses?

> So that it's consistent with the rest of the message and more readable
> (I believe it's also more formal English).

Sure.
> 
> 
>>                                  disk_super->label, devid, found_transid, path,
>> +                               MAJOR(path_devt), MINOR(path_devt),
>>                                  current->comm, task_pid_nr(current));
>>                  else
>>                          pr_info(
>> -       "BTRFS: device fsid %pU devid %llu transid %llu %s scanned by %s (%d)\n",
>> +"BTRFS: device fsid %pU devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",
> 
> Same here.
> 

yep.

Thanks,  Anand


> With that adjusted:
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> 
> Thanks.
> 
>>                                  disk_super->fsid, devid, found_transid, path,
>> +                               MAJOR(path_devt), MINOR(path_devt),
>>                                  current->comm, task_pid_nr(current));
>>
>>          } else if (!device->name || strcmp(device->name->str, path)) {
>> --
>> 2.38.1
>>
>>

Re: [PATCH] btrfs: include device major and minor numbers in the device scan notice

[Sweet Tea Dorminy]

On 2/23/24 06:26, Anand Jain wrote:
> To better debug issues surrounding device scans, include the device's
> major and minor numbers in the device scan notice for btrfs.
> 

It would also be nice to add in maj:min into all the error messages in 
that function too, if you're sending a new version. Or I can send 
another patch with that if it feels too different in spirit to you.

But either way (with Filipe's adjustment):
Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Re: [PATCH] btrfs: include device major and minor numbers in the device scan notice

[Anand Jain]

On 2/23/24 21:42, Sweet Tea Dorminy wrote:
> 
> 
> On 2/23/24 06:26, Anand Jain wrote:
>> To better debug issues surrounding device scans, include the device's
>> major and minor numbers in the device scan notice for btrfs.
>>
> 
> It would also be nice to add in maj:min into all the error messages in 
> that function too, if you're sending a new version. Or I can send 
> another patch with that if it feels too different in spirit to you.
> 

Sure, I'll add that, certainly these logs need clarity. Thanks.

-Anand

> But either way (with Filipe's adjustment):
> Reviewed-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Re: [PATCH] btrfs: include device major and minor numbers in the device scan notice

[Qu Wenruo]

在 2024/2/23 21:56, Anand Jain 写道:
> To better debug issues surrounding device scans, include the device's
> major and minor numbers in the device scan notice for btrfs.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Maybe you want to expand the device number to all other device name output?

Especially considering the recent device name problem, device
major/minor looks a much better supplement.

Thanks,
Qu
> ---
>   fs/btrfs/volumes.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 32312f0de2bb..6db37615a3e5 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -824,13 +824,15 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>
>   		if (disk_super->label[0])
>   			pr_info(
> -	"BTRFS: device label %s devid %llu transid %llu %s scanned by %s (%d)\n",
> +"BTRFS: device label %s devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",
>   				disk_super->label, devid, found_transid, path,
> +				MAJOR(path_devt), MINOR(path_devt),
>   				current->comm, task_pid_nr(current));
>   		else
>   			pr_info(
> -	"BTRFS: device fsid %pU devid %llu transid %llu %s scanned by %s (%d)\n",
> +"BTRFS: device fsid %pU devid %llu transid %llu %s(%d:%d) scanned by %s (%d)\n",
>   				disk_super->fsid, devid, found_transid, path,
> +				MAJOR(path_devt), MINOR(path_devt),
>   				current->comm, task_pid_nr(current));
>
>   	} else if (!device->name || strcmp(device->name->str, path)) {

Re: [PATCH] btrfs: include device major and minor numbers in the device scan notice

[Anand Jain]

On 2/24/24 01:27, Qu Wenruo wrote:
> 
> 
> 在 2024/2/23 21:56, Anand Jain 写道:
>> To better debug issues surrounding device scans, include the device's
>> major and minor numbers in the device scan notice for btrfs.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> Maybe you want to expand the device number to all other device name output?
> 
> Especially considering the recent device name problem, device
> major/minor looks a much better supplement.

  Yeah.. added to all scan related messages.

Thanks, Anand

> 
> Thanks,
> Qu
>> ---
>>   fs/btrfs/volumes.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 32312f0de2bb..6db37615a3e5 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -824,13 +824,15 @@ static noinline struct btrfs_device 
>> *device_list_add(const char *path,
>>
>>           if (disk_super->label[0])
>>               pr_info(
>> -    "BTRFS: device label %s devid %llu transid %llu %s scanned by %s 
>> (%d)\n",
>> +"BTRFS: device label %s devid %llu transid %llu %s(%d:%d) scanned by 
>> %s (%d)\n",
>>                   disk_super->label, devid, found_transid, path,
>> +                MAJOR(path_devt), MINOR(path_devt),
>>                   current->comm, task_pid_nr(current));
>>           else
>>               pr_info(
>> -    "BTRFS: device fsid %pU devid %llu transid %llu %s scanned by %s 
>> (%d)\n",
>> +"BTRFS: device fsid %pU devid %llu transid %llu %s(%d:%d) scanned by 
>> %s (%d)\n",
>>                   disk_super->fsid, devid, found_transid, path,
>> +                MAJOR(path_devt), MINOR(path_devt),
>>                   current->comm, task_pid_nr(current));
>>
>>       } else if (!device->name || strcmp(device->name->str, path)) {