diff mbox series

[v2,1/6] ceph: add the *_client debug macros support

Message ID 20230612114359.220895-2-xiubli@redhat.com
State New
Headers show
Series ceph: print the client global id for debug logs | expand

Commit Message

Xiubo Li June 12, 2023, 11:43 a.m. UTC 
From: Xiubo Li <xiubli@redhat.com>

This will help print the client's global_id in debug logs.

URL: https://tracker.ceph.com/issues/61590
Cc: Patrick Donnelly <pdonnell@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 include/linux/ceph/ceph_debug.h | 38 ++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

Comments

Ilya Dryomov June 13, 2023, 8:39 a.m. UTC | #1 
On Mon, Jun 12, 2023 at 1:46 PM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> This will help print the client's global_id in debug logs.

Hi Xiubo,

There is a related concern that clients can belong to different
clusters, in which case their global IDs might clash.  If you chose
to disregard that as an unlikely scenario, it's probably fine, but
it would be nice to make that explicit in the commit message.

If account for that, the identifier block could look like:

  [<cluster fsid> <gid>]

instead of:

  [client.<gid>]

The "client." string prefix seems a bit redundant since the kernel
client's entity is always CEPH_ENTITY_TYPE_CLIENT.  If you like it
anyway, I would at least get rid of the dot at the end to align with
how it's presented elsewhere (e.g. debugfs directory name or
"ceph.client_id" xattr).

Thanks,

                Ilya
Xiubo Li June 13, 2023, 9:27 a.m. UTC | #2 
On 6/13/23 16:39, Ilya Dryomov wrote:
> On Mon, Jun 12, 2023 at 1:46 PM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> This will help print the client's global_id in debug logs.
> Hi Xiubo,
>
> There is a related concern that clients can belong to different
> clusters, in which case their global IDs might clash.  If you chose
> to disregard that as an unlikely scenario, it's probably fine, but
> it would be nice to make that explicit in the commit message.
>
> If account for that, the identifier block could look like:
>
>    [<cluster fsid> <gid>]

The fsid string is a little long:

[5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4236]

Maybe we could just print part of that as:

[5ea1e13c.. 4236]

?


>
> instead of:
>
>    [client.<gid>]
>
> The "client." string prefix seems a bit redundant since the kernel
> client's entity is always CEPH_ENTITY_TYPE_CLIENT.  If you like it
> anyway, I would at least get rid of the dot at the end to align with
> how it's presented elsewhere (e.g. debugfs directory name or
> "ceph.client_id" xattr).

Sure, I will remove it.

Thanks

- Xiubo


>
> Thanks,
>
>                  Ilya
>
Ilya Dryomov June 13, 2023, 10:08 a.m. UTC | #3 
On Tue, Jun 13, 2023 at 11:27 AM Xiubo Li <xiubli@redhat.com> wrote:
>
>
> On 6/13/23 16:39, Ilya Dryomov wrote:
> > On Mon, Jun 12, 2023 at 1:46 PM <xiubli@redhat.com> wrote:
> >> From: Xiubo Li <xiubli@redhat.com>
> >>
> >> This will help print the client's global_id in debug logs.
> > Hi Xiubo,
> >
> > There is a related concern that clients can belong to different
> > clusters, in which case their global IDs might clash.  If you chose
> > to disregard that as an unlikely scenario, it's probably fine, but
> > it would be nice to make that explicit in the commit message.
> >
> > If account for that, the identifier block could look like:
> >
> >    [<cluster fsid> <gid>]
>
> The fsid string is a little long:
>
> [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4236]
>
> Maybe we could just print part of that as:
>
> [5ea1e13c.. 4236]
>
> ?

If printing it at all, I would probably print the entire UUID.  But
I don't have a strong opinion here.

Thanks,

                Ilya
Xiubo Li June 13, 2023, 10:54 a.m. UTC | #4 
On 6/13/23 18:08, Ilya Dryomov wrote:
> On Tue, Jun 13, 2023 at 11:27 AM Xiubo Li <xiubli@redhat.com> wrote:
>>
>> On 6/13/23 16:39, Ilya Dryomov wrote:
>>> On Mon, Jun 12, 2023 at 1:46 PM <xiubli@redhat.com> wrote:
>>>> From: Xiubo Li <xiubli@redhat.com>
>>>>
>>>> This will help print the client's global_id in debug logs.
>>> Hi Xiubo,
>>>
>>> There is a related concern that clients can belong to different
>>> clusters, in which case their global IDs might clash.  If you chose
>>> to disregard that as an unlikely scenario, it's probably fine, but
>>> it would be nice to make that explicit in the commit message.
>>>
>>> If account for that, the identifier block could look like:
>>>
>>>     [<cluster fsid> <gid>]
>> The fsid string is a little long:
>>
>> [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4236]
>>
>> Maybe we could just print part of that as:
>>
>> [5ea1e13c.. 4236]
>>
>> ?
> If printing it at all, I would probably print the entire UUID.  But
> I don't have a strong opinion here.

I am okay with this, then it will be:

