All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: linux-kernel@vger.kernel.org, Phil Auld <pauld@redhat.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Kate Carcia <kcarcia@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alexandre Chartre <alexandre.chartre@oracle.com>,
	Clark Willaims <williams@redhat.com>,
	John Kacur <jkacur@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH V3 7/9] tracing: Add __print_ns_to_secs() and __print_ns_without_secs() helpers
Date: Thu, 03 Jun 2021 21:19:50 -0700	[thread overview]
Message-ID: <1e068d21106bb6db05b735b4916bb420e6c9842a.camel@perches.com> (raw)
In-Reply-To: <20210603172902.41648183@gandalf.local.home>

On Thu, 2021-06-03 at 17:29 -0400, Steven Rostedt wrote:
> > +++ b/include/trace/trace_events.h
> > @@ -358,6 +358,21 @@ TRACE_MAKE_SYSTEM_STR();
> >  	trace_print_hex_dump_seq(p, prefix_str, prefix_type,		\
> >  				 rowsize, groupsize, buf, len, ascii)
> >  
> > 
> > +#undef __print_ns_to_secs
> > +#define __print_ns_to_secs(value)			\
> > +	({						\
> > +		u64 ____val = (u64)value;		\
> > +		do_div(____val, NSEC_PER_SEC);		\
> > +		____val;				\
> > +	})
> 
> I know my name is on this, but we need parenthesis around "value".

If tracing cleanups for trace_events.h are being done, perhaps
another bit of untidiness is the macro definition and uses of
__assign_str.

$ git grep -w -1 __assign_str include/trace/trace_events.h
include/trace/trace_events.h-
include/trace/trace_events.h:#undef __assign_str
include/trace/trace_events.h:#define __assign_str(dst, src)                                             \
include/trace/trace_events.h-   strcpy(__get_str(dst), (src) ? (const char *)(src) : "(null)");

Its definition has a semicolon as do most uses but a dozen handfuls of
other uses do not have a semicolon.  It'd be more consistent to add a
semicolon to the uses without them and when done treewide, then remove
the semicolon from the macro declaration.

$ git grep -P '\b__assign_str\b' | wc -l
551
$ git grep -P '\b__assign_str\b.*;' | wc -l
480

For instance: sunrpc.h has a mixture of uses with and without semicolons.

$ git grep -P '\b__assign_str\b' include/trace/events/sunrpc.h | wc -l
65
$ git grep -P '\b__assign_str\b.*;' include/trace/events/sunrpc.h | wc -l
38

It's especially odd in the last block below to have 3 successive uses
where the first and last has a semicolon, but the middle one does not.

