On Thu, Jun 15, 2023 at 3:49 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > On 6/14/23 16:21, Ilya Dryomov wrote: > > On Wed, Jun 14, 2023 at 3:33 AM <xiubli@xxxxxxxxxx> wrote: > >> From: Xiubo Li <xiubli@xxxxxxxxxx> > >> > >> This will help print the fsid and client's global_id in debug logs, > >> and also print the function names. > >> > >> URL: https://tracker.ceph.com/issues/61590 > >> Cc: Patrick Donnelly <pdonnell@xxxxxxxxxx> > >> Reviewed-by: Patrick Donnelly <pdonnell@xxxxxxxxxx> > >> Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > >> --- > >> include/linux/ceph/ceph_debug.h | 44 ++++++++++++++++++++++++++++++++- > >> 1 file changed, 43 insertions(+), 1 deletion(-) > >> > >> diff --git a/include/linux/ceph/ceph_debug.h b/include/linux/ceph/ceph_debug.h > >> index d5a5da838caf..26b9212bf359 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 : [%pU %lld] " fmt, \ > >> + 8 - (int)sizeof(KBUILD_MODNAME), " ", \ > >> + kbasename(__FILE__), __LINE__, \ > >> + &client->fsid, 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,39 @@ > >> * or, just wrap pr_debug > >> */ > >> # define dout(fmt, ...) pr_debug(" " fmt, ##__VA_ARGS__) > >> - > >> +# define dout_client(client, fmt, ...) \ > >> + pr_debug("[%pU %lld] %s: " fmt, &client->fsid, \ > >> + client->monc.auth->global_id, __func__, \ > >> + ##__VA_ARGS__) > >> #endif > >> > >> +# define pr_notice_client(client, fmt, ...) \ > >> + pr_notice("[%pU %lld] %s: " fmt, &client->fsid, \ > >> + client->monc.auth->global_id, __func__, \ > >> + ##__VA_ARGS__) > > Hi Xiubo, > > > > We definitely don't want the framework to include function names in > > user-facing messages (i.e. in pr_* messages). In the example that > > spawned this series ("ceph: mds3 session blocklisted"), it's really > > irrelevant to the user which function happens to detect blocklisting. > > > > It's a bit less clear-cut for dout() messages, but honestly I don't > > think it's needed there either. I know that we include it manually in > > many places but most of the time it's actually redundant. > > The function name will include the most info needed in the log messages, > before almost all the log messages will add that explicitly or will add > one function name directly. Which may make the length of the 'fmt' > string exceeded 80 chars. > > If this doesn't make sense I will remove this from the framework. I'm fine with keeping it for dout() messages. To further help with line lengths, how about naming the new macro doutc()? Since it takes an extra argument before the format string, it feels distinctive enough despite it being just a single character. Thanks, Ilya
