Re: [RFC PATCH 4/5] tracing: Remove conditional locking from __DO_TRACE()

[Mathieu Desnoyers]

On 2024-11-23 12:38, Linus Torvalds wrote:
> On Sat, 23 Nov 2024 at 07:31, Mathieu Desnoyers
> <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> >   include/linux/tracepoint.h | 45 ++++++++++----------------------------
> >   1 file changed, 12 insertions(+), 33 deletions(-)
>
> Thanks. This looks much more straightforward, and obviously is smaller too.
>
> Side note: I realize I was the one suggesting "scoped_guard()", but
> looking at the patch I do think that just unnecessarily added another
> level of indentation. Since you already wrote the
>
>      if (cond) {
>          ..
>      }
>
> part as a block statement, there's no upside to the guard having its
> own scoped block, so instead of
>
>      if (cond) { \
>          scoped_guard(preempt_notrace)           \
>              __DO_TRACE_CALL(name, TP_ARGS(args)); \
>      }
>
> this might be simpler as just a plain "guard()" and one less indentation:
>
>      if (cond) { \
>          guard(preempt_notrace);           \
>          __DO_TRACE_CALL(name, TP_ARGS(args)); \
>      }
>
> but by now this is just an unimportant detail.
>
> I think I suggested scoped_guard() mainly because that would then just
> make the "{ }" in the if-statement superfluous, but that's such a
> random reason that it *really* doesn't matter.

Thanks for the follow up. I agree that guard() would remove one level
of nesting and would be an improvement.

Steven, do you want me to update the series with this change or
should I leave the scoped_guard() as is considering the ongoing
testing in linux-next ? We can always keep this minor change
(scoped_guard -> guard) for a follow up patch.

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com