So perhaps as a start:
---
 include/trace/events/sunrpc.h | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index d02e01a27b690..861f199896c6a 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -154,8 +154,8 @@ TRACE_EVENT(rpc_clnt_new,
 		__entry->client_id = clnt->cl_clid;
 		__assign_str(addr, xprt->address_strings[RPC_DISPLAY_ADDR]);
 		__assign_str(port, xprt->address_strings[RPC_DISPLAY_PORT]);
-		__assign_str(program, program)
-		__assign_str(server, server)
+		__assign_str(program, program);
+		__assign_str(server, server);
 	),
 
 	TP_printk("client=%u peer=[%s]:%s program=%s server=%s",
@@ -180,8 +180,8 @@ TRACE_EVENT(rpc_clnt_new_err,
 
 	TP_fast_assign(
 		__entry->error = error;
-		__assign_str(program, program)
-		__assign_str(server, server)
+		__assign_str(program, program);
+		__assign_str(server, server);
 	),
 
 	TP_printk("program=%s server=%s error=%d",
@@ -284,8 +284,8 @@ TRACE_EVENT(rpc_request,
 		__entry->client_id = task->tk_client->cl_clid;
 		__entry->version = task->tk_client->cl_vers;
 		__entry->async = RPC_IS_ASYNC(task);
-		__assign_str(progname, task->tk_client->cl_program->name)
-		__assign_str(procname, rpc_proc_name(task))
+		__assign_str(progname, task->tk_client->cl_program->name);
+		__assign_str(procname, rpc_proc_name(task));
 	),
 
 	TP_printk("task:%u@%u %sv%d %s (%ssync)",
@@ -494,10 +494,10 @@ DECLARE_EVENT_CLASS(rpc_reply_event,
 		__entry->task_id = task->tk_pid;
 		__entry->client_id = task->tk_client->cl_clid;
 		__entry->xid = be32_to_cpu(task->tk_rqstp->rq_xid);
-		__assign_str(progname, task->tk_client->cl_program->name)
+		__assign_str(progname, task->tk_client->cl_program->name);
 		__entry->version = task->tk_client->cl_vers;
-		__assign_str(procname, rpc_proc_name(task))
-		__assign_str(servername, task->tk_xprt->servername)
+		__assign_str(procname, rpc_proc_name(task));
+		__assign_str(servername, task->tk_xprt->servername);
 	),
 
 	TP_printk("task:%u@%d server=%s xid=0x%08x %sv%d %s",
@@ -622,8 +622,8 @@ TRACE_EVENT(rpc_stats_latency,
 		__entry->task_id = task->tk_pid;
 		__entry->xid = be32_to_cpu(task->tk_rqstp->rq_xid);
 		__entry->version = task->tk_client->cl_vers;
-		__assign_str(progname, task->tk_client->cl_program->name)
-		__assign_str(procname, rpc_proc_name(task))
+		__assign_str(progname, task->tk_client->cl_program->name);
+		__assign_str(procname, rpc_proc_name(task));
 		__entry->backlog = ktime_to_us(backlog);
 		__entry->rtt = ktime_to_us(rtt);
 		__entry->execute = ktime_to_us(execute);
@@ -669,15 +669,15 @@ TRACE_EVENT(rpc_xdr_overflow,
 			__entry->task_id = task->tk_pid;
 			__entry->client_id = task->tk_client->cl_clid;
 			__assign_str(progname,
-				     task->tk_client->cl_program->name)
+				     task->tk_client->cl_program->name);
 			__entry->version = task->tk_client->cl_vers;
-			__assign_str(procedure, task->tk_msg.rpc_proc->p_name)
+			__assign_str(procedure, task->tk_msg.rpc_proc->p_name);
 		} else {
 			__entry->task_id = 0;
 			__entry->client_id = 0;
-			__assign_str(progname, "unknown")
+			__assign_str(progname, "unknown");
 			__entry->version = 0;
-			__assign_str(procedure, "unknown")
+			__assign_str(procedure, "unknown");
 		}
 		__entry->requested = requested;
 		__entry->end = xdr->end;
@@ -735,9 +735,9 @@ TRACE_EVENT(rpc_xdr_alignment,
 		__entry->task_id = task->tk_pid;
 		__entry->client_id = task->tk_client->cl_clid;
 		__assign_str(progname,
-			     task->tk_client->cl_program->name)
+			     task->tk_client->cl_program->name);
 		__entry->version = task->tk_client->cl_vers;
-		__assign_str(procedure, task->tk_msg.rpc_proc->p_name)
+		__assign_str(procedure, task->tk_msg.rpc_proc->p_name);
 
 		__entry->offset = offset;
 		__entry->copied = copied;
@@ -1107,9 +1107,9 @@ TRACE_EVENT(xprt_retransmit,
 		__entry->xid = be32_to_cpu(rqst->rq_xid);
 		__entry->ntrans = rqst->rq_ntrans;
 		__assign_str(progname,
-			     task->tk_client->cl_program->name)
+			     task->tk_client->cl_program->name);
 		__entry->version = task->tk_client->cl_vers;
-		__assign_str(procedure, task->tk_msg.rpc_proc->p_name)
+		__assign_str(procedure, task->tk_msg.rpc_proc->p_name);
 	),
 
 	TP_printk(
@@ -1842,7 +1842,7 @@ TRACE_EVENT(svc_xprt_accept,
 
 	TP_fast_assign(
 		__assign_str(addr, xprt->xpt_remotebuf);
-		__assign_str(protocol, xprt->xpt_class->xcl_name)
+		__assign_str(protocol, xprt->xpt_class->xcl_name);
 		__assign_str(service, service);
 	),
 


  reply	other threads:[~2021-06-04  4:19 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14 20:51 [PATCH V3 0/9] hwlat improvements and osnoise/timerlat tracers Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 1/9] tracing/hwlat: Fix Clark's email Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 2/9] tracing/hwlat: Implement the mode config option Daniel Bristot de Oliveira
2021-06-03 20:11   ` Steven Rostedt
2021-05-14 20:51 ` [PATCH V3 3/9] tracing/hwlat: Switch disable_migrate to mode none Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 4/9] tracing/hwlat: Implement the per-cpu mode Daniel Bristot de Oliveira
2021-05-27 11:58   ` Juri Lelli
2021-05-27 12:29     ` Daniel Bristot de Oliveira
2021-06-03 21:17   ` Steven Rostedt
2021-06-04 15:31     ` Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 5/9] tracing/trace: Add a generic function to read/write u64 values from tracefs Daniel Bristot de Oliveira
2021-06-03 21:22   ` Steven Rostedt
2021-06-04 16:05     ` Daniel Bristot de Oliveira
2021-06-04 16:18       ` Steven Rostedt
2021-06-04 16:34         ` Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 6/9] trace/hwlat: Use the generic function to read/write width and window Daniel Bristot de Oliveira
2021-06-03 21:27   ` Steven Rostedt
2021-06-04 16:36     ` Daniel Bristot de Oliveira
2021-06-04 20:50       ` Steven Rostedt
2021-05-14 20:51 ` [PATCH V3 7/9] tracing: Add __print_ns_to_secs() and __print_ns_without_secs() helpers Daniel Bristot de Oliveira
2021-06-03 21:29   ` Steven Rostedt
2021-06-04  4:19     ` Joe Perches [this message]
2021-06-04 16:21       ` Steven Rostedt
2021-06-04 19:09         ` [PATCH] treewide: Add missing semicolons to __assign_str uses Joe Perches
2021-06-04 19:09           ` Joe Perches
2021-06-04 19:09           ` Joe Perches
2021-06-04 19:38         ` Joe Perches
2021-06-04 19:38           ` Joe Perches
2021-06-04 19:38           ` Joe Perches
2021-06-07 23:18           ` Jason Gunthorpe
2021-06-07 23:18             ` Jason Gunthorpe
2021-06-07 23:18             ` Jason Gunthorpe
2021-06-12 15:42         ` [PATCH V2] " Joe Perches
2021-06-12 15:42           ` Joe Perches
2021-06-12 15:42           ` Joe Perches
2021-06-12 23:11           ` Steven Rostedt
2021-06-12 23:11             ` Steven Rostedt
2021-06-12 23:11             ` Steven Rostedt
2021-06-30 11:28           ` Joe Perches
2021-06-30 11:28             ` Joe Perches
2021-06-30 11:28             ` Joe Perches
2021-06-30 12:22             ` Steven Rostedt
2021-06-30 12:22               ` Steven Rostedt
2021-06-30 12:22               ` Steven Rostedt
2021-06-04 16:07     ` [PATCH V3 7/9] tracing: Add __print_ns_to_secs() and __print_ns_without_secs() helpers Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 8/9] tracing: Add osnoise tracer Daniel Bristot de Oliveira
2021-06-03 21:31   ` Steven Rostedt
2021-06-04 21:28   ` Steven Rostedt
2021-06-07 12:00     ` Daniel Bristot de Oliveira
2021-06-07 15:47       ` Steven Rostedt
2021-06-08 15:24         ` Daniel Bristot de Oliveira
2021-06-08 17:17     ` Daniel Bristot de Oliveira
2021-06-08 17:39       ` Steven Rostedt
2021-06-08 19:33         ` Daniel Bristot de Oliveira
2021-06-08 19:42           ` Steven Rostedt
2021-06-09 12:14     ` Daniel Bristot de Oliveira
2021-06-09 13:03       ` Steven Rostedt
2021-06-09 13:44         ` Daniel Bristot de Oliveira
2021-05-14 20:51 ` [PATCH V3 9/9] tracing: Add timerlat tracer Daniel Bristot de Oliveira
2021-06-08  1:36   ` Steven Rostedt
2021-06-11 12:59     ` Daniel Bristot de Oliveira
2021-06-11 20:03       ` Steven Rostedt
2021-06-12  9:41         ` Daniel Bristot de Oliveira
2021-06-12 23:06           ` Steven Rostedt
2021-06-11 14:13     ` Daniel Bristot de Oliveira
2021-06-11 20:48       ` Steven Rostedt
2021-06-12  8:47         ` Daniel Bristot de Oliveira
2021-06-12 23:09           ` Steven Rostedt
2021-06-15  8:18             ` Daniel Bristot de Oliveira
2021-05-27 12:07 ` [PATCH V3 0/9] hwlat improvements and osnoise/timerlat tracers Juri Lelli
2021-05-29  2:16   ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1e068d21106bb6db05b735b4916bb420e6c9842a.camel@perches.com \
    --to=joe@perches.com \
    --cc=alexandre.chartre@oracle.com \
    --cc=bigeasy@linutronix.de \
    --cc=bristot@redhat.com \
    --cc=corbet@lwn.net \
    --cc=jkacur@redhat.com \
    --cc=juri.lelli@redhat.com \
    --cc=kcarcia@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.