On Thu, 12 Mar 2026 09:24:21 -0600
Keith Busch <[email protected]> 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
