diff mbox series

[5.19] io_uring: move io_uring_get_opcode out of TP_printk

Message ID 20220623083743.2648321-1-dylany@fb.com (mailing list archive)
State New
Headers show
Series [5.19] io_uring: move io_uring_get_opcode out of TP_printk | expand

Commit Message

Dylan Yudaken June 23, 2022, 8:37 a.m. UTC 
The TP_printk macro's are not supposed to use custom code ([1]) or else
tools such as perf cannot use these events.

Convert the opcode string representation to use the __string wiring that
the event framework provides ([2]).

[1]: https://lwn.net/Articles/379903/
[2]: https://lwn.net/Articles/381064/

Fixes: 033b87d24f72 ("io_uring: use the text representation of ops in trace")
Signed-off-by: Dylan Yudaken <dylany@fb.com>
---

Hi,

I think this probably should get queued up for 5.19 as it can break some userspace
tooling (eg. perf) otherwise.

I've done the rebase here to 5.19, but there will be a conflict with Pavel's
"io_uring: clean up tracing events" if you apply this and rebase 5.20 on it.

Dylan

 include/trace/events/io_uring.h | 43 +++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 8 deletions(-)


base-commit: c0737fa9a5a5cf5a053bcc983f72d58919b997c6

Comments

Jens Axboe June 23, 2022, 2:36 p.m. UTC | #1 
On 6/23/22 2:37 AM, Dylan Yudaken wrote:
> @@ -390,6 +402,8 @@ TRACE_EVENT(io_uring_submit_sqe,
>  		__field(  u32,			flags		)
>  		__field(  bool,			force_nonblock	)
>  		__field(  bool,			sq_thread	)
> +
> +		__string( op_str, io_uring_get_opcode(opcode) )
>  	),
>  
>  	TP_fast_assign(
> @@ -399,12 +413,13 @@ TRACE_EVENT(io_uring_submit_sqe,
>  		__entry->opcode		= opcode;
>  		__entry->flags		= flags;
>  		__entry->force_nonblock	= force_nonblock;
> -		__entry->sq_thread	= sq_thread;
> +
> +		__assign_str(op_str, io_uring_get_opcode(opcode));
>  	),
>  

Looks like a spurious removal here of the sq_thread assignment? I will
fix it up.
Dylan Yudaken June 23, 2022, 2:59 p.m. UTC | #2 
On Thu, 2022-06-23 at 08:36 -0600, Jens Axboe wrote:
> On 6/23/22 2:37 AM, Dylan Yudaken wrote:
> > @@ -390,6 +402,8 @@ TRACE_EVENT(io_uring_submit_sqe,
> >                 __field(  u32,                  flags           )
> >                 __field(  bool,                 force_nonblock  )
> >                 __field(  bool,                 sq_thread       )
> > +
> > +               __string( op_str, io_uring_get_opcode(opcode) )
> >         ),
> >  
> >         TP_fast_assign(
> > @@ -399,12 +413,13 @@ TRACE_EVENT(io_uring_submit_sqe,
> >                 __entry->opcode         = opcode;
> >                 __entry->flags          = flags;
> >                 __entry->force_nonblock = force_nonblock;
> > -               __entry->sq_thread      = sq_thread;
> > +
> > +               __assign_str(op_str, io_uring_get_opcode(opcode));
> >         ),
> >  
> 
> Looks like a spurious removal here of the sq_thread assignment? I
> will
> fix it up.
> 

Ah damn. Good spot!

Thanks!
Jens Axboe June 25, 2022, 12:48 p.m. UTC | #3 
On Thu, 23 Jun 2022 01:37:43 -0700, Dylan Yudaken wrote:
> The TP_printk macro's are not supposed to use custom code ([1]) or else
> tools such as perf cannot use these events.
> 
> Convert the opcode string representation to use the __string wiring that
> the event framework provides ([2]).
> 
> [1]: https://lwn.net/Articles/379903/
> [2]: https://lwn.net/Articles/381064/
> 
> [...]

Applied, thanks!

[1/1] io_uring: move io_uring_get_opcode out of TP_printk
      commit: e70b64a3f28b9f54602ae3e706b1dc1338de3df7

Best regards,
diff mbox series

Patch

diff --git a/include/trace/events/io_uring.h b/include/trace/events/io_uring.h
index 66fcc5a1a5b1..ede64cde1704 100644
--- a/include/trace/events/io_uring.h
+++ b/include/trace/events/io_uring.h
@@ -158,6 +158,8 @@  TRACE_EVENT(io_uring_queue_async_work,
 		__field(  unsigned int,			flags		)
 		__field(  struct io_wq_work *,		work		)
 		__field(  int,				rw		)
+
+		__string( op_str, io_uring_get_opcode(opcode)	)
 	),
 
 	TP_fast_assign(
@@ -168,11 +170,13 @@  TRACE_EVENT(io_uring_queue_async_work,
 		__entry->opcode		= opcode;
 		__entry->work		= work;
 		__entry->rw		= rw;
+
+		__assign_str(op_str, io_uring_get_opcode(opcode));
 	),
 
 	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s, flags 0x%x, %s queue, work %p",
 		__entry->ctx, __entry->req, __entry->user_data,
-		io_uring_get_opcode(__entry->opcode),
+		__get_str(op_str),
 		__entry->flags, __entry->rw ? "hashed" : "normal", __entry->work)
 );
 
@@ -198,6 +202,8 @@  TRACE_EVENT(io_uring_defer,
 		__field(  void *,		req	)
 		__field(  unsigned long long,	data	)
 		__field(  u8,			opcode	)
+
+		__string( op_str, io_uring_get_opcode(opcode) )
 	),
 
 	TP_fast_assign(
@@ -205,11 +211,13 @@  TRACE_EVENT(io_uring_defer,
 		__entry->req	= req;
 		__entry->data	= user_data;
 		__entry->opcode	= opcode;
+
+		__assign_str(op_str, io_uring_get_opcode(opcode));
 	),
 
 	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s",
 		__entry->ctx, __entry->req, __entry->data,
-		io_uring_get_opcode(__entry->opcode))
+		__get_str(op_str))
 );
 
 /**
@@ -298,6 +306,8 @@  TRACE_EVENT(io_uring_fail_link,
 		__field(  unsigned long long,	user_data	)
 		__field(  u8,			opcode		)
 		__field(  void *,		link		)
+
+		__string( op_str, io_uring_get_opcode(opcode) )
 	),
 
 	TP_fast_assign(
@@ -306,11 +316,13 @@  TRACE_EVENT(io_uring_fail_link,
 		__entry->user_data	= user_data;
 		__entry->opcode		= opcode;
 		__entry->link		= link;
+
+		__assign_str(op_str, io_uring_get_opcode(opcode));
 	),
 
 	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s, link %p",
 		__entry->ctx, __entry->req, __entry->user_data,
-		io_uring_get_opcode(__entry->opcode), __entry->link)
+		__get_str(op_str), __entry->link)
 );
 
 /**
@@ -390,6 +402,8 @@  TRACE_EVENT(io_uring_submit_sqe,
 		__field(  u32,			flags		)
 		__field(  bool,			force_nonblock	)
 		__field(  bool,			sq_thread	)
+
+		__string( op_str, io_uring_get_opcode(opcode) )
 	),
 
 	TP_fast_assign(
@@ -399,12 +413,13 @@  TRACE_EVENT(io_uring_submit_sqe,
 		__entry->opcode		= opcode;
 		__entry->flags		= flags;
 		__entry->force_nonblock	= force_nonblock;
-		__entry->sq_thread	= sq_thread;
+
+		__assign_str(op_str, io_uring_get_opcode(opcode));
 	),
 
 	TP_printk("ring %p, req %p, user_data 0x%llx, opcode %s, flags 0x%x, "
 		  "non block %d, sq_thread %d", __entry->ctx, __entry->req,
-		  __entry->user_data, io_uring_get_opcode(__entry->opcode),
+		  __entry->user_data, __get_str(op_str),
 		  __entry->flags, __entry->force_nonblock, __entry->sq_thread)
 );
 
@@ -435,6 +450,8 @@  TRACE_EVENT(io_uring_poll_arm,
 		__field(  u8,			opcode		)
 		__field(  int,			mask		)
 		__field(  int,			events		)
+
+		__string( op_str, io_uring_get_opcode(opcode) )
 	),
 
 	TP_fast_assign(
@@ -444,11 +461,13 @@  TRACE_EVENT(io_uring_poll_arm,
 		__entry->opcode		= opcode;
 		__entry->mask		= mask;
 		__entry->events		= events;
+
+		__assign_str(op_str, io_uring_get_opcode(opcode));
 	),
 
 	TP_printk("ring %p, req %p, user_data 0x%llx, opcode %s, mask 0x%x, events 0x%x",
 		  __entry->ctx, __entry->req, __entry->user_data,
-		  io_uring_get_opcode(__entry->opcode),
+		  __get_str(op_str),
 		  __entry->mask, __entry->events)
 );
 
@@ -474,6 +493,8 @@  TRACE_EVENT(io_uring_task_add,
 		__field(  unsigned long long,	user_data	)
 		__field(  u8,			opcode		)
 		__field(  int,			mask		)
+
+		__string( op_str, io_uring_get_opcode(opcode) )
 	),
 
 	TP_fast_assign(
@@ -482,11 +503,13 @@  TRACE_EVENT(io_uring_task_add,
 		__entry->user_data	= user_data;
 		__entry->opcode		= opcode;
 		__entry->mask		= mask;
+
+		__assign_str(op_str, io_uring_get_opcode(opcode));
 	),
 
 	TP_printk("ring %p, req %p, user_data 0x%llx, opcode %s, mask %x",
 		__entry->ctx, __entry->req, __entry->user_data,
-		io_uring_get_opcode(__entry->opcode),
+		__get_str(op_str),
 		__entry->mask)
 );
 
@@ -523,6 +546,8 @@  TRACE_EVENT(io_uring_req_failed,
 		__field( u64,			pad1		)
 		__field( u64,			addr3		)
 		__field( int,			error		)
+
+		__string( op_str, io_uring_get_opcode(sqe->opcode) )
 	),
 
 	TP_fast_assign(
@@ -542,6 +567,8 @@  TRACE_EVENT(io_uring_req_failed,
 		__entry->pad1		= sqe->__pad2[0];
 		__entry->addr3		= sqe->addr3;
 		__entry->error		= error;
+
+		__assign_str(op_str, io_uring_get_opcode(sqe->opcode));
 	),
 
 	TP_printk("ring %p, req %p, user_data 0x%llx, "
@@ -550,7 +577,7 @@  TRACE_EVENT(io_uring_req_failed,
 		  "personality=%d, file_index=%d, pad=0x%llx, addr3=%llx, "
 		  "error=%d",
 		  __entry->ctx, __entry->req, __entry->user_data,
-		  io_uring_get_opcode(__entry->opcode),
+		  __get_str(op_str),
 		  __entry->flags, __entry->ioprio,
 		  (unsigned long long)__entry->off,
 		  (unsigned long long) __entry->addr, __entry->len,