From: Masami Hiramatsu <mhiramat@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
lkml <linux-kernel@vger.kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Ravi Bangoria <ravi.bangoria@amd.com>,
Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] tracing: Iterate trace_[ku]probe objects directly
Date: Fri, 26 Nov 2021 21:40:55 +0900 [thread overview]
Message-ID: <20211126214055.b6a84e8add8f7ae3deb9411b@kernel.org> (raw)
In-Reply-To: <20211125202852.406405-1-jolsa@kernel.org>
On Thu, 25 Nov 2021 21:28:52 +0100
Jiri Olsa <jolsa@redhat.com> wrote:
> As suggested by Linus [1] using list_for_each_entry to iterate
> directly trace_[ku]probe objects so we can skip another call to
> container_of in these loops.
>
This looks very good to me :)
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
> [1] https://lore.kernel.org/r/CAHk-=wjakjw6-rDzDDBsuMoDCqd+9ogifR_EE1F0K-jYek1CdA@mail.gmail.com
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> kernel/trace/trace_kprobe.c | 13 ++++---------
> kernel/trace/trace_uprobe.c | 23 ++++++++---------------
> 2 files changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 33272a7b6912..1cddb42af20c 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -327,11 +327,9 @@ static inline int __enable_trace_kprobe(struct trace_kprobe *tk)
>
> static void __disable_trace_kprobe(struct trace_probe *tp)
> {
> - struct trace_probe *pos;
> struct trace_kprobe *tk;
>
> - list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> - tk = container_of(pos, struct trace_kprobe, tp);
> + list_for_each_entry(tk, trace_probe_probe_list(tp), tp.list) {
> if (!trace_kprobe_is_registered(tk))
> continue;
> if (trace_kprobe_is_return(tk))
> @@ -348,7 +346,7 @@ static void __disable_trace_kprobe(struct trace_probe *tp)
> static int enable_trace_kprobe(struct trace_event_call *call,
> struct trace_event_file *file)
> {
> - struct trace_probe *pos, *tp;
> + struct trace_probe *tp;
> struct trace_kprobe *tk;
> bool enabled;
> int ret = 0;
> @@ -369,8 +367,7 @@ static int enable_trace_kprobe(struct trace_event_call *call,
> if (enabled)
> return 0;
>
> - list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> - tk = container_of(pos, struct trace_kprobe, tp);
> + list_for_each_entry(tk, trace_probe_probe_list(tp), tp.list) {
> if (trace_kprobe_has_gone(tk))
> continue;
> ret = __enable_trace_kprobe(tk);
> @@ -559,11 +556,9 @@ static bool trace_kprobe_has_same_kprobe(struct trace_kprobe *orig,
> struct trace_kprobe *comp)
> {
> struct trace_probe_event *tpe = orig->tp.event;
> - struct trace_probe *pos;
> int i;
>
> - list_for_each_entry(pos, &tpe->probes, list) {
> - orig = container_of(pos, struct trace_kprobe, tp);
> + list_for_each_entry(orig, &tpe->probes, tp.list) {
> if (strcmp(trace_kprobe_symbol(orig),
> trace_kprobe_symbol(comp)) ||
> trace_kprobe_offset(orig) != trace_kprobe_offset(comp))
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index f5f0039d31e5..a9a294e6b183 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -409,12 +409,10 @@ static bool trace_uprobe_has_same_uprobe(struct trace_uprobe *orig,
> struct trace_uprobe *comp)
> {
> struct trace_probe_event *tpe = orig->tp.event;
> - struct trace_probe *pos;
> struct inode *comp_inode = d_real_inode(comp->path.dentry);
> int i;
>
> - list_for_each_entry(pos, &tpe->probes, list) {
> - orig = container_of(pos, struct trace_uprobe, tp);
> + list_for_each_entry(orig, &tpe->probes, tp.list) {
> if (comp_inode != d_real_inode(orig->path.dentry) ||
> comp->offset != orig->offset)
> continue;
> @@ -1075,14 +1073,12 @@ static int trace_uprobe_enable(struct trace_uprobe *tu, filter_func_t filter)
>
> static void __probe_event_disable(struct trace_probe *tp)
> {
> - struct trace_probe *pos;
> struct trace_uprobe *tu;
>
> tu = container_of(tp, struct trace_uprobe, tp);
> WARN_ON(!uprobe_filter_is_empty(tu->tp.event->filter));
>
> - list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> - tu = container_of(pos, struct trace_uprobe, tp);
> + list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) {
> if (!tu->inode)
> continue;
>
> @@ -1094,7 +1090,7 @@ static void __probe_event_disable(struct trace_probe *tp)
> static int probe_event_enable(struct trace_event_call *call,
> struct trace_event_file *file, filter_func_t filter)
> {
> - struct trace_probe *pos, *tp;
> + struct trace_probe *tp;
> struct trace_uprobe *tu;
> bool enabled;
> int ret;
> @@ -1129,8 +1125,7 @@ static int probe_event_enable(struct trace_event_call *call,
> if (ret)
> goto err_flags;
>
> - list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> - tu = container_of(pos, struct trace_uprobe, tp);
> + list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) {
> ret = trace_uprobe_enable(tu, filter);
> if (ret) {
> __probe_event_disable(tp);
> @@ -1275,7 +1270,7 @@ static bool trace_uprobe_filter_add(struct trace_uprobe_filter *filter,
> static int uprobe_perf_close(struct trace_event_call *call,
> struct perf_event *event)
> {
> - struct trace_probe *pos, *tp;
> + struct trace_probe *tp;
> struct trace_uprobe *tu;
> int ret = 0;
>
> @@ -1287,8 +1282,7 @@ static int uprobe_perf_close(struct trace_event_call *call,
> if (trace_uprobe_filter_remove(tu->tp.event->filter, event))
> return 0;
>
> - list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> - tu = container_of(pos, struct trace_uprobe, tp);
> + list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) {
> ret = uprobe_apply(tu->inode, tu->offset, &tu->consumer, false);
> if (ret)
> break;
> @@ -1300,7 +1294,7 @@ static int uprobe_perf_close(struct trace_event_call *call,
> static int uprobe_perf_open(struct trace_event_call *call,
> struct perf_event *event)
> {
> - struct trace_probe *pos, *tp;
> + struct trace_probe *tp;
> struct trace_uprobe *tu;
> int err = 0;
>
> @@ -1312,8 +1306,7 @@ static int uprobe_perf_open(struct trace_event_call *call,
> if (trace_uprobe_filter_add(tu->tp.event->filter, event))
> return 0;
>
> - list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> - tu = container_of(pos, struct trace_uprobe, tp);
> + list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) {
> err = uprobe_apply(tu->inode, tu->offset, &tu->consumer, true);
> if (err) {
> uprobe_perf_close(call, event);
> --
> 2.33.1
>
--
Masami Hiramatsu <mhiramat@kernel.org>
next prev parent reply other threads:[~2021-11-26 12:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-25 20:28 Jiri Olsa
2021-11-26 12:40 ` Masami Hiramatsu [this message]
2021-12-08 20:46 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211126214055.b6a84e8add8f7ae3deb9411b@kernel.org \
--to=mhiramat@kernel.org \
--cc=acme@kernel.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=ravi.bangoria@amd.com \
--cc=rostedt@goodmis.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®