diff mbox series

[v5,2/3] perf: add helper map__fprintf_dsoname_dsoff

Message ID	20230418031825.1262579-3-changbin.du@huawei.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Changbin Du <changbin.du@huawei.com> To: Arnaldo Carvalho de Melo <acme@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com> CC: Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, <linux-perf-users@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Hui Wang <hw.huiwang@huawei.com>, Changbin Du <changbin.du@huawei.com> Subject: [PATCH v5 2/3] perf: add helper map__fprintf_dsoname_dsoff Date: Tue, 18 Apr 2023 11:18:24 +0800 Message-ID: <20230418031825.1262579-3-changbin.du@huawei.com> In-Reply-To: <20230418031825.1262579-1-changbin.du@huawei.com> References: <20230418031825.1262579-1-changbin.du@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII Precedence: bulk
Series	perf script: Have consistent output for symbol address \| [v5,0/3] perf script: Have consistent output for symbol address [v5,1/3] perf script: print raw ip instead of binary offset for callchain [v5,2/3] perf: add helper map__fprintf_dsoname_dsoff [v5,3/3] perf: script: add new output field 'dsoff' to print dso offset

Commit Message

Changbin Du April 18, 2023, 3:18 a.m. UTC

  This adds a helper function map__fprintf_dsoname_dsoff() to print dsoname
with optional dso offset.

Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Changbin Du <changbin.du@huawei.com>
---
 tools/perf/util/map.c | 13 +++++++++++++
 tools/perf/util/map.h |  1 +
 2 files changed, 14 insertions(+)

Comments

Adrian Hunter April 19, 2023, 6:58 p.m. UTC | #1

On 18/04/23 06:18, Changbin Du wrote:
> This adds a helper function map__fprintf_dsoname_dsoff() to print dsoname
> with optional dso offset.
> 
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> ---
>  tools/perf/util/map.c | 13 +++++++++++++
>  tools/perf/util/map.h |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index d81b6ca18ee9..7da96b41100f 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -445,6 +445,19 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>  	return fprintf(fp, "%s", dsoname);
>  }
>  
> +size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
> +{
> +	int printed = 0;
> +
> +	printed += fprintf(fp, " (");
> +	printed += map__fprintf_dsoname(map, fp);
> +	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)

That will also block vmlinux offsets not just [kernel.kallsyms]
Is that what was intended?

> +		printed += fprintf(fp, "+0x%" PRIx64, addr);
> +	printed += fprintf(fp, ")");
> +
> +	return printed;
> +}
> +
>  char *map__srcline(struct map *map, u64 addr, struct symbol *sym)
>  {
>  	if (map == NULL)
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index f89ab7c2d327..4cca211b6e66 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -175,6 +175,7 @@ static inline void __map__zput(struct map **map)
>  
>  size_t map__fprintf(struct map *map, FILE *fp);
>  size_t map__fprintf_dsoname(struct map *map, FILE *fp);
> +size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp);
>  char *map__srcline(struct map *map, u64 addr, struct symbol *sym);
>  int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
>  			 FILE *fp);

Changbin Du April 20, 2023, 2:55 a.m. UTC | #2

On Wed, Apr 19, 2023 at 09:58:10PM +0300, Adrian Hunter wrote:
> On 18/04/23 06:18, Changbin Du wrote:
> > This adds a helper function map__fprintf_dsoname_dsoff() to print dsoname
> > with optional dso offset.
> > 
> > Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > ---
> >  tools/perf/util/map.c | 13 +++++++++++++
> >  tools/perf/util/map.h |  1 +
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> > index d81b6ca18ee9..7da96b41100f 100644
> > --- a/tools/perf/util/map.c
> > +++ b/tools/perf/util/map.c
> > @@ -445,6 +445,19 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
> >  	return fprintf(fp, "%s", dsoname);
> >  }
> >  
> > +size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
> > +{
> > +	int printed = 0;
> > +
> > +	printed += fprintf(fp, " (");
> > +	printed += map__fprintf_dsoname(map, fp);
> > +	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
> 
> That will also block vmlinux offsets not just [kernel.kallsyms]
> Is that what was intended?
>
Not only vmlinux, modules are also blocked. We'd better print offset for
vmlinux and modules.

$ sudo perf script -k vmlinux -F +dsoff
       perf-exec 2531039 4120893.685967:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
       perf-exec 2531039 4120893.685970:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
       perf-exec 2531039 4120893.685972:          9 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
       perf-exec 2531039 4120893.685973:        194 cycles:  ffffffff99094ec6 [unknown] (vmlinux)
       perf-exec 2531039 4120893.685974:       4605 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
       perf-exec 2531039 4120893.685976:     108083 cycles:  ffffffff9928e8d0 [unknown] (vmlinux)
              ls 2531039 4120893.686003:    2197682 cycles:  ffffffff993c92bc [unknown] (vmlinux)
              ls 2531039 4120893.686554:    4497190 cycles:  ffffffffc159692b strcasestr+0x7b (/lib/modules/5.15.0-60-generic/extra/isac_ipc.ko)
              ls 2531039 4120893.687700:    4189758 cycles:  ffffffffc18a5d66 delete_net_reject_cache+0x76 (/lib/modules/5.15.0-60-generic/extra/isac_networkfilter.ko)
              ls 2531039 4120893.688786:    3780376 cycles:  ffffffffc18a67de delete_net_process_info+0x5e (/lib/modules/5.15.0-60-generic/extra/isac_networkfilter.ko)
              ls 2531039 4120893.689716:    3416607 cycles:  ffffffffc18a67de delete_net_process_info+0x5e (/lib/modules/5.15.0-60-generic/extra/isac_networkfilter.ko)

But I found addr returned by map__dso_map_ip() for 'vmlinux' is not a 'dso
offset'.

$ sudo perf script -k vmlinux -F +dsoff
       perf-exec 2531039 4120893.685967:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux+0xffffffff99094ec4)
       perf-exec 2531039 4120893.685970:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux+0xffffffff99094ec4)
       perf-exec 2531039 4120893.685972:          9 cycles:  ffffffff99094ec4 [unknown] (vmlinux+0xffffffff99094ec4)
       perf-exec 2531039 4120893.685973:        194 cycles:  ffffffff99094ec6 [unknown] (vmlinux+0xffffffff99094ec6)

Do you have better idea?

Adrian Hunter April 20, 2023, 8:39 a.m. UTC | #3

