* [PATCH] perf diff: bug fix, donot overwrite valid build id
@ 2016-12-13 15:29 kan.liang
2016-12-19 14:53 ` Liang, Kan
2016-12-20 19:30 ` [tip:perf/urgent] perf diff: Do not " tip-bot for Kan Liang
0 siblings, 2 replies; 4+ messages in thread
From: kan.liang @ 2016-12-13 15:29 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: dima, jolsa, namhyung, andi, Kan Liang
From: Kan Liang <kan.liang@intel.com>
Fixes a perf diff regression issue which was introduced by
commit 5baecbcd9c9a ("perf symbols: we can now read separate debug-info
files based on a build ID")
The binary name could be same when perf diff different binaries. Build
id is used to distinguish between them.
However, the previous patch assumes the same binary name has same build
id. So it overwrites the build id according to the binary name,
regardless of whether the build id is set or not.
Check the has_build_id in dso__load. If the build id is already set, use
it.
Before the fix applies,
sudo ./perf diff 1.perf.data 2.perf.data
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... ................ .............................
#
99.83% -99.80% tchain_edit [.] f2
0.12% +99.81% tchain_edit [.] f3
0.02% -0.01% [ixgbe] [k] ixgbe_read_reg
After the fix applies,
sudo ./perf diff 1.perf.data 2.perf.data
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... ................ .............................
#
99.83% +0.10% tchain_edit [.] f3
0.12% -0.08% tchain_edit [.] f2
Signed-off-by: Kan Liang <kan.liang@intel.com>
---
tools/perf/util/symbol.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index df2482b..dc93940 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1459,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map)
* Read the build id if possible. This is required for
* DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
*/
- if (is_regular_file(dso->long_name) &&
+ if (!dso->has_build_id &&
+ is_regular_file(dso->long_name) &&
filename__read_build_id(dso->long_name, build_id, BUILD_ID_SIZE) > 0)
dso__set_build_id(dso, build_id);
--
2.4.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [PATCH] perf diff: bug fix, donot overwrite valid build id
2016-12-13 15:29 [PATCH] perf diff: bug fix, donot overwrite valid build id kan.liang
@ 2016-12-19 14:53 ` Liang, Kan
2016-12-19 18:37 ` acme
2016-12-20 19:30 ` [tip:perf/urgent] perf diff: Do not " tip-bot for Kan Liang
1 sibling, 1 reply; 4+ messages in thread
From: Liang, Kan @ 2016-12-19 14:53 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: dima, jolsa, namhyung, andi
Hi Arnaldo,
Ping
Are you OK with the fix?
Thanks,
Kan
>
> From: Kan Liang <kan.liang@intel.com>
>
> Fixes a perf diff regression issue which was introduced by commit
> 5baecbcd9c9a ("perf symbols: we can now read separate debug-info files
> based on a build ID")
>
> The binary name could be same when perf diff different binaries. Build id is
> used to distinguish between them.
> However, the previous patch assumes the same binary name has same
> build id. So it overwrites the build id according to the binary name,
> regardless of whether the build id is set or not.
>
> Check the has_build_id in dso__load. If the build id is already set, use it.
>
> Before the fix applies,
> sudo ./perf diff 1.perf.data 2.perf.data # Event 'cycles'
> #
> # Baseline Delta Shared Object Symbol
> # ........ ....... ................ .............................
> #
> 99.83% -99.80% tchain_edit [.] f2
> 0.12% +99.81% tchain_edit [.] f3
> 0.02% -0.01% [ixgbe] [k] ixgbe_read_reg
>
> After the fix applies,
> sudo ./perf diff 1.perf.data 2.perf.data # Event 'cycles'
> #
> # Baseline Delta Shared Object Symbol
> # ........ ....... ................ .............................
> #
> 99.83% +0.10% tchain_edit [.] f3
> 0.12% -0.08% tchain_edit [.] f2
>
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
> tools/perf/util/symbol.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index
> df2482b..dc93940 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1459,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map)
> * Read the build id if possible. This is required for
> * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
> */
> - if (is_regular_file(dso->long_name) &&
> + if (!dso->has_build_id &&
> + is_regular_file(dso->long_name) &&
> filename__read_build_id(dso->long_name, build_id,
> BUILD_ID_SIZE) > 0)
> dso__set_build_id(dso, build_id);
>
> --
> 2.4.3
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf diff: bug fix, donot overwrite valid build id
2016-12-19 14:53 ` Liang, Kan
@ 2016-12-19 18:37 ` acme
0 siblings, 0 replies; 4+ messages in thread
From: acme @ 2016-12-19 18:37 UTC (permalink / raw)
To: Liang, Kan; +Cc: linux-kernel, dima, jolsa, namhyung, andi
Em Mon, Dec 19, 2016 at 02:53:30PM +0000, Liang, Kan escreveu:
> Hi Arnaldo,
>
> Ping
>
> Are you OK with the fix?
Yeah, looks ok, will merge it.
- Arnaldo
> Thanks,
> Kan
>
> >
> > From: Kan Liang <kan.liang@intel.com>
> >
> > Fixes a perf diff regression issue which was introduced by commit
> > 5baecbcd9c9a ("perf symbols: we can now read separate debug-info files
> > based on a build ID")
> >
> > The binary name could be same when perf diff different binaries. Build id is
> > used to distinguish between them.
> > However, the previous patch assumes the same binary name has same
> > build id. So it overwrites the build id according to the binary name,
> > regardless of whether the build id is set or not.
> >
> > Check the has_build_id in dso__load. If the build id is already set, use it.
> >
> > Before the fix applies,
> > sudo ./perf diff 1.perf.data 2.perf.data # Event 'cycles'
> > #
> > # Baseline Delta Shared Object Symbol
> > # ........ ....... ................ .............................
> > #
> > 99.83% -99.80% tchain_edit [.] f2
> > 0.12% +99.81% tchain_edit [.] f3
> > 0.02% -0.01% [ixgbe] [k] ixgbe_read_reg
> >
> > After the fix applies,
> > sudo ./perf diff 1.perf.data 2.perf.data # Event 'cycles'
> > #
> > # Baseline Delta Shared Object Symbol
> > # ........ ....... ................ .............................
> > #
> > 99.83% +0.10% tchain_edit [.] f3
> > 0.12% -0.08% tchain_edit [.] f2
> >
> > Signed-off-by: Kan Liang <kan.liang@intel.com>
> > ---
> > tools/perf/util/symbol.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index
> > df2482b..dc93940 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1459,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map)
> > * Read the build id if possible. This is required for
> > * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
> > */
> > - if (is_regular_file(dso->long_name) &&
> > + if (!dso->has_build_id &&
> > + is_regular_file(dso->long_name) &&
> > filename__read_build_id(dso->long_name, build_id,
> > BUILD_ID_SIZE) > 0)
> > dso__set_build_id(dso, build_id);
> >
> > --
> > 2.4.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:perf/urgent] perf diff: Do not overwrite valid build id
2016-12-13 15:29 [PATCH] perf diff: bug fix, donot overwrite valid build id kan.liang
2016-12-19 14:53 ` Liang, Kan
@ 2016-12-20 19:30 ` tip-bot for Kan Liang
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Kan Liang @ 2016-12-20 19:30 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, namhyung, andi, dima, acme, tglx, jolsa, hpa,
linux-kernel, kan.liang
Commit-ID: ed6c166cc7dc329736cace3affd2df984fb22ec8
Gitweb: http://git.kernel.org/tip/ed6c166cc7dc329736cace3affd2df984fb22ec8
Author: Kan Liang <kan.liang@intel.com>
AuthorDate: Tue, 13 Dec 2016 10:29:44 -0500
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 20 Dec 2016 12:00:38 -0300
perf diff: Do not overwrite valid build id
Fixes a perf diff regression issue which was introduced by commit
5baecbcd9c9a ("perf symbols: we can now read separate debug-info files
based on a build ID")
The binary name could be same when perf diff different binaries. Build
id is used to distinguish between them.
However, the previous patch assumes the same binary name has same build
id. So it overwrites the build id according to the binary name,
regardless of whether the build id is set or not.
Check the has_build_id in dso__load. If the build id is already set, use
it.
Before the fix:
$ perf diff 1.perf.data 2.perf.data
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... ................ .............................
#
99.83% -99.80% tchain_edit [.] f2
0.12% +99.81% tchain_edit [.] f3
0.02% -0.01% [ixgbe] [k] ixgbe_read_reg
After the fix:
$ perf diff 1.perf.data 2.perf.data
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... ................ .............................
#
99.83% +0.10% tchain_edit [.] f3
0.12% -0.08% tchain_edit [.] f2
Signed-off-by: Kan Liang <kan.liang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
CC: Dima Kogan <dima@secretsauce.net>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Fixes: 5baecbcd9c9a ("perf symbols: we can now read separate debug-info files based on a build ID")
Link: http://lkml.kernel.org/r/1481642984-13593-1-git-send-email-kan.liang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/symbol.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index df2482b..dc93940 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1459,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map)
* Read the build id if possible. This is required for
* DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
*/
- if (is_regular_file(dso->long_name) &&
+ if (!dso->has_build_id &&
+ is_regular_file(dso->long_name) &&
filename__read_build_id(dso->long_name, build_id, BUILD_ID_SIZE) > 0)
dso__set_build_id(dso, build_id);
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-20 19:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-13 15:29 [PATCH] perf diff: bug fix, donot overwrite valid build id kan.liang
2016-12-19 14:53 ` Liang, Kan
2016-12-19 18:37 ` acme
2016-12-20 19:30 ` [tip:perf/urgent] perf diff: Do not " tip-bot for Kan Liang
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 NNTP newsgroup(s).