From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Darren Hart <dvhart@infradead.org>,
Davidlohr Bueso <dave@stgolabs.net>,
James Clark <james.clark@arm.com>,
John Garry <john.g.garry@oracle.com>,
Riccardo Mancini <rickyman7@gmail.com>,
Yury Norov <yury.norov@gmail.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Leo Yan <leo.yan@linaro.org>, Andi Kleen <ak@linux.intel.com>,
Thomas Richter <tmricht@linux.ibm.com>,
Kan Liang <kan.liang@linux.intel.com>,
Madhavan Srinivasan <maddy@linux.ibm.com>,
Shunsuke Nakamura <nakamura.shun@fujitsu.com>,
Song Liu <song@kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
Miaoqian Lin <linmq006@gmail.com>,
Stephen Brennan <stephen.s.brennan@oracle.com>,
Kajol Jain <kjain@linux.ibm.com>,
Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>,
German Gomez <german.gomez@arm.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
Eric Dumazet <edumazet@google.com>,
Dmitry Vyukov <dvyukov@google.com>, Hao Luo <haoluo@google.com>,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH v6 01/12] perf map: Rename map_ip and unmap_ip
Date: Wed, 5 Apr 2023 10:32:17 -0300 [thread overview]
Message-ID: <ZC14YdPV+Az3A7/G@kernel.org> (raw)
In-Reply-To: <ZC12sgYsbvF/C28z@kernel.org>
Em Wed, Apr 05, 2023 at 10:25:06AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Apr 04, 2023 at 01:59:43PM -0700, Ian Rogers escreveu:
> > Add dso to match comment. This avoids a naming conflict with later
> > added accessor functions for variables in struct map.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/builtin-kmem.c | 2 +-
> > tools/perf/builtin-script.c | 4 ++--
> > tools/perf/util/machine.c | 4 ++--
> > tools/perf/util/map.c | 8 ++++----
> > tools/perf/util/map.h | 4 ++--
> > tools/perf/util/symbol-elf.c | 4 ++--
> > 6 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> > index f3029742b800..4d4b770a401c 100644
> > --- a/tools/perf/builtin-kmem.c
> > +++ b/tools/perf/builtin-kmem.c
> > @@ -423,7 +423,7 @@ static u64 find_callsite(struct evsel *evsel, struct perf_sample *sample)
> > if (!caller) {
> > /* found */
> > if (node->ms.map)
> > - addr = map__unmap_ip(node->ms.map, node->ip);
> > + addr = map__dso_unmap_ip(node->ms.map, node->ip);
>
> As we chatted, I think this is a good opportunity to make this more
> clear, perhaps rename map__unmap_ip() to map__addr_offset_to_virt() and
> map__map_ip() to map__addr_virt_to_offset()?
Maybe prefix both with __, i.e. __map__addr_offset_to_virt() and __map__addr_virt_to_offset(), since
then you need to wrap map->unmap_ip() to
map__map__addr_offset_to_virt(), that in some cases will map to
__map__addr_offset_to_virt(), modulo in identity maps, like with the
kernel where we use identify__map_ip().
- Arnaldo
> - Arnaldo
>
> > else
> > addr = node->ip;
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 1d078106abc4..af0a69c7f41f 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -1012,11 +1012,11 @@ static int perf_sample__fprintf_brstackoff(struct perf_sample *sample,
> >
> > if (thread__find_map_fb(thread, sample->cpumode, from, &alf) &&
> > !map__dso(alf.map)->adjust_symbols)
> > - from = map__map_ip(alf.map, from);
> > + from = map__dso_map_ip(alf.map, from);
> >
> > if (thread__find_map_fb(thread, sample->cpumode, to, &alt) &&
> > !map__dso(alt.map)->adjust_symbols)
> > - to = map__map_ip(alt.map, to);
> > + to = map__dso_map_ip(alt.map, to);
> >
> > printed += fprintf(fp, " 0x%"PRIx64, from);
> > if (PRINT_FIELD(DSO)) {
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 7852b97da10a..9d24980a0a93 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -3058,7 +3058,7 @@ static int append_inlines(struct callchain_cursor *cursor, struct map_symbol *ms
> > if (!symbol_conf.inline_name || !map || !sym)
> > return ret;
> >
> > - addr = map__map_ip(map, ip);
> > + addr = map__dso_map_ip(map, ip);
> > addr = map__rip_2objdump(map, addr);
> > dso = map__dso(map);
> >
> > @@ -3103,7 +3103,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> > * its corresponding binary.
> > */
> > if (entry->ms.map)
> > - addr = map__map_ip(entry->ms.map, entry->ip);
> > + addr = map__dso_map_ip(entry->ms.map, entry->ip);
> >
> > srcline = callchain_srcline(&entry->ms, addr);
> > return callchain_cursor_append(cursor, entry->ip, &entry->ms,
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index 416fc449bde8..d97a6d20626f 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -109,8 +109,8 @@ void map__init(struct map *map, u64 start, u64 end, u64 pgoff, struct dso *dso)
> > map->pgoff = pgoff;
> > map->reloc = 0;
> > map->dso = dso__get(dso);
> > - map->map_ip = map__map_ip;
> > - map->unmap_ip = map__unmap_ip;
> > + map->map_ip = map__dso_map_ip;
> > + map->unmap_ip = map__dso_unmap_ip;
> > map->erange_warned = false;
> > refcount_set(&map->refcnt, 1);
> > }
> > @@ -590,12 +590,12 @@ struct maps *map__kmaps(struct map *map)
> > return kmap->kmaps;
> > }
> >
> > -u64 map__map_ip(const struct map *map, u64 ip)
> > +u64 map__dso_map_ip(const struct map *map, u64 ip)
> > {
> > return ip - map__start(map) + map->pgoff;
> > }
> >
> > -u64 map__unmap_ip(const struct map *map, u64 ip)
> > +u64 map__dso_unmap_ip(const struct map *map, u64 ip)
> > {
> > return ip + map__start(map) - map->pgoff;
> > }
> > diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> > index 16646b94fa3a..9b0a84e46e48 100644
> > --- a/tools/perf/util/map.h
> > +++ b/tools/perf/util/map.h
> > @@ -41,9 +41,9 @@ struct kmap *map__kmap(struct map *map);
> > struct maps *map__kmaps(struct map *map);
> >
> > /* ip -> dso rip */
> > -u64 map__map_ip(const struct map *map, u64 ip);
> > +u64 map__dso_map_ip(const struct map *map, u64 ip);
> > /* dso rip -> ip */
> > -u64 map__unmap_ip(const struct map *map, u64 ip);
> > +u64 map__dso_unmap_ip(const struct map *map, u64 ip);
> > /* Returns ip */
> > u64 identity__map_ip(const struct map *map __maybe_unused, u64 ip);
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index e715869eab8a..c55981116f68 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -1357,8 +1357,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
> > map->start = shdr->sh_addr + ref_reloc(kmap);
> > map->end = map__start(map) + shdr->sh_size;
> > map->pgoff = shdr->sh_offset;
> > - map->map_ip = map__map_ip;
> > - map->unmap_ip = map__unmap_ip;
> > + map->map_ip = map__dso_map_ip;
> > + map->unmap_ip = map__dso_unmap_ip;
> > /* Ensure maps are correctly ordered */
> > if (kmaps) {
> > int err;
> > --
> > 2.40.0.348.gf938b09366-goog
> >
>
> --
>
> - Arnaldo
--
- Arnaldo
next prev parent reply other threads:[~2023-04-05 13:32 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-04 20:59 [PATCH v6 00/12] Reference count checker and related fixes Ian Rogers
2023-04-04 20:59 ` [PATCH v6 01/12] perf map: Rename map_ip and unmap_ip Ian Rogers
2023-04-05 13:25 ` Arnaldo Carvalho de Melo
2023-04-05 13:32 ` Arnaldo Carvalho de Melo [this message]
2023-04-07 0:40 ` Ian Rogers
2023-04-04 20:59 ` [PATCH v6 02/12] perf map: Add helper for " Ian Rogers
2023-04-07 1:06 ` Arnaldo Carvalho de Melo
2023-04-07 1:12 ` Ian Rogers
2023-04-07 2:05 ` Arnaldo Carvalho de Melo
2023-04-04 20:59 ` [PATCH v6 03/12] perf map: Add accessors for prot, priv and flags Ian Rogers
2023-04-04 20:59 ` [PATCH v6 04/12] perf map: Add accessors for pgoff and reloc Ian Rogers
2023-04-04 20:59 ` [PATCH v6 05/12] perf test: Add extra diagnostics to maps test Ian Rogers
2023-04-04 20:59 ` [PATCH v6 06/12] perf maps: Modify maps_by_name to hold a reference to a map Ian Rogers
2023-04-04 20:59 ` [PATCH v6 07/12] perf map: Changes to reference counting Ian Rogers
2023-04-04 20:59 ` [PATCH v6 08/12] libperf: Add reference count checking macros Ian Rogers
2023-04-04 20:59 ` [PATCH v6 09/12] perf cpumap: Add reference count checking Ian Rogers
2023-04-04 20:59 ` [PATCH v6 10/12] perf namespaces: " Ian Rogers
2023-04-04 20:59 ` [PATCH v6 11/12] perf maps: " Ian Rogers
2023-04-04 20:59 ` [PATCH v6 12/12] perf map: " Ian Rogers
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=ZC14YdPV+Az3A7/G@kernel.org \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=alexey.v.bayduraev@linux.intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=dvyukov@google.com \
--cc=edumazet@google.com \
--cc=eranian@google.com \
--cc=german.gomez@arm.com \
--cc=haoluo@google.com \
--cc=irogers@google.com \
--cc=james.clark@arm.com \
--cc=john.g.garry@oracle.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=kjain@linux.ibm.com \
--cc=leo.yan@linaro.org \
--cc=linmq006@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=maddy@linux.ibm.com \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=nakamura.shun@fujitsu.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=rickyman7@gmail.com \
--cc=rostedt@goodmis.org \
--cc=song@kernel.org \
--cc=stephen.s.brennan@oracle.com \
--cc=tglx@linutronix.de \
--cc=tmricht@linux.ibm.com \
--cc=yury.norov@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.