[PATCH 03/15] io_uring: Use trace_invoke_##name() at guarded tracepoint call sites

Vineeth Pillai (Google) posted 15 patches 3 weeks, 4 days ago
There is a newer version of this series
[PATCH 03/15] io_uring: Use trace_invoke_##name() at guarded tracepoint call sites
Posted by Vineeth Pillai (Google) 3 weeks, 4 days ago
Replace trace_foo() with the new trace_invoke_foo() at sites already
guarded by trace_foo_enabled(), avoiding a redundant
static_branch_unlikely() re-evaluation inside the tracepoint.
trace_invoke_foo() calls the tracepoint callbacks directly without
utilizing the static branch again.

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Vineeth Pillai (Google) <vineeth@bitbyteword.org>
Assisted-by: Claude:claude-sonnet-4-6
---
 io_uring/io_uring.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 0fa844faf2871..68b7656e1547a 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -299,7 +299,7 @@ static __always_inline bool io_fill_cqe_req(struct io_ring_ctx *ctx,
 	}
 
 	if (trace_io_uring_complete_enabled())
-		trace_io_uring_complete(req->ctx, req, cqe);
+		trace_invoke_io_uring_complete(req->ctx, req, cqe);
 	return true;
 }
 
-- 
2.53.0
Re: [PATCH 03/15] io_uring: Use trace_invoke_##name() at guarded tracepoint call sites
Posted by Keith Busch 3 weeks, 4 days ago
On Thu, Mar 12, 2026 at 11:04:58AM -0400, Vineeth Pillai (Google) wrote:
>  	if (trace_io_uring_complete_enabled())
> -		trace_io_uring_complete(req->ctx, req, cqe);
> +		trace_invoke_io_uring_complete(req->ctx, req, cqe);

Curious, this one doesn't follow that pattern of "if (enabed && cond)"
that this cover letter said it was addressing, so why doesn't this call
just drop the 'if' check and go straight to trace_io_uring_complete()? I
followed this usage to commit a0730c738309a06, which says that the
compiler was generating code to move args before checking if the trace
was enabled. That commit was a while ago though, and suggests to remove
the check if that problem is solved. Is it still a problem?
Re: [PATCH 03/15] io_uring: Use trace_invoke_##name() at guarded tracepoint call sites
Posted by Steven Rostedt 3 weeks, 4 days ago
On Thu, 12 Mar 2026 09:24:21 -0600
Keith Busch <kbusch@kernel.org> wrote:

> On Thu, Mar 12, 2026 at 11:04:58AM -0400, Vineeth Pillai (Google) wrote:
> >  	if (trace_io_uring_complete_enabled())
> > -		trace_io_uring_complete(req->ctx, req, cqe);
> > +		trace_invoke_io_uring_complete(req->ctx, req, cqe);  
> 
> Curious, this one doesn't follow that pattern of "if (enabed && cond)"
> that this cover letter said it was addressing, so why doesn't this call
> just drop the 'if' check and go straight to trace_io_uring_complete()? I
> followed this usage to commit a0730c738309a06, which says that the

You mean 'a0727c738309a06'? As I could not find the above 'a0730c738309a06'

> compiler was generating code to move args before checking if the trace
> was enabled. That commit was a while ago though, and suggests to remove

It was only 2023.

> the check if that problem is solved. Is it still a problem?

We should check. But also, tracepoints should never be in a header file.
That really should be:

#include <linux/tracepoint-defs.h>

DECLARE_TRACEPOINT(io_uring_complete);

[..]

	if (tracepoint_enabled(io_uring_complete))
		do_trace_io_uring_complete(...);

And in a C file, that should be:

void do_io_uring_complete(...)
{
	trace_inovke_io_uring_complete(...);
}


Which reminds me. There's other places that have that tracepoint_enabled()
in header files that do the above. The C wrapper functions should also
convert the callback to the trace_invoke_<event>() call.

-- Steve
Re: [PATCH 03/15] io_uring: Use trace_invoke_##name() at guarded tracepoint call sites
Posted by Vineeth Remanan Pillai 2 weeks, 5 days ago
On Thu, Mar 12, 2026 at 11:38 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 12 Mar 2026 09:24:21 -0600
> Keith Busch <kbusch@kernel.org> wrote:
>
> > On Thu, Mar 12, 2026 at 11:04:58AM -0400, Vineeth Pillai (Google) wrote:
> > >     if (trace_io_uring_complete_enabled())
> > > -           trace_io_uring_complete(req->ctx, req, cqe);
> > > +           trace_invoke_io_uring_complete(req->ctx, req, cqe);
> >
> > Curious, this one doesn't follow that pattern of "if (enabed && cond)"
> > that this cover letter said it was addressing, so why doesn't this call
> > just drop the 'if' check and go straight to trace_io_uring_complete()? I
> > followed this usage to commit a0730c738309a06, which says that the
>
> You mean 'a0727c738309a06'? As I could not find the above 'a0730c738309a06'
>
> > compiler was generating code to move args before checking if the trace
> > was enabled. That commit was a while ago though, and suggests to remove
>
> It was only 2023.
>
> > the check if that problem is solved. Is it still a problem?
>
> We should check.

I shall leave this patch as is for now.

> Which reminds me. There's other places that have that tracepoint_enabled()
> in header files that do the above. The C wrapper functions should also
> convert the callback to the trace_invoke_<event>() call.
>

Thanks for pointing this out. I just had a look and its not too much.
But I feel it would be better to take it up as a new series. What do
you think?

Thanks,
Vineeth
> -- Steve