On 20/04/23 05:55, Changbin Du wrote:
> On Wed, Apr 19, 2023 at 09:58:10PM +0300, Adrian Hunter wrote:
>> On 18/04/23 06:18, Changbin Du wrote:
>>> This adds a helper function map__fprintf_dsoname_dsoff() to print dsoname
>>> with optional dso offset.
>>>
>>> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
>>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
>>> ---
>>>  tools/perf/util/map.c | 13 +++++++++++++
>>>  tools/perf/util/map.h |  1 +
>>>  2 files changed, 14 insertions(+)
>>>
>>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>>> index d81b6ca18ee9..7da96b41100f 100644
>>> --- a/tools/perf/util/map.c
>>> +++ b/tools/perf/util/map.c
>>> @@ -445,6 +445,19 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>>>  	return fprintf(fp, "%s", dsoname);
>>>  }
>>>  
>>> +size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
>>> +{
>>> +	int printed = 0;
>>> +
>>> +	printed += fprintf(fp, " (");
>>> +	printed += map__fprintf_dsoname(map, fp);
>>> +	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
>>
>> That will also block vmlinux offsets not just [kernel.kallsyms]
>> Is that what was intended?
>>
> Not only vmlinux, modules are also blocked. We'd better print offset for
> vmlinux and modules.
> 
> $ sudo perf script -k vmlinux -F +dsoff
>        perf-exec 2531039 4120893.685967:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
>        perf-exec 2531039 4120893.685970:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
>        perf-exec 2531039 4120893.685972:          9 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
>        perf-exec 2531039 4120893.685973:        194 cycles:  ffffffff99094ec6 [unknown] (vmlinux)
>        perf-exec 2531039 4120893.685974:       4605 cycles:  ffffffff99094ec4 [unknown] (vmlinux)
>        perf-exec 2531039 4120893.685976:     108083 cycles:  ffffffff9928e8d0 [unknown] (vmlinux)
>               ls 2531039 4120893.686003:    2197682 cycles:  ffffffff993c92bc [unknown] (vmlinux)
>               ls 2531039 4120893.686554:    4497190 cycles:  ffffffffc159692b strcasestr+0x7b (/lib/modules/5.15.0-60-generic/extra/isac_ipc.ko)
>               ls 2531039 4120893.687700:    4189758 cycles:  ffffffffc18a5d66 delete_net_reject_cache+0x76 (/lib/modules/5.15.0-60-generic/extra/isac_networkfilter.ko)
>               ls 2531039 4120893.688786:    3780376 cycles:  ffffffffc18a67de delete_net_process_info+0x5e (/lib/modules/5.15.0-60-generic/extra/isac_networkfilter.ko)
>               ls 2531039 4120893.689716:    3416607 cycles:  ffffffffc18a67de delete_net_process_info+0x5e (/lib/modules/5.15.0-60-generic/extra/isac_networkfilter.ko)
> 
> But I found addr returned by map__dso_map_ip() for 'vmlinux' is not a 'dso
> offset'.
> 
> $ sudo perf script -k vmlinux -F +dsoff
>        perf-exec 2531039 4120893.685967:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux+0xffffffff99094ec4)
>        perf-exec 2531039 4120893.685970:          1 cycles:  ffffffff99094ec4 [unknown] (vmlinux+0xffffffff99094ec4)
>        perf-exec 2531039 4120893.685972:          9 cycles:  ffffffff99094ec4 [unknown] (vmlinux+0xffffffff99094ec4)
>        perf-exec 2531039 4120893.685973:        194 cycles:  ffffffff99094ec6 [unknown] (vmlinux+0xffffffff99094ec6)
> 
> Do you have better idea?
> 

What do you get if you try below diff on top of
your patches:

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index c7bf1ac14e90..df0d21141185 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -576,8 +576,11 @@ static void set_print_ip_opts(struct perf_event_attr *attr)
 	if (PRINT_FIELD(DSO))
 		output[type].print_ip_opts |= EVSEL__PRINT_DSO;
 
-	if (PRINT_FIELD(DSOFF))
+	if (PRINT_FIELD(DSOFF)) {
 		output[type].print_ip_opts |= EVSEL__PRINT_DSOFF;
+		/* DSO offset is relative to dso->longname */
+		symbol_conf.show_kernel_path = true;
+	}
 
 	if (PRINT_FIELD(SYMOFFSET))
 		output[type].print_ip_opts |= EVSEL__PRINT_SYMOFFSET;
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index a86614599269..19ebfd3468cc 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -67,6 +67,42 @@ char dso__symtab_origin(const struct dso *dso)
 	return origin[dso->symtab_type];
 }
 
+bool dso__is_file(const struct dso *dso)
+{
+	switch (dso->binary_type) {
+	case DSO_BINARY_TYPE__KALLSYMS:
+	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
+		return false;
+	case DSO_BINARY_TYPE__VMLINUX:
+	case DSO_BINARY_TYPE__GUEST_VMLINUX:
+		return true;
+	case DSO_BINARY_TYPE__JAVA_JIT:
+		return false;
+	case DSO_BINARY_TYPE__DEBUGLINK:
+	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
+	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
+	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
+	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
+	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
+	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
+	case DSO_BINARY_TYPE__GUEST_KMODULE:
+	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
+	case DSO_BINARY_TYPE__KCORE:
+	case DSO_BINARY_TYPE__GUEST_KCORE:
+	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
+		return true;
+	case DSO_BINARY_TYPE__BPF_PROG_INFO:
+	case DSO_BINARY_TYPE__BPF_IMAGE:
+	case DSO_BINARY_TYPE__OOL:
+	case DSO_BINARY_TYPE__NOT_FOUND:
+	default:
+		return false;
+	}
+}
+
 int dso__read_binary_type_filename(const struct dso *dso,
 				   enum dso_binary_type type,
 				   char *root_dir, char *filename, size_t size)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 0b7c7633b9f6..fb33f5224fb6 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -396,6 +396,8 @@ static inline bool dso__is_kallsyms(struct dso *dso)
 	return dso->kernel && dso->long_name[0] != '/';
 }
 
