perf report: Fix some option handling on --stdio
Namhyung Kim
maintainerPATCH 1/1 perf report: Fix some option handling on --stdio
There's a bug that perf report sometimes ignore some options on --stdio output. This bug is triggered only if a related config variable is set. For example, let's assume we have a following config file.
$ cat ~/.perfconfig
[call-graph]
print-type = graph
[hist]
percentage = absoluteThen, following perf config will not honor some options.
$ perf record -ag sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.199 MB perf.data (77 samples) ]
$ perf report -g none --stdio
# To display the perf.data header info, please use --header/--header-only options.
#
# Samples: 77 of event 'cycles'
# Event count (approx.): 25425383
#
# Overhead Command Shared Object Symbol
# ........ ............... ....................... ..............
#
16.34% swapper [kernel.vmlinux] [k] intel_idle
|
---intel_idle
cpuidle_enter_state
cpuidle_enter
cpu_startup_entry
...With '-g none' option, it should not show callchains, but it still shows callchains. However it works as expected on --tui output.
Similarly, '--percentage relative' option is not work and still shows a absolute percentage values.
Looking at the source, I found that those setting were overwritten by config variables when setup_pager() called. The setup_pager() is to start a pager process so that it can manage long lines of output on the stdio mode. But as it calls the perf_config() after parsing arguments, the settings were overwritten regardless of command line options.
The reason it calls perf_config() is to find the 'pager_program' which might be set by a config variable, I guess. However current perf code does not provide the config variable for it, so it's just meaningless IMHO. Eliminating the call makes the option working as expected.
--- tools/perf/util/cache.h | 1 - tools/perf/util/environment.c | 1 - tools/perf/util/pager.c | 5 ----- 3 files changed, 7 deletions(-)
tools/perf/util/cache.h +0 -1
@@ -30,7 +30,6 @@ extern const char *perf_config_dirname(const char *, const char *);/* pager.c */ extern void setup_pager (void ); -extern const char *pager_program;extern int pager_in_use (void );extern int pager_use_color;
tools/perf/util/environment.c +0 -1
@@ -5,5 +5,4 @@*/ #include "cache.h" -const char *pager_program;int pager_use_color =1 ;
tools/perf/util/pager.c +0 -5
@@ -50,11 +50,6 @@ void setup_pager(void)if (!isatty (1 ))return ; -if (!pager) { -if (!pager_program) -perf_config (perf_default_config,NULL ); - pager = pager_program; - }if (!pager) pager =getenv ("PAGER" );if (!(pager ||access ("/usr/bin/pager" ,X_OK )))
Signature · 1 line
2.4.0
Arnaldo Carvalho de Melo
maintainerEm Thu, May 14, 2015 at 12:03:26AM +0900, Namhyung Kim escreveu:
Quoted 3 lines
There's a bug that perf report sometimes ignore some options on --stdio output. This bug is triggered only if a related config variable is set. For example, let's assume we have a following config file.
Testing, please next time indicate against what branch this is to be applied, as I thought about perf/core, but it doesn't apply there, so tried perf/urgent, where it applies.
- Arnaldo
Quoted 94 lines
$ cat ~/.perfconfig [call-graph] print-type = graph [hist] percentage = absoluteThen, following perf config will not honor some options.
$ perf record -ag sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.199 MB perf.data (77 samples) ]$ perf report -g none --stdio # To display the perf.data header info, please use --header/--header-only options. # # Samples: 77 of event 'cycles' # Event count (approx.): 25425383 # # Overhead Command Shared Object Symbol # ........ ............... ....................... .............. # 16.34% swapper [kernel.vmlinux] [k] intel_idle | ---intel_idle cpuidle_enter_state cpuidle_enter cpu_startup_entry ...With '-g none' option, it should not show callchains, but it still shows callchains. However it works as expected on --tui output.
Similarly, '--percentage relative' option is not work and still shows a absolute percentage values.
Looking at the source, I found that those setting were overwritten by config variables when
setup_pager()called. Thesetup_pager()is to start a pager process so that it can manage long lines of output on the stdio mode. But as it calls theperf_config()after parsing arguments, the settings were overwritten regardless of command line options.The reason it calls
perf_config()is to find the 'pager_program' which might be set by a config variable, I guess. However current perf code does not provide the config variable for it, so it's just meaningless IMHO. Eliminating the call makes the option working as expected.Cc Taeung Song <[email protected]>Signed-off-by Namhyung Kim <[email protected]>--- tools/perf/util/cache.h | 1 - tools/perf/util/environment.c | 1 - tools/perf/util/pager.c | 5 ----- 3 files changed, 7 deletions(-)tools/perf/util/cache.h +0
-1@@ -30,7 +30,6 @@ extern const char *perf_config_dirname(const char *, const char *);/* pager.c */ extern void setup_pager (void ); -extern const char *pager_program;extern int pager_in_use (void );extern int pager_use_color;tools/perf/util/environment.c +0
-1@@ -5,5 +5,4 @@*/ #include "cache.h" -const char *pager_program;int pager_use_color =1 ;tools/perf/util/pager.c +0
-5@@ -50,11 +50,6 @@ void setup_pager(void)if (!isatty (1 ))return ; -if (!pager) { -if (!pager_program) -perf_config (perf_default_config,NULL ); - pager = pager_program; - }if (!pager) pager =getenv ("PAGER" );if (!(pager ||access ("/usr/bin/pager" ,X_OK )))Signature · 1 line
2.4.0
Namhyung Kim
maintainerHi Arnaldo,
Quoted 8 lines · Arnaldo Carvalho de Melo
Em Thu, May 14, 2015 at 12:03:26AM +0900, Namhyung Kim escreveu:
Quoted 3 lines
There's a bug that perf report sometimes ignore some options on --stdio output. This bug is triggered only if a related config variable is set. For example, let's assume we have a following config file.
Testing, please next time indicate against what branch this is to be applied, as I thought about perf/core, but it doesn't apply there, so tried perf/urgent, where it applies.
Oh, sorry about that. I actually intended to apply it to the perf/core but forgot to rebase to the current code. It's fortunate that it can be applied to perf/urgent cleanly though. :)
I'll add an indication next time if patch should go to other branch than perf/core.
Thanks,
Namhyung
tip-bot for Namhyung Kim
merge botperf report: Fix some option handling on --stdio
Commit-ID: 4fd113b5ce803da0b8fa0494513bedfdf2feb483 Gitweb: http://git.kernel.org/tip/4fd113b5ce803da0b8fa0494513bedfdf2feb483 Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 14 May 2015 00:03:26 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 14 May 2015 10:05:22 -0300
perf report: Fix some option handling on --stdio
There's a bug that perf report sometimes ignore some options on --stdio output. This bug is triggered only if a related config variable is set. For example, let's assume we have a following config file.
$ cat ~/.perfconfig
[call-graph]
print-type = graph
[hist]
percentage = absoluteThen, following perf config will not honor some options.
$ perf record -ag sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.199 MB perf.data (77 samples) ]
$ perf report -g none --stdio
# To display the perf.data header info, please use --header/--header-only options.
#
# Samples: 77 of event 'cycles'
# Event count (approx.): 25425383
#
# Overhead Command Shared Object Symbol
# ........ ............... ....................... ..............
#
16.34% swapper [kernel.vmlinux] [k] intel_idle
|
---intel_idle
cpuidle_enter_state
cpuidle_enter
cpu_startup_entry
...With '-g none' option, it should not show callchains, but it still shows callchains. However it works as expected on --tui output.
Similarly, '--percentage relative' option is not work and still shows a absolute percentage values.
Looking at the source, I found that those setting were overwritten by config variables when setup_pager() called. The setup_pager() is to start a pager process so that it can manage long lines of output on the stdio mode. But as it calls the perf_config() after parsing arguments, the settings were overwritten regardless of command line options.
The reason it calls perf_config() is to find the 'pager_program' which might be set by a config variable, I guess. However current perf code does not provide the config variable for it, so it's just meaningless IMHO. Eliminating the call makes the option working as expected.
--- tools/perf/util/cache.h | 1 - tools/perf/util/environment.c | 1 - tools/perf/util/pager.c | 5 ----- 3 files changed, 7 deletions(-)
tools/perf/util/cache.h +0 -1
@@ -30,7 +30,6 @@ extern const char *perf_config_dirname(const char *, const char *);/* pager.c */ extern void setup_pager (void ); -extern const char *pager_program;extern int pager_in_use (void );extern int pager_use_color;
tools/perf/util/environment.c +0 -1
@@ -5,5 +5,4 @@*/ #include "cache.h" -const char *pager_program;int pager_use_color =1 ;
tools/perf/util/pager.c +0 -5
@@ -50,11 +50,6 @@ void setup_pager(void)if (!isatty (1 ))return ; -if (!pager) { -if (!pager_program) -perf_config (perf_default_config,NULL ); - pager = pager_program; - }if (!pager) pager =getenv ("PAGER" );if (!(pager ||access ("/usr/bin/pager" ,X_OK )))