From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
To: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ingo Molnar <mingo@kernel.org>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
David Ahern <dsahern@gmail.com>,
lkml <linux-kernel@vger.kernel.org>,
"Steven Rostedt (Red Hat)" <rostedt@goodmis.org>,
Oleg Nesterov <oleg@redhat.com>,
"David A. Long" <dave.long@linaro.org>,
systemtap@sourceware.org, yrl.pp-manager.tt@hitachi.com,
Namhyung Kim <namhyung@kernel.org>
Subject: Re: [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path
Date: Mon, 23 Dec 2013 14:28:00 -0000 [thread overview]
Message-ID: <20131223142832.GF28878@ghostprotocols.net> (raw)
In-Reply-To: <52B75B0D.6010401@hitachi.com>
Em Mon, Dec 23, 2013 at 06:35:09AM +0900, Masami Hiramatsu escreveu:
> (2013/12/21 3:00), Arnaldo Carvalho de Melo wrote:
> > Em Fri, Dec 20, 2013 at 10:02:57AM +0000, Masami Hiramatsu escreveu:
> >> Expand given path to absolute path in option parser,
> >> except for a module name. Instead of expanding it later,
> >> this get the absolute path in early stage.
> >
> > What is the problem this solves?
> >
> > Can you provide some output showing the problem, i.e. before you apply
> > this patch?
>
> No, this is just a code cleanup, for the later enhancements.
Ok, this is just a cleanup, but what does this cleanup achieves? Why is
it better to "getting the absolute path in early stage"?
I.e. you're describing what the patch does, and I can see it from
reading code, but why is it good to do it in an early stage?
> Should I put it into the next patch?
No need for that, just, please, clarify why it is needed.
> Thank you,
>
> >
> > Then can you provide the output after the patch is applied?
> >
> > - Arnaldo
> >
> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> ---
> >> tools/perf/builtin-probe.c | 9 +++++++++
> >> tools/perf/util/probe-event.c | 11 ++---------
> >> 2 files changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> >> index 6ea9e85..b40d064 100644
> >> --- a/tools/perf/builtin-probe.c
> >> +++ b/tools/perf/builtin-probe.c
> >> @@ -180,6 +180,15 @@ static int opt_set_target(const struct option *opt, const char *str,
> >> else
> >> return ret;
> >>
> >> + /* Expand given path to absolute path, except for modulename */
> >> + if (params.uprobes || strchr(str, '/')) {
> >> + str = realpath(str, NULL);
> >> + if (!str) {
> >> + pr_warning("Failed to find the path of %s.\n",
> >> + str);
> >> + return ret;
> >> + }
> >> + }
> >> params.target = str;
> >> ret = 0;
> >> }
> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >> index d7cff57..05be5de 100644
> >> --- a/tools/perf/util/probe-event.c
> >> +++ b/tools/perf/util/probe-event.c
> >> @@ -2281,7 +2281,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
> >> struct perf_probe_point *pp = &pev->point;
> >> struct symbol *sym;
> >> struct map *map = NULL;
> >> - char *function = NULL, *name = NULL;
> >> + char *function = NULL;
> >> int ret = -EINVAL;
> >> unsigned long long vaddr = 0;
> >>
> >> @@ -2297,12 +2297,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
> >> goto out;
> >> }
> >>
> >> - name = realpath(exec, NULL);
> >> - if (!name) {
> >> - pr_warning("Cannot find realpath for %s.\n", exec);
> >> - goto out;
> >> - }
> >> - map = dso__new_map(name);
> >> + map = dso__new_map(exec);
> >> if (!map) {
> >> pr_warning("Cannot find appropriate DSO for %s.\n", exec);
> >> goto out;
> >> @@ -2367,7 +2362,5 @@ out:
> >> }
> >> if (function)
> >> free(function);
> >> - if (name)
> >> - free(name);
> >> return ret;
> >> }
> >>
> >
>
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
next prev parent reply other threads:[~2013-12-23 14:28 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-20 10:08 [PATCH -tip 0/3] perf-probe: Dwarf support for uprobes Masami Hiramatsu
2013-12-20 10:08 ` [PATCH -tip 3/3] perf-probe: Use the actual address as a hint " Masami Hiramatsu
2013-12-20 18:04 ` Arnaldo Carvalho de Melo
2013-12-22 21:54 ` Masami Hiramatsu
2013-12-23 7:46 ` Namhyung Kim
2013-12-23 10:50 ` Masami Hiramatsu
2013-12-24 7:54 ` Namhyung Kim
2013-12-24 8:28 ` Masami Hiramatsu
2013-12-24 8:46 ` Namhyung Kim
2013-12-24 15:03 ` Masami Hiramatsu
2013-12-20 10:08 ` [PATCH -tip 2/3] perf-probe: Support dwarf on uprobe events Masami Hiramatsu
2013-12-20 18:02 ` Arnaldo Carvalho de Melo
2013-12-22 21:39 ` Masami Hiramatsu
2013-12-23 14:34 ` Arnaldo Carvalho de Melo
2013-12-24 1:03 ` Masami Hiramatsu
2013-12-20 10:08 ` [PATCH -tip 1/3] [CLEANUP] perf-probe: Expand given path to absolute path Masami Hiramatsu
2013-12-20 18:00 ` Arnaldo Carvalho de Melo
2013-12-22 21:35 ` Masami Hiramatsu
2013-12-23 14:28 ` Arnaldo Carvalho de Melo [this message]
2013-12-24 6:51 ` Masami Hiramatsu
2013-12-23 6:17 ` Namhyung Kim
2013-12-23 10:46 ` Masami Hiramatsu
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=20131223142832.GF28878@ghostprotocols.net \
--to=acme@ghostprotocols.net \
--cc=dave.long@linaro.org \
--cc=dsahern@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=oleg@redhat.com \
--cc=rostedt@goodmis.org \
--cc=srikar@linux.vnet.ibm.com \
--cc=systemtap@sourceware.org \
--cc=yrl.pp-manager.tt@hitachi.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).