+bool dso__is_file(const struct dso *dso);
+
 void dso__free_a2l(struct dso *dso);
 
 enum dso_type dso__type(struct dso *dso, struct machine *machine);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 7da96b41100f..9b79f88d371c 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -447,11 +447,12 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
 
 size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
 {
+	const struct dso *dso = map ? map__dso(map) : NULL;
 	int printed = 0;
 
 	printed += fprintf(fp, " (");
 	printed += map__fprintf_dsoname(map, fp);
-	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
+	if (print_off && dso && dso__is_file(dso))
 		printed += fprintf(fp, "+0x%" PRIx64, addr);
 	printed += fprintf(fp, ")");

Changbin Du April 21, 2023, 5:04 a.m. UTC | #4

> What do you get if you try below diff on top of
> your patches:
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index c7bf1ac14e90..df0d21141185 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -576,8 +576,11 @@ static void set_print_ip_opts(struct perf_event_attr *attr)
>  	if (PRINT_FIELD(DSO))
>  		output[type].print_ip_opts |= EVSEL__PRINT_DSO;
>  
> -	if (PRINT_FIELD(DSOFF))
> +	if (PRINT_FIELD(DSOFF)) {
>  		output[type].print_ip_opts |= EVSEL__PRINT_DSOFF;
> +		/* DSO offset is relative to dso->longname */
> +		symbol_conf.show_kernel_path = true;
> +	}
>  
>  	if (PRINT_FIELD(SYMOFFSET))
>  		output[type].print_ip_opts |= EVSEL__PRINT_SYMOFFSET;
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index a86614599269..19ebfd3468cc 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -67,6 +67,42 @@ char dso__symtab_origin(const struct dso *dso)
>  	return origin[dso->symtab_type];
>  }
>  
> +bool dso__is_file(const struct dso *dso)
> +{
> +	switch (dso->binary_type) {
> +	case DSO_BINARY_TYPE__KALLSYMS:
> +	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
> +		return false;
> +	case DSO_BINARY_TYPE__VMLINUX:
> +	case DSO_BINARY_TYPE__GUEST_VMLINUX:
> +		return true;
> +	case DSO_BINARY_TYPE__JAVA_JIT:
> +		return false;
> +	case DSO_BINARY_TYPE__DEBUGLINK:
> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
> +	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
> +	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
> +	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
> +	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
> +	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
> +	case DSO_BINARY_TYPE__GUEST_KMODULE:
> +	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
> +	case DSO_BINARY_TYPE__KCORE:
> +	case DSO_BINARY_TYPE__GUEST_KCORE:
> +	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
> +		return true;
> +	case DSO_BINARY_TYPE__BPF_PROG_INFO:
> +	case DSO_BINARY_TYPE__BPF_IMAGE:
> +	case DSO_BINARY_TYPE__OOL:
> +	case DSO_BINARY_TYPE__NOT_FOUND:
> +	default:
> +		return false;
> +	}
> +}
> +
>  int dso__read_binary_type_filename(const struct dso *dso,
>  				   enum dso_binary_type type,
>  				   char *root_dir, char *filename, size_t size)
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 0b7c7633b9f6..fb33f5224fb6 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -396,6 +396,8 @@ static inline bool dso__is_kallsyms(struct dso *dso)
>  	return dso->kernel && dso->long_name[0] != '/';
>  }
>  
> +bool dso__is_file(const struct dso *dso);
> +
>  void dso__free_a2l(struct dso *dso);
>  
>  enum dso_type dso__type(struct dso *dso, struct machine *machine);
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 7da96b41100f..9b79f88d371c 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -447,11 +447,12 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>  
>  size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
>  {
> +	const struct dso *dso = map ? map__dso(map) : NULL;
>  	int printed = 0;
>  
>  	printed += fprintf(fp, " (");
>  	printed += map__fprintf_dsoname(map, fp);
> -	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
> +	if (print_off && dso && dso__is_file(dso))
>  		printed += fprintf(fp, "+0x%" PRIx64, addr);
>  	printed += fprintf(fp, ")");
>  
> 

Here are the outputs with above change.

For elf in build-id cache, it works as expected.
$ sudo perf script -F +dsoff
       perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
       perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
       perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
       perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 native_write_msr+0x6 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee46)
       perf-exec 12768   135.648032:       8850 cycles:  ffffffff96c4c686 native_sched_clock+0x66 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x24c686)
              ls 27521   501.120978:    4309123 cycles:      7f31cb51c591 _dl_sort_maps+0x301 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)

But when I specify my vmlinux: (the binary_type is DSO_BINARY_TYPE__SYSTEM_PATH_DSO)
$ sudo perf script -k linux/vmlinux -F +dsoff
       perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux)
       perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
       perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
       perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee46)

This is for kcore file:
$ sudo perf script --kallsyms /proc/kallsyms -F +dsoff
       perf-exec 18922   267.284368:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
       perf-exec 18922   267.284372:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
       perf-exec 18922   267.284374:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)

For ko, it's wierd that the binary_type of first one is DSO_BINARY_TYPE__NOT_FOUND.
$ sudo perf script -F +dsoff | grep '.ko'
           gedit 37410   769.927194:     199304 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko)
           gedit 37410   770.459667:     271035 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
           gedit 37410   770.649838:     271878 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
           gedit 37410   771.239221:     216084 cycles:  ffffffffc0a3d13b ipt_do_table+0xeb (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1db)
           gedit 37410   771.257816:     219469 cycles:  ffffffffc0a3d165 ipt_do_table+0x115 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x205)
           gedit 37410   771.531158:     288970 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
           gedit 37410   771.816916:     321215 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
           gedit 37410   773.624786:     332528 cycles:  ffffffffc0a3d2ea ipt_do_table+0x29a (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x38a)

Maybe we can just filter by name? e.g. '[kernel.kallsyms]', '[guest.kernel.kallsyms]'.

Adrian Hunter April 21, 2023, 7:30 a.m. UTC | #5

On 21/04/23 08:04, Changbin Du wrote:
>> What do you get if you try below diff on top of
>> your patches:
>>
>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>> index c7bf1ac14e90..df0d21141185 100644
>> --- a/tools/perf/builtin-script.c
>> +++ b/tools/perf/builtin-script.c
>> @@ -576,8 +576,11 @@ static void set_print_ip_opts(struct perf_event_attr *attr)
>>  	if (PRINT_FIELD(DSO))
>>  		output[type].print_ip_opts |= EVSEL__PRINT_DSO;
>>  
>> -	if (PRINT_FIELD(DSOFF))
>> +	if (PRINT_FIELD(DSOFF)) {
>>  		output[type].print_ip_opts |= EVSEL__PRINT_DSOFF;
>> +		/* DSO offset is relative to dso->longname */
>> +		symbol_conf.show_kernel_path = true;
>> +	}
>>  
>>  	if (PRINT_FIELD(SYMOFFSET))
>>  		output[type].print_ip_opts |= EVSEL__PRINT_SYMOFFSET;
>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
>> index a86614599269..19ebfd3468cc 100644
>> --- a/tools/perf/util/dso.c
>> +++ b/tools/perf/util/dso.c
>> @@ -67,6 +67,42 @@ char dso__symtab_origin(const struct dso *dso)
>>  	return origin[dso->symtab_type];
>>  }
>>  
>> +bool dso__is_file(const struct dso *dso)
>> +{
>> +	switch (dso->binary_type) {
>> +	case DSO_BINARY_TYPE__KALLSYMS:
>> +	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
>> +		return false;
>> +	case DSO_BINARY_TYPE__VMLINUX:
>> +	case DSO_BINARY_TYPE__GUEST_VMLINUX:
>> +		return true;
>> +	case DSO_BINARY_TYPE__JAVA_JIT:
>> +		return false;
>> +	case DSO_BINARY_TYPE__DEBUGLINK:
>> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
>> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
>> +	case DSO_BINARY_TYPE__GUEST_KMODULE:
>> +	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
>> +	case DSO_BINARY_TYPE__KCORE:
>> +	case DSO_BINARY_TYPE__GUEST_KCORE:
>> +	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
>> +		return true;
>> +	case DSO_BINARY_TYPE__BPF_PROG_INFO:
>> +	case DSO_BINARY_TYPE__BPF_IMAGE:
>> +	case DSO_BINARY_TYPE__OOL:
>> +	case DSO_BINARY_TYPE__NOT_FOUND:
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>>  int dso__read_binary_type_filename(const struct dso *dso,
>>  				   enum dso_binary_type type,
>>  				   char *root_dir, char *filename, size_t size)
>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
>> index 0b7c7633b9f6..fb33f5224fb6 100644
>> --- a/tools/perf/util/dso.h
>> +++ b/tools/perf/util/dso.h
>> @@ -396,6 +396,8 @@ static inline bool dso__is_kallsyms(struct dso *dso)
>>  	return dso->kernel && dso->long_name[0] != '/';
>>  }
>>  
>> +bool dso__is_file(const struct dso *dso);
>> +
>>  void dso__free_a2l(struct dso *dso);
>>  
>>  enum dso_type dso__type(struct dso *dso, struct machine *machine);
>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>> index 7da96b41100f..9b79f88d371c 100644
>> --- a/tools/perf/util/map.c
>> +++ b/tools/perf/util/map.c
>> @@ -447,11 +447,12 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>>  
>>  size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
>>  {
>> +	const struct dso *dso = map ? map__dso(map) : NULL;
>>  	int printed = 0;
>>  
>>  	printed += fprintf(fp, " (");
>>  	printed += map__fprintf_dsoname(map, fp);
>> -	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
>> +	if (print_off && dso && dso__is_file(dso))
>>  		printed += fprintf(fp, "+0x%" PRIx64, addr);
>>  	printed += fprintf(fp, ")");
>>  
>>
> 
> Here are the outputs with above change.
> 
> For elf in build-id cache, it works as expected.
> $ sudo perf script -F +dsoff
>        perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
>        perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
>        perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
>        perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 native_write_msr+0x6 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee46)
>        perf-exec 12768   135.648032:       8850 cycles:  ffffffff96c4c686 native_sched_clock+0x66 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x24c686)

