[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=wh+bbO9nYxCz5CPf6oGB0upCFH4jx9Bx36f1f3+_DcDUQ@mail.gmail.com>
Date: Fri, 22 Nov 2024 14:30:10 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>,
Andrii Nakryiko <andrii@...nel.org>, Colin Ian King <colin.i.king@...il.com>,
Jeff Xie <jeff.xie@...ux.dev>, Jinjie Ruan <ruanjinjie@...wei.com>,
Josh Poimboeuf <jpoimboe@...nel.org>, Justin Stitt <justinstitt@...gle.com>,
Levi Yun <yeoreum.yun@....com>, Li Chen <chenl311@...natelecom.cn>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Ryan Roberts <ryan.roberts@....com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Tatsuya S <tatsuya.s2862@...il.com>,
Uros Bizjak <ubizjak@...il.com>, Zheng Yejian <zhengyejian@...weicloud.com>,
guoweikang <guoweikang.kernel@...il.com>
Subject: Re: [GIT PULL] tracing: Updates for v6.13
On Fri, 22 Nov 2024 at 14:12, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> Hmm, if we make a __DO_TRACE_SYSCALL(), I don't think it needs to even have
> that condition parameter.
That was my point. The whole conditional - and the parameter - seems
to be completely pointless as far as I can tell.
That said, I think you can actually simplify things even further: if
you move the TO_CONDITION() checking into the caller, you could move
the locking there too.
IOW, instead of this pattern:
if (static_branch_unlikely(&__tracepoint_##name.key)) \
__DO_TRACE(name, \
TP_ARGS(args), \
TP_CONDITION(cond), 0); \
you could make it be something like this instead:
if (static_branch_unlikely(&__tracepoint_##name.key)) \
if (TP_CONDITION(cond)) \
scoped_guard(preempt_notrace) \
__DO_TRACE(name, TP_ARGS(args)); \
where __DO_TRACE() would get neither the "cond" argument _nor_ that
locking argument, because both are just done by the two users (the
other one would use "scoped_guard(rcu_read_trace)" of course.
And look, this is another reason why unconditional locking is a good
thing: now you can use the "guard()" model for the lock, and don't
need an explicit unlock, simplifying the code more.
Of course, you want "guard(rcu_read_trace)" (for system call events)
and "guard(preempt_notrace)" (for the regular trace event case), and
we don't have the "notrace" versions of those guard classes yet.
But adding those would literally be trivial, ie something like
DEFINE_LOCK_GUARD_0(rcu_notrace,
rcu_read_lock_notrace(), rcu_read_unlock_notrace())
wouldn't that make it all look really nice?
NOTE NOTE NOTE! I didn't actually try any of the above in real life,
so I might be missing some important detail. I'm just pointing out
that making this all unconditional and not based on random flags has
the potential for even more cleanups.
And I might have gotten the different lock names confused too.
Linus
Powered by blists - more mailing lists