<7>[117633.216478] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_do_getattr inode 00000000f7600773 1.fffffffffffffffe mask As mode 
040755
<7>[117633.216486] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_caps_issued_mask mask ino 0x1 cap 00000000a1dd2c71 issued 
pAsLsXsFs (mask As)
<7>[117633.216493] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__touch_cap 00000000f7600773 cap 00000000a1dd2c71 mds0
<7>[117633.216501] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
ceph_d_revalidate 0000000013595462 'a.txt' inode 000000008738cf69 offset 
0xff5cc5890000002 nokey 0
<7>[117633.216509] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
dentry_lease_is_valid - dentry 0000000013595462 = 0
<7>[117633.216515] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_caps_issued_mask mask ino 0x1 cap 00000000a1dd2c71 issued 
pAsLsXsFs (mask Fs)
<7>[117633.216521] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__touch_cap 00000000f7600773 cap 00000000a1dd2c71 mds0
<7>[117633.216528] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_dentry_dir_lease_touch 0000000045691e1a 0000000013595462 'a.txt' 
(offset 0xff5cc5890000002)
<7>[117633.216535] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
dir_lease_is_valid dir 1.fffffffffffffffe v2 dentry 0000000013595462 
'a.txt' = 1
<7>[117633.216542] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
ceph_d_revalidate 0000000013595462 'a.txt' valid
<7>[117633.216551] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_do_getattr inode 000000008738cf69 10000000000.fffffffffffffffe 
mask As mode 0100644
<7>[117633.216558] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_caps_issued_mask mask ino 0x10000000000 cap 00000000610f13ac 
issued pAsLsXsFscr (mask As)
<7>[117633.216565] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__touch_cap 000000008738cf69 cap 00000000610f13ac mds0
<7>[117633.216572] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_caps_issued_mask mask ino 0x1 cap 00000000a1dd2c71 issued 
pAsLsXsFs (mask Fs)
<7>[117633.216599] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_do_getattr inode 00000000f7600773 1.fffffffffffffffe mask As mode 
040755
<7>[117633.216607] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_caps_issued_mask mask ino 0x1 cap 00000000a1dd2c71 issued 
pAsLsXsFs (mask As)
<7>[117633.216613] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__touch_cap 00000000f7600773 cap 00000000a1dd2c71 mds0
<7>[117633.216620] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
ceph_d_revalidate 0000000013595462 'a.txt' inode 000000008738cf69 offset 
0xff5cc5890000002 nokey 0
<7>[117633.216627] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
dentry_lease_is_valid - dentry 0000000013595462 = 0
<7>[117633.216633] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__ceph_caps_issued_mask mask ino 0x1 cap 00000000a1dd2c71 issued 
pAsLsXsFs (mask Fs)
<7>[117633.216639] ceph: [5ea1e13c-4034-426c-bf8f-8a3a70d9e812 4245] 
__touch_cap 00000000f7600773 cap 00000000a1dd2c71 mds0

Thanks

- Xiubo


> Thanks,
>
>                  Ilya
>
diff mbox series

Patch

diff --git a/include/linux/ceph/ceph_debug.h b/include/linux/ceph/ceph_debug.h
index d5a5da838caf..41bfbdb5dd85 100644
--- a/include/linux/ceph/ceph_debug.h
+++ b/include/linux/ceph/ceph_debug.h
@@ -19,12 +19,22 @@ 
 	pr_debug("%.*s %12.12s:%-4d : " fmt,				\
 		 8 - (int)sizeof(KBUILD_MODNAME), "    ",		\
 		 kbasename(__FILE__), __LINE__, ##__VA_ARGS__)
+#  define dout_client(client, fmt, ...)					\
+	pr_debug("%.*s %12.12s:%-4d : [client.%lld] " fmt,		\
+		 8 - (int)sizeof(KBUILD_MODNAME), "    ",		\
+		 kbasename(__FILE__), __LINE__,				\
+		 client->monc.auth->global_id,				\
+		 ##__VA_ARGS__)
 # else
 /* faux printk call just to see any compiler warnings. */
 #  define dout(fmt, ...)	do {				\
 		if (0)						\
 			printk(KERN_DEBUG fmt, ##__VA_ARGS__);	\
 	} while (0)
+#  define dout_client(client, fmt, ...)	do {			\
+		if (0)						\
+			printk(KERN_DEBUG fmt, ##__VA_ARGS__);	\
+	} while (0)
 # endif
 
 #else
@@ -33,7 +43,33 @@ 
  * or, just wrap pr_debug
  */
 # define dout(fmt, ...)	pr_debug(" " fmt, ##__VA_ARGS__)
-
+# define dout_client(client, fmt, ...)					 \
+	pr_debug(" [client.%lld] " fmt,	client->monc.auth->global_id,	 \
+		 ##__VA_ARGS__)
 #endif
 
+# define pr_notice_client(client, fmt, ...)				 \
+	pr_notice(" [client.%lld] " fmt, client->monc.auth->global_id,	 \
+		##__VA_ARGS__)
+# define pr_info_client(client, fmt, ...)				 \
+	pr_info(" [client.%lld] " fmt, client->monc.auth->global_id,	 \
+		##__VA_ARGS__)
+# define pr_warn_client(client, fmt, ...)				 \
+	pr_warn(" [client.%lld] " fmt, client->monc.auth->global_id,	 \
+		##__VA_ARGS__)
+# define pr_warn_once_client(client, fmt, ...)				 \
+	pr_warn_once(" [client.%lld] " fmt, client->monc.auth->global_id,\
+		##__VA_ARGS__)
+# define pr_err_client(client, fmt, ...)				 \
+	pr_err(" [client.%lld] " fmt, client->monc.auth->global_id,	 \
+		##__VA_ARGS__)
+# define pr_warn_ratelimited_client(client, fmt, ...)			 \
+	pr_warn_ratelimited(" [client.%lld] " fmt,			 \
+			    client->monc.auth->global_id,		 \
+			    ##__VA_ARGS__)
+# define pr_err_ratelimited_client(client, fmt, ...)			 \
+	pr_err_ratelimited(" [client.%lld] " fmt,			 \
+			    client->monc.auth->global_id,		 \
+			    ##__VA_ARGS__)
+
 #endif