A bit messy though.  User can use option --show-kernel-path
so let's not force that after all.

>               ls 27521   501.120978:    4309123 cycles:      7f31cb51c591 _dl_sort_maps+0x301 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
> 
> But when I specify my vmlinux: (the binary_type is DSO_BINARY_TYPE__SYSTEM_PATH_DSO)
> $ sudo perf script -k linux/vmlinux -F +dsoff
>        perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux)
>        perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
>        perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
>        perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee46)
> 
> This is for kcore file:
> $ sudo perf script --kallsyms /proc/kallsyms -F +dsoff
>        perf-exec 18922   267.284368:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
>        perf-exec 18922   267.284372:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
>        perf-exec 18922   267.284374:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
> 
> For ko, it's wierd that the binary_type of first one is DSO_BINARY_TYPE__NOT_FOUND.
> $ sudo perf script -F +dsoff | grep '.ko'
>            gedit 37410   769.927194:     199304 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko)
>            gedit 37410   770.459667:     271035 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
>            gedit 37410   770.649838:     271878 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
>            gedit 37410   771.239221:     216084 cycles:  ffffffffc0a3d13b ipt_do_table+0xeb (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1db)
>            gedit 37410   771.257816:     219469 cycles:  ffffffffc0a3d165 ipt_do_table+0x115 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x205)
>            gedit 37410   771.531158:     288970 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
>            gedit 37410   771.816916:     321215 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
>            gedit 37410   773.624786:     332528 cycles:  ffffffffc0a3d2ea ipt_do_table+0x29a (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x38a)
> 
> Maybe we can just filter by name? e.g. '[kernel.kallsyms]', '[guest.kernel.kallsyms]'.

Using the dso->binary_type deals with other cases also.
We can just change case DSO_BINARY_TYPE__NOT_FOUND to return
true.

How about this:

commit abaf1cbf5be5d50b1b3682511b92794394b72178
Author: Adrian Hunter <adrian.hunter@intel.com>
Date:   Fri Apr 21 10:15:14 2023 +0300

    perf script: Refine printing of dso offset
    
    Print dso offset only for object files, and in those cases force using the
    dso->long_name if the dso->name starts with '[' or the dso is kcore, in
    order to avoid special names such as [vdso], or mixing up kcore with
    vmlinux.
    
    Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index a86614599269..046fbfcfdaab 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -67,6 +67,39 @@ char dso__symtab_origin(const struct dso *dso)
 	return origin[dso->symtab_type];
 }
 
+bool dso__is_object_file(const struct dso *dso)
+{
+	switch (dso->binary_type) {
+	case DSO_BINARY_TYPE__KALLSYMS:
+	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
+	case DSO_BINARY_TYPE__JAVA_JIT:
+	case DSO_BINARY_TYPE__BPF_PROG_INFO:
+	case DSO_BINARY_TYPE__BPF_IMAGE:
+	case DSO_BINARY_TYPE__OOL:
+		return false;
+	case DSO_BINARY_TYPE__VMLINUX:
+	case DSO_BINARY_TYPE__GUEST_VMLINUX:
+	case DSO_BINARY_TYPE__DEBUGLINK:
+	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
+	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
+	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
+	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
+	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
+	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
+	case DSO_BINARY_TYPE__GUEST_KMODULE:
+	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
+	case DSO_BINARY_TYPE__KCORE:
+	case DSO_BINARY_TYPE__GUEST_KCORE:
+	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
+	case DSO_BINARY_TYPE__NOT_FOUND:
+	default:
+		return true;
+	}
+}
+
 int dso__read_binary_type_filename(const struct dso *dso,
 				   enum dso_binary_type type,
 				   char *root_dir, char *filename, size_t size)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 0b7c7633b9f6..b23a157c914d 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -379,23 +379,25 @@ void dso__reset_find_symbol_cache(struct dso *dso);
 size_t dso__fprintf_symbols_by_name(struct dso *dso, FILE *fp);
 size_t dso__fprintf(struct dso *dso, FILE *fp);
 
-static inline bool dso__is_vmlinux(struct dso *dso)
+static inline bool dso__is_vmlinux(const struct dso *dso)
 {
 	return dso->binary_type == DSO_BINARY_TYPE__VMLINUX ||
 	       dso->binary_type == DSO_BINARY_TYPE__GUEST_VMLINUX;
 }
 
-static inline bool dso__is_kcore(struct dso *dso)
+static inline bool dso__is_kcore(const struct dso *dso)
 {
 	return dso->binary_type == DSO_BINARY_TYPE__KCORE ||
 	       dso->binary_type == DSO_BINARY_TYPE__GUEST_KCORE;
 }
 
-static inline bool dso__is_kallsyms(struct dso *dso)
+static inline bool dso__is_kallsyms(const struct dso *dso)
 {
 	return dso->kernel && dso->long_name[0] != '/';
 }
 
+bool dso__is_object_file(const struct dso *dso);
+
 void dso__free_a2l(struct dso *dso);
 
 enum dso_type dso__type(struct dso *dso, struct machine *machine);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 7da96b41100f..5e7808b0bc87 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -424,14 +424,21 @@ size_t map__fprintf(struct map *map, FILE *fp)
 		       map__start(map), map__end(map), map__pgoff(map), dso->name);
 }
 
-size_t map__fprintf_dsoname(struct map *map, FILE *fp)
+static bool prefer_dso_long_name(const struct dso *dso, bool print_off)
+{
+	return dso->long_name &&
+	       (symbol_conf.show_kernel_path ||
+		(print_off && (dso->name[0] == '[' || dso__is_kcore(dso))));
+}
+
+static size_t __map__fprintf_dsoname(struct map *map, bool print_off, FILE *fp)
 {
 	char buf[symbol_conf.pad_output_len_dso + 1];
 	const char *dsoname = "[unknown]";
 	const struct dso *dso = map ? map__dso(map) : NULL;
 
 	if (dso) {
-		if (symbol_conf.show_kernel_path && dso->long_name)
+		if (prefer_dso_long_name(dso, print_off))
 			dsoname = dso->long_name;
 		else
 			dsoname = dso->name;
@@ -445,13 +452,21 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
 	return fprintf(fp, "%s", dsoname);
 }
 
+size_t map__fprintf_dsoname(struct map *map, FILE *fp)
+{
+	return __map__fprintf_dsoname(map, false, fp);
+}
+
 size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
 {
+	const struct dso *dso = map ? map__dso(map) : NULL;
 	int printed = 0;
 
+	if (print_off && (!dso || !dso__is_object_file(dso)))
+		print_off = false;
 	printed += fprintf(fp, " (");
-	printed += map__fprintf_dsoname(map, fp);
-	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
+	printed += __map__fprintf_dsoname(map, print_off, fp);
+	if (print_off)
 		printed += fprintf(fp, "+0x%" PRIx64, addr);
 	printed += fprintf(fp, ")");

Changbin Du April 23, 2023, 4:32 a.m. UTC | #6

On Fri, Apr 21, 2023 at 10:30:45AM +0300, Adrian Hunter wrote:
> On 21/04/23 08:04, Changbin Du wrote:
> >> What do you get if you try below diff on top of
> >> your patches:
> >>
> >> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> >> index c7bf1ac14e90..df0d21141185 100644
> >> --- a/tools/perf/builtin-script.c
> >> +++ b/tools/perf/builtin-script.c
> >> @@ -576,8 +576,11 @@ static void set_print_ip_opts(struct perf_event_attr *attr)
> >>  	if (PRINT_FIELD(DSO))
> >>  		output[type].print_ip_opts |= EVSEL__PRINT_DSO;
> >>  
> >> -	if (PRINT_FIELD(DSOFF))
> >> +	if (PRINT_FIELD(DSOFF)) {
> >>  		output[type].print_ip_opts |= EVSEL__PRINT_DSOFF;
> >> +		/* DSO offset is relative to dso->longname */
> >> +		symbol_conf.show_kernel_path = true;
> >> +	}
> >>  
> >>  	if (PRINT_FIELD(SYMOFFSET))
> >>  		output[type].print_ip_opts |= EVSEL__PRINT_SYMOFFSET;
> >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> >> index a86614599269..19ebfd3468cc 100644
> >> --- a/tools/perf/util/dso.c
> >> +++ b/tools/perf/util/dso.c
> >> @@ -67,6 +67,42 @@ char dso__symtab_origin(const struct dso *dso)
> >>  	return origin[dso->symtab_type];
> >>  }
> >>  
> >> +bool dso__is_file(const struct dso *dso)
> >> +{
> >> +	switch (dso->binary_type) {
> >> +	case DSO_BINARY_TYPE__KALLSYMS:
> >> +	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
> >> +		return false;
> >> +	case DSO_BINARY_TYPE__VMLINUX:
> >> +	case DSO_BINARY_TYPE__GUEST_VMLINUX:
> >> +		return true;
> >> +	case DSO_BINARY_TYPE__JAVA_JIT:
> >> +		return false;
> >> +	case DSO_BINARY_TYPE__DEBUGLINK:
> >> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
> >> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
> >> +	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
> >> +	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
> >> +	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
> >> +	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
> >> +	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
> >> +	case DSO_BINARY_TYPE__GUEST_KMODULE:
> >> +	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
> >> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
> >> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
> >> +	case DSO_BINARY_TYPE__KCORE:
> >> +	case DSO_BINARY_TYPE__GUEST_KCORE:
> >> +	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
> >> +		return true;
> >> +	case DSO_BINARY_TYPE__BPF_PROG_INFO:
> >> +	case DSO_BINARY_TYPE__BPF_IMAGE:
> >> +	case DSO_BINARY_TYPE__OOL:
> >> +	case DSO_BINARY_TYPE__NOT_FOUND:
> >> +	default:
> >> +		return false;
> >> +	}
> >> +}
> >> +
> >>  int dso__read_binary_type_filename(const struct dso *dso,
> >>  				   enum dso_binary_type type,
> >>  				   char *root_dir, char *filename, size_t size)
> >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> >> index 0b7c7633b9f6..fb33f5224fb6 100644
> >> --- a/tools/perf/util/dso.h
> >> +++ b/tools/perf/util/dso.h
> >> @@ -396,6 +396,8 @@ static inline bool dso__is_kallsyms(struct dso *dso)
> >>  	return dso->kernel && dso->long_name[0] != '/';
> >>  }
> >>  
> >> +bool dso__is_file(const struct dso *dso);
> >> +
> >>  void dso__free_a2l(struct dso *dso);
> >>  
> >>  enum dso_type dso__type(struct dso *dso, struct machine *machine);
> >> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> >> index 7da96b41100f..9b79f88d371c 100644
> >> --- a/tools/perf/util/map.c
> >> +++ b/tools/perf/util/map.c
> >> @@ -447,11 +447,12 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
> >>  
> >>  size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
> >>  {
> >> +	const struct dso *dso = map ? map__dso(map) : NULL;
> >>  	int printed = 0;
> >>  
> >>  	printed += fprintf(fp, " (");
> >>  	printed += map__fprintf_dsoname(map, fp);
> >> -	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
> >> +	if (print_off && dso && dso__is_file(dso))
> >>  		printed += fprintf(fp, "+0x%" PRIx64, addr);
> >>  	printed += fprintf(fp, ")");
> >>  
> >>
> > 
> > Here are the outputs with above change.
> > 
> > For elf in build-id cache, it works as expected.
> > $ sudo perf script -F +dsoff
> >        perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
> >        perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
> >        perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
> >        perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 native_write_msr+0x6 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee46)
> >        perf-exec 12768   135.648032:       8850 cycles:  ffffffff96c4c686 native_sched_clock+0x66 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x24c686)
> 
> A bit messy though.  User can use option --show-kernel-path
> so let's not force that after all.
> 
> >               ls 27521   501.120978:    4309123 cycles:      7f31cb51c591 _dl_sort_maps+0x301 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
> > 
> > But when I specify my vmlinux: (the binary_type is DSO_BINARY_TYPE__SYSTEM_PATH_DSO)
> > $ sudo perf script -k linux/vmlinux -F +dsoff
> >        perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux)
> >        perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
> >        perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
> >        perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee46)
> > 
> > This is for kcore file:
> > $ sudo perf script --kallsyms /proc/kallsyms -F +dsoff
> >        perf-exec 18922   267.284368:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
> >        perf-exec 18922   267.284372:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
> >        perf-exec 18922   267.284374:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
> > 
> > For ko, it's wierd that the binary_type of first one is DSO_BINARY_TYPE__NOT_FOUND.
> > $ sudo perf script -F +dsoff | grep '.ko'
> >            gedit 37410   769.927194:     199304 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko)
> >            gedit 37410   770.459667:     271035 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
> >            gedit 37410   770.649838:     271878 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
> >            gedit 37410   771.239221:     216084 cycles:  ffffffffc0a3d13b ipt_do_table+0xeb (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1db)
> >            gedit 37410   771.257816:     219469 cycles:  ffffffffc0a3d165 ipt_do_table+0x115 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x205)
> >            gedit 37410   771.531158:     288970 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
> >            gedit 37410   771.816916:     321215 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
> >            gedit 37410   773.624786:     332528 cycles:  ffffffffc0a3d2ea ipt_do_table+0x29a (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x38a)
> > 
> > Maybe we can just filter by name? e.g. '[kernel.kallsyms]', '[guest.kernel.kallsyms]'.
> 
> Using the dso->binary_type deals with other cases also.
> We can just change case DSO_BINARY_TYPE__NOT_FOUND to return
> true.
> 
> How about this:
> 
> commit abaf1cbf5be5d50b1b3682511b92794394b72178
> Author: Adrian Hunter <adrian.hunter@intel.com>
> Date:   Fri Apr 21 10:15:14 2023 +0300
> 
>     perf script: Refine printing of dso offset
>     
>     Print dso offset only for object files, and in those cases force using the
>     dso->long_name if the dso->name starts with '[' or the dso is kcore, in
>     order to avoid special names such as [vdso], or mixing up kcore with
>     vmlinux.
>     
>     Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index a86614599269..046fbfcfdaab 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -67,6 +67,39 @@ char dso__symtab_origin(const struct dso *dso)
>  	return origin[dso->symtab_type];
>  }
>  
> +bool dso__is_object_file(const struct dso *dso)
> +{
> +	switch (dso->binary_type) {
> +	case DSO_BINARY_TYPE__KALLSYMS:
> +	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
> +	case DSO_BINARY_TYPE__JAVA_JIT:
> +	case DSO_BINARY_TYPE__BPF_PROG_INFO:
> +	case DSO_BINARY_TYPE__BPF_IMAGE:
> +	case DSO_BINARY_TYPE__OOL:
> +		return false;
> +	case DSO_BINARY_TYPE__VMLINUX:
> +	case DSO_BINARY_TYPE__GUEST_VMLINUX:
> +	case DSO_BINARY_TYPE__DEBUGLINK:
> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
> +	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
> +	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
> +	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
> +	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
> +	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
> +	case DSO_BINARY_TYPE__GUEST_KMODULE:
> +	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
> +	case DSO_BINARY_TYPE__KCORE:
> +	case DSO_BINARY_TYPE__GUEST_KCORE:
> +	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
> +	case DSO_BINARY_TYPE__NOT_FOUND:
> +	default:
> +		return true;
> +	}
> +}
> +
>  int dso__read_binary_type_filename(const struct dso *dso,
>  				   enum dso_binary_type type,
>  				   char *root_dir, char *filename, size_t size)
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 0b7c7633b9f6..b23a157c914d 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -379,23 +379,25 @@ void dso__reset_find_symbol_cache(struct dso *dso);
>  size_t dso__fprintf_symbols_by_name(struct dso *dso, FILE *fp);
>  size_t dso__fprintf(struct dso *dso, FILE *fp);
>  
> -static inline bool dso__is_vmlinux(struct dso *dso)
> +static inline bool dso__is_vmlinux(const struct dso *dso)
>  {
>  	return dso->binary_type == DSO_BINARY_TYPE__VMLINUX ||
>  	       dso->binary_type == DSO_BINARY_TYPE__GUEST_VMLINUX;
>  }
>  
> -static inline bool dso__is_kcore(struct dso *dso)
> +static inline bool dso__is_kcore(const struct dso *dso)
>  {
>  	return dso->binary_type == DSO_BINARY_TYPE__KCORE ||
>  	       dso->binary_type == DSO_BINARY_TYPE__GUEST_KCORE;
>  }
>  
> -static inline bool dso__is_kallsyms(struct dso *dso)
> +static inline bool dso__is_kallsyms(const struct dso *dso)
>  {
>  	return dso->kernel && dso->long_name[0] != '/';
>  }
>  
> +bool dso__is_object_file(const struct dso *dso);
> +
>  void dso__free_a2l(struct dso *dso);
>  
>  enum dso_type dso__type(struct dso *dso, struct machine *machine);
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 7da96b41100f..5e7808b0bc87 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -424,14 +424,21 @@ size_t map__fprintf(struct map *map, FILE *fp)
>  		       map__start(map), map__end(map), map__pgoff(map), dso->name);
>  }
>  
> -size_t map__fprintf_dsoname(struct map *map, FILE *fp)
> +static bool prefer_dso_long_name(const struct dso *dso, bool print_off)
> +{
> +	return dso->long_name &&
> +	       (symbol_conf.show_kernel_path ||
> +		(print_off && (dso->name[0] == '[' || dso__is_kcore(dso))));
> +}
> +
> +static size_t __map__fprintf_dsoname(struct map *map, bool print_off, FILE *fp)
>  {
>  	char buf[symbol_conf.pad_output_len_dso + 1];
>  	const char *dsoname = "[unknown]";
>  	const struct dso *dso = map ? map__dso(map) : NULL;
>  
>  	if (dso) {
> -		if (symbol_conf.show_kernel_path && dso->long_name)
> +		if (prefer_dso_long_name(dso, print_off))
>  			dsoname = dso->long_name;
>  		else
>  			dsoname = dso->name;
> @@ -445,13 +452,21 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>  	return fprintf(fp, "%s", dsoname);
>  }
>  
> +size_t map__fprintf_dsoname(struct map *map, FILE *fp)
> +{
> +	return __map__fprintf_dsoname(map, false, fp);
> +}
> +
>  size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
>  {
> +	const struct dso *dso = map ? map__dso(map) : NULL;
>  	int printed = 0;
>  
> +	if (print_off && (!dso || !dso__is_object_file(dso)))
> +		print_off = false;
>  	printed += fprintf(fp, " (");
> -	printed += map__fprintf_dsoname(map, fp);
> -	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
> +	printed += __map__fprintf_dsoname(map, print_off, fp);
> +	if (print_off)
>  		printed += fprintf(fp, "+0x%" PRIx64, addr);
>  	printed += fprintf(fp, ")");
>  
> 
> 
I am fine with this polish patch. So will send this standalone or append to this
thread?

Adrian Hunter April 24, 2023, 5:25 a.m. UTC | #7

On 23/04/23 07:32, Changbin Du wrote:
> On Fri, Apr 21, 2023 at 10:30:45AM +0300, Adrian Hunter wrote:
>> On 21/04/23 08:04, Changbin Du wrote:
>>>> What do you get if you try below diff on top of
>>>> your patches:
>>>>
>>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>>> index c7bf1ac14e90..df0d21141185 100644
>>>> --- a/tools/perf/builtin-script.c
>>>> +++ b/tools/perf/builtin-script.c
>>>> @@ -576,8 +576,11 @@ static void set_print_ip_opts(struct perf_event_attr *attr)
>>>>  	if (PRINT_FIELD(DSO))
>>>>  		output[type].print_ip_opts |= EVSEL__PRINT_DSO;
>>>>  
>>>> -	if (PRINT_FIELD(DSOFF))
>>>> +	if (PRINT_FIELD(DSOFF)) {
>>>>  		output[type].print_ip_opts |= EVSEL__PRINT_DSOFF;
>>>> +		/* DSO offset is relative to dso->longname */
>>>> +		symbol_conf.show_kernel_path = true;
>>>> +	}
>>>>  
>>>>  	if (PRINT_FIELD(SYMOFFSET))
>>>>  		output[type].print_ip_opts |= EVSEL__PRINT_SYMOFFSET;
>>>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
>>>> index a86614599269..19ebfd3468cc 100644
>>>> --- a/tools/perf/util/dso.c
>>>> +++ b/tools/perf/util/dso.c
>>>> @@ -67,6 +67,42 @@ char dso__symtab_origin(const struct dso *dso)
>>>>  	return origin[dso->symtab_type];
>>>>  }
>>>>  
>>>> +bool dso__is_file(const struct dso *dso)
>>>> +{
>>>> +	switch (dso->binary_type) {
>>>> +	case DSO_BINARY_TYPE__KALLSYMS:
>>>> +	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
>>>> +		return false;
>>>> +	case DSO_BINARY_TYPE__VMLINUX:
>>>> +	case DSO_BINARY_TYPE__GUEST_VMLINUX:
>>>> +		return true;
>>>> +	case DSO_BINARY_TYPE__JAVA_JIT:
>>>> +		return false;
>>>> +	case DSO_BINARY_TYPE__DEBUGLINK:
>>>> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
>>>> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
>>>> +	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
>>>> +	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
>>>> +	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
>>>> +	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
>>>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
>>>> +	case DSO_BINARY_TYPE__GUEST_KMODULE:
>>>> +	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
>>>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
>>>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
>>>> +	case DSO_BINARY_TYPE__KCORE:
>>>> +	case DSO_BINARY_TYPE__GUEST_KCORE:
>>>> +	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
>>>> +		return true;
>>>> +	case DSO_BINARY_TYPE__BPF_PROG_INFO:
>>>> +	case DSO_BINARY_TYPE__BPF_IMAGE:
>>>> +	case DSO_BINARY_TYPE__OOL:
>>>> +	case DSO_BINARY_TYPE__NOT_FOUND:
>>>> +	default:
>>>> +		return false;
>>>> +	}
>>>> +}
>>>> +
>>>>  int dso__read_binary_type_filename(const struct dso *dso,
>>>>  				   enum dso_binary_type type,
>>>>  				   char *root_dir, char *filename, size_t size)
>>>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
>>>> index 0b7c7633b9f6..fb33f5224fb6 100644
>>>> --- a/tools/perf/util/dso.h
>>>> +++ b/tools/perf/util/dso.h
>>>> @@ -396,6 +396,8 @@ static inline bool dso__is_kallsyms(struct dso *dso)
>>>>  	return dso->kernel && dso->long_name[0] != '/';
>>>>  }
>>>>  
>>>> +bool dso__is_file(const struct dso *dso);
>>>> +
>>>>  void dso__free_a2l(struct dso *dso);
>>>>  
>>>>  enum dso_type dso__type(struct dso *dso, struct machine *machine);
>>>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>>>> index 7da96b41100f..9b79f88d371c 100644
>>>> --- a/tools/perf/util/map.c
>>>> +++ b/tools/perf/util/map.c
>>>> @@ -447,11 +447,12 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>>>>  
>>>>  size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
>>>>  {
>>>> +	const struct dso *dso = map ? map__dso(map) : NULL;
>>>>  	int printed = 0;
>>>>  
>>>>  	printed += fprintf(fp, " (");
>>>>  	printed += map__fprintf_dsoname(map, fp);
>>>> -	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
>>>> +	if (print_off && dso && dso__is_file(dso))
>>>>  		printed += fprintf(fp, "+0x%" PRIx64, addr);
>>>>  	printed += fprintf(fp, ")");
>>>>  
>>>>
>>>
>>> Here are the outputs with above change.
>>>
>>> For elf in build-id cache, it works as expected.
>>> $ sudo perf script -F +dsoff
>>>        perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
>>>        perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
>>>        perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee44)
>>>        perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 native_write_msr+0x6 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x28ee46)
>>>        perf-exec 12768   135.648032:       8850 cycles:  ffffffff96c4c686 native_sched_clock+0x66 (/home/changbin/.debug/.build-id/5e/2fa721660d663f38b6e1aa98d6fa3776974b54/elf+0x24c686)
>>
>> A bit messy though.  User can use option --show-kernel-path
>> so let's not force that after all.
>>
>>>               ls 27521   501.120978:    4309123 cycles:      7f31cb51c591 _dl_sort_maps+0x301 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
>>>
>>> But when I specify my vmlinux: (the binary_type is DSO_BINARY_TYPE__SYSTEM_PATH_DSO)
>>> $ sudo perf script -k linux/vmlinux -F +dsoff
>>>        perf-exec 12768   135.648023:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux)
>>>        perf-exec 12768   135.648028:          1 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
>>>        perf-exec 12768   135.648030:         11 cycles:  ffffffff96c8ee44 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee44)
>>>        perf-exec 12768   135.648031:        295 cycles:  ffffffff96c8ee46 [unknown] (/lib/modules/6.2.12/build/vmlinux+0xffffffff96c8ee46)
>>>
>>> This is for kcore file:
>>> $ sudo perf script --kallsyms /proc/kallsyms -F +dsoff
>>>        perf-exec 18922   267.284368:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
>>>        perf-exec 18922   267.284372:          1 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
>>>        perf-exec 18922   267.284374:         11 cycles:  ffffffff96c8ee44 native_write_msr+0x4 (/proc/kcore+0x7fff96c91e44)
>>>
>>> For ko, it's wierd that the binary_type of first one is DSO_BINARY_TYPE__NOT_FOUND.
>>> $ sudo perf script -F +dsoff | grep '.ko'
>>>            gedit 37410   769.927194:     199304 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko)
>>>            gedit 37410   770.459667:     271035 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
>>>            gedit 37410   770.649838:     271878 cycles:  ffffffffc0a3d050 ipt_do_table+0x0 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0xf0)
>>>            gedit 37410   771.239221:     216084 cycles:  ffffffffc0a3d13b ipt_do_table+0xeb (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1db)
>>>            gedit 37410   771.257816:     219469 cycles:  ffffffffc0a3d165 ipt_do_table+0x115 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x205)
>>>            gedit 37410   771.531158:     288970 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
>>>            gedit 37410   771.816916:     321215 cycles:  ffffffffc0a3d151 ipt_do_table+0x101 (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x1f1)
>>>            gedit 37410   773.624786:     332528 cycles:  ffffffffc0a3d2ea ipt_do_table+0x29a (/lib/modules/6.2.12/kernel/net/ipv4/netfilter/ip_tables.ko+0x38a)
>>>
>>> Maybe we can just filter by name? e.g. '[kernel.kallsyms]', '[guest.kernel.kallsyms]'.
>>
>> Using the dso->binary_type deals with other cases also.
>> We can just change case DSO_BINARY_TYPE__NOT_FOUND to return
>> true.
>>
>> How about this:
>>
>> commit abaf1cbf5be5d50b1b3682511b92794394b72178
>> Author: Adrian Hunter <adrian.hunter@intel.com>
>> Date:   Fri Apr 21 10:15:14 2023 +0300
>>
>>     perf script: Refine printing of dso offset
>>     
>>     Print dso offset only for object files, and in those cases force using the
>>     dso->long_name if the dso->name starts with '[' or the dso is kcore, in
>>     order to avoid special names such as [vdso], or mixing up kcore with
>>     vmlinux.
>>     
>>     Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>
>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
>> index a86614599269..046fbfcfdaab 100644
>> --- a/tools/perf/util/dso.c
>> +++ b/tools/perf/util/dso.c
>> @@ -67,6 +67,39 @@ char dso__symtab_origin(const struct dso *dso)
>>  	return origin[dso->symtab_type];
>>  }
>>  
>> +bool dso__is_object_file(const struct dso *dso)
>> +{
>> +	switch (dso->binary_type) {
>> +	case DSO_BINARY_TYPE__KALLSYMS:
>> +	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
>> +	case DSO_BINARY_TYPE__JAVA_JIT:
>> +	case DSO_BINARY_TYPE__BPF_PROG_INFO:
>> +	case DSO_BINARY_TYPE__BPF_IMAGE:
>> +	case DSO_BINARY_TYPE__OOL:
>> +		return false;
>> +	case DSO_BINARY_TYPE__VMLINUX:
>> +	case DSO_BINARY_TYPE__GUEST_VMLINUX:
>> +	case DSO_BINARY_TYPE__DEBUGLINK:
>> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
>> +	case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
>> +	case DSO_BINARY_TYPE__GUEST_KMODULE:
>> +	case DSO_BINARY_TYPE__GUEST_KMODULE_COMP:
>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
>> +	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
>> +	case DSO_BINARY_TYPE__KCORE:
>> +	case DSO_BINARY_TYPE__GUEST_KCORE:
>> +	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
>> +	case DSO_BINARY_TYPE__NOT_FOUND:
>> +	default:
>> +		return true;
>> +	}
>> +}
>> +
>>  int dso__read_binary_type_filename(const struct dso *dso,
>>  				   enum dso_binary_type type,
>>  				   char *root_dir, char *filename, size_t size)
>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
>> index 0b7c7633b9f6..b23a157c914d 100644
>> --- a/tools/perf/util/dso.h
>> +++ b/tools/perf/util/dso.h
>> @@ -379,23 +379,25 @@ void dso__reset_find_symbol_cache(struct dso *dso);
>>  size_t dso__fprintf_symbols_by_name(struct dso *dso, FILE *fp);
>>  size_t dso__fprintf(struct dso *dso, FILE *fp);
>>  
>> -static inline bool dso__is_vmlinux(struct dso *dso)
>> +static inline bool dso__is_vmlinux(const struct dso *dso)
>>  {
>>  	return dso->binary_type == DSO_BINARY_TYPE__VMLINUX ||
>>  	       dso->binary_type == DSO_BINARY_TYPE__GUEST_VMLINUX;
>>  }
>>  
>> -static inline bool dso__is_kcore(struct dso *dso)
>> +static inline bool dso__is_kcore(const struct dso *dso)
>>  {
>>  	return dso->binary_type == DSO_BINARY_TYPE__KCORE ||
>>  	       dso->binary_type == DSO_BINARY_TYPE__GUEST_KCORE;
>>  }
>>  
>> -static inline bool dso__is_kallsyms(struct dso *dso)
>> +static inline bool dso__is_kallsyms(const struct dso *dso)
>>  {
>>  	return dso->kernel && dso->long_name[0] != '/';
>>  }
>>  
>> +bool dso__is_object_file(const struct dso *dso);
>> +
>>  void dso__free_a2l(struct dso *dso);
>>  
>>  enum dso_type dso__type(struct dso *dso, struct machine *machine);
>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>> index 7da96b41100f..5e7808b0bc87 100644
>> --- a/tools/perf/util/map.c
>> +++ b/tools/perf/util/map.c
>> @@ -424,14 +424,21 @@ size_t map__fprintf(struct map *map, FILE *fp)
>>  		       map__start(map), map__end(map), map__pgoff(map), dso->name);
>>  }
>>  
>> -size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>> +static bool prefer_dso_long_name(const struct dso *dso, bool print_off)
>> +{
>> +	return dso->long_name &&
>> +	       (symbol_conf.show_kernel_path ||
>> +		(print_off && (dso->name[0] == '[' || dso__is_kcore(dso))));
>> +}
>> +
>> +static size_t __map__fprintf_dsoname(struct map *map, bool print_off, FILE *fp)
>>  {
>>  	char buf[symbol_conf.pad_output_len_dso + 1];
>>  	const char *dsoname = "[unknown]";
>>  	const struct dso *dso = map ? map__dso(map) : NULL;
>>  
>>  	if (dso) {
>> -		if (symbol_conf.show_kernel_path && dso->long_name)
>> +		if (prefer_dso_long_name(dso, print_off))
>>  			dsoname = dso->long_name;
>>  		else
>>  			dsoname = dso->name;
>> @@ -445,13 +452,21 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>>  	return fprintf(fp, "%s", dsoname);
>>  }
>>  
>> +size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>> +{
>> +	return __map__fprintf_dsoname(map, false, fp);
>> +}
>> +
>>  size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
>>  {
>> +	const struct dso *dso = map ? map__dso(map) : NULL;
>>  	int printed = 0;
>>  
>> +	if (print_off && (!dso || !dso__is_object_file(dso)))
>> +		print_off = false;
>>  	printed += fprintf(fp, " (");
>> -	printed += map__fprintf_dsoname(map, fp);
>> -	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
>> +	printed += __map__fprintf_dsoname(map, print_off, fp);
>> +	if (print_off)
>>  		printed += fprintf(fp, "+0x%" PRIx64, addr);
>>  	printed += fprintf(fp, ")");
>>  
>>
>>
> I am fine with this polish patch. So will send this standalone or append to this
> thread?

I will send my patch separately.

Adrian Hunter April 24, 2023, 5:25 a.m. UTC | #8

On 18/04/23 06:18, Changbin Du wrote:
> This adds a helper function map__fprintf_dsoname_dsoff() to print dsoname
> with optional dso offset.
> 
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Changbin Du <changbin.du@huawei.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/util/map.c | 13 +++++++++++++
>  tools/perf/util/map.h |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index d81b6ca18ee9..7da96b41100f 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -445,6 +445,19 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp)
>  	return fprintf(fp, "%s", dsoname);
>  }
>  
> +size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
> +{
> +	int printed = 0;
> +
> +	printed += fprintf(fp, " (");
> +	printed += map__fprintf_dsoname(map, fp);
> +	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
> +		printed += fprintf(fp, "+0x%" PRIx64, addr);
> +	printed += fprintf(fp, ")");
> +
> +	return printed;
> +}
> +
>  char *map__srcline(struct map *map, u64 addr, struct symbol *sym)
>  {
>  	if (map == NULL)
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index f89ab7c2d327..4cca211b6e66 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -175,6 +175,7 @@ static inline void __map__zput(struct map **map)
>  
>  size_t map__fprintf(struct map *map, FILE *fp);
>  size_t map__fprintf_dsoname(struct map *map, FILE *fp);
> +size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp);
>  char *map__srcline(struct map *map, u64 addr, struct symbol *sym);
>  int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
>  			 FILE *fp);

diff mbox series

Patch

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index d81b6ca18ee9..7da96b41100f 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -445,6 +445,19 @@  size_t map__fprintf_dsoname(struct map *map, FILE *fp)
 	return fprintf(fp, "%s", dsoname);
 }
 
+size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp)
+{
+	int printed = 0;
+
+	printed += fprintf(fp, " (");
+	printed += map__fprintf_dsoname(map, fp);
+	if (print_off && map && map__dso(map) && !map__dso(map)->kernel)
+		printed += fprintf(fp, "+0x%" PRIx64, addr);
+	printed += fprintf(fp, ")");
+
+	return printed;
+}
+
 char *map__srcline(struct map *map, u64 addr, struct symbol *sym)
 {
 	if (map == NULL)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index f89ab7c2d327..4cca211b6e66 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -175,6 +175,7 @@  static inline void __map__zput(struct map **map)
 
 size_t map__fprintf(struct map *map, FILE *fp);
 size_t map__fprintf_dsoname(struct map *map, FILE *fp);
+size_t map__fprintf_dsoname_dsoff(struct map *map, bool print_off, u64 addr, FILE *fp);
 char *map__srcline(struct map *map, u64 addr, struct symbol *sym);
 int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
 			 FILE *fp);