Create source symlink in perf object dir
PATCH v8 1/4 Create source symlink in perf object dir
Create a source symlink to the original source in the objdir. This is similar to what the main kernel build script does.
--- tools/perf/Makefile.perf | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/Makefile.perf +2 -0
@@ -163,6 +163,8 @@ ifneq ($(OUTPUT),) # for flex/bison parsers. VPATH += $(OUTPUT) export VPATH +# create symlink to the original source +SOURCE := $(shell ln -sf $(srctree)/tools/perf $(OUTPUT)/source) endif ifeq ($(V),1)
Signature · 1 line
2.45.2
PATCH v8 2/4 perf test: Support external tests for separate objdir
Extend the searching for the test files so that it works when running perf from a separate objdir, and also when the perf executable is symlinked.
----
v2: Fix string test (Namhyung)
Handle both in source and out of source builds.
--- tools/perf/tests/tests-scripts.c | 37 +++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)
tools/perf/tests/tests-scripts.c +34 -3
@@ -29,16 +29,47 @@static int shell_tests__dir_fd (void ) { -char path[PATH_MAX ], *exec_path; -static const char *const devel_dirs[] = {"./tools/perf/tests/shell" ,"./tests/shell" , }; +struct stat st; +char path[PATH_MAX ], path2[PATH_MAX ], *exec_path; +static const char *const devel_dirs[] = { +"./tools/perf/tests/shell" , +"./tests/shell" , +"./source/tests/shell" + }; +int fd; +char *p;for (size_t i =0 ; i <ARRAY_SIZE (devel_dirs); ++i) { -int fd =open (devel_dirs[i],O_PATH ); + fd =open (devel_dirs[i],O_PATH );if (fd >=0 )return fd; } +/* Use directory of executable */ +if (readlink ("/proc/self/exe" , path2,sizeof path2) <0 ) +return -1 ; +/* Follow another level of symlink if there */ +if (lstat (path2, &st) ==0 && (st.st_mode &S_IFMT ) ==S_IFLNK ) { +scnprintf (path,sizeof (path), path2); +if (readlink (path, path2,sizeof path2) <0 ) +return -1 ; + } +/* Get directory */ + p =strrchr (path2,'/' ); +if (p) + *p =0 ; +scnprintf (path,sizeof (path),"%s/tests/shell" , path2); + fd =open (path,O_PATH ); +if (fd >=0 ) +return fd; +if (p) + *p =0 ; +scnprintf (path,sizeof (path),"%s/source/tests/shell" , path2); + fd =open (path,O_PATH ); +if (fd >=0 ) +return fd; +/* Then installed path. */ exec_path =get_argv_exec_path ();scnprintf (path,sizeof (path),"%s/tests/shell" , exec_path);
Signature · 1 line
2.45.2
Namhyung Kim
maintainerRe: [PATCH v8 2/4] perf test: Support external tests for separate objdir
Quoted 51 lines · Andi Kleen
Extend the searching for the test files so that it works when running perf from a separate objdir, and also when the perf executable is symlinked.
Signed-off-by Andi Kleen <[email protected]>----
v2: Fix string test (Namhyung)
Handle both in source and out of source builds.--- tools/perf/tests/tests-scripts.c | 37 +++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)tools/perf/tests/tests-scripts.c +19
-3@@ -29,16 +29,47 @@static int shell_tests__dir_fd (void ) { -char path[PATH_MAX ], *exec_path; -static const char *const devel_dirs[] = {"./tools/perf/tests/shell" ,"./tests/shell" , }; +struct stat st; +char path[PATH_MAX ], path2[PATH_MAX ], *exec_path; +static const char *const devel_dirs[] = { +"./tools/perf/tests/shell" , +"./tests/shell" , +"./source/tests/shell" + }; +int fd; +char *p;for (size_t i =0 ; i <ARRAY_SIZE (devel_dirs); ++i) { -int fd =open (devel_dirs[i],O_PATH ); + fd =open (devel_dirs[i],O_PATH );if (fd >=0 )return fd; } +/* Use directory of executable */ +if (readlink ("/proc/self/exe" , path2,sizeof path2) <0 ) +return -1 ; +/* Follow another level of symlink if there */ +if (lstat (path2, &st) ==0 && (st.st_mode &S_IFMT ) ==S_IFLNK ) { +scnprintf (path,sizeof (path), path2); +if (readlink (path, path2,sizeof path2) <0 ) +return -1 ; + }
tools/perf/tests/tests-scripts.c line 67
Quoted 10 lines
tools/perf/tests/tests-scripts.c +10
-0+/* Get directory */ + p =strrchr (path2,'/' ); +if (p) + *p =0 ; +scnprintf (path,sizeof (path),"%s/tests/shell" , path2); + fd =open (path,O_PATH ); +if (fd >=0 ) +return fd; +if (p) + *p =0 ;
This part looks unnecessary.
Thanks,
Namhyung
Quoted 11 lines
tools/perf/tests/tests-scripts.c +5
-0+scnprintf (path,sizeof (path),"%s/source/tests/shell" , path2); + fd =open (path,O_PATH ); +if (fd >=0 ) +return fd; +/* Then installed path. */ exec_path =get_argv_exec_path ();scnprintf (path,sizeof (path),"%s/tests/shell" , exec_path);Signature · 1 line
2.45.2
PATCH v8 3/4 perf script: Fix perf script -F +metric
In reply to Andi KleenThis fixes a regression with perf script -F +metric originally caused by :
commit 37cc8ad77cf81f3ffd226856c367b0e15333a738
Author: Ian Rogers <[email protected]> Date: Sun Feb 19 01:28:46 2023 -0800
perf metric: Directly use counts rather than saved_value
In the perf script environment the evsel wouldn't allocate an aggr values array, which led to a -1 reference because the metric evaluation would try to reference NULL - 1 (for aggr_idx)
Give the perf script evsels a single CPU aggr setup. That's enough because the groups are always contiguous, so no need to store more than one CPU's worth of values.
Before
% perf record -e '{cycles,instructions}:S' perf bench mem memcpy % perf script -F +metric
Segmentation fault (core dumped)
After:
% perf record -e '{cycles,instructions}:S' perf bench mem memcpy
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.028 MB perf.data (90 samples) ] % perf script -F +metric
perf-exec 1847557 264658.180789: 3009 cycles: ffffffff990a579a native_write_msr+0xa ([kernel.kallsyms])
perf-exec 1847557 264658.180789: 382 instructions: ffffffff990a579a native_write_msr+0xa ([kernel.kallsyms])
perf-exec 1847557 264658.180789: metric: 0.13 insn per cycle...
----
v2: Reformat code
v3: Work around bogus warning
v4: Set up aggr map only for metrics case to keep perf stat record working
v5: Broken version
v6: Only set up limited aggregation mode with -F +metric. Add conflict checks with perf stat record files.
v7: Remove some unnecessary conflict checks. Fix buffer overflow. Minor cleanups.
v8: Add check for leader sampling. Update some comments.
--- tools/perf/builtin-script.c | 50 +++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-)
tools/perf/builtin-script.c +45 -5
@@ -335,7 +335,6 @@ struct evsel_script {FILE *fp;u64 samples;/* For metric output */ -u64 val;int gnum; };
@@ -2127,18 +2126,30 @@ static void perf_sample__fprint_metric(struct perf_script *script, };struct evsel *ev2;u64 val; +static int printed;if (!evsel->stats)evlist__alloc_stats (&stat_config, script->session->evlist,/*alloc_raw=*/ false);if (evsel_script (leader)->gnum++ ==0 )perf_stat__reset_shadow_stats (); - val = sample->period * evsel->scale; -evsel_script (evsel)->val = val; + val = sample->period; +if (!(evsel->core.attr.sample_type &PERF_SAMPLE_READ )) { +if (!printed) +fprintf (stderr,"perf script: -F metric requires {}:S for groups leader sampling\n" ); + printed =1 ; +return ; + } +/* +* Always use the first entry as storage because the leader sampling +* groups are contiguous and there's no need to handle multiple indexes +* for anything. +*/ + evsel->stats->aggr[0 ].counts.val = val;if (evsel_script (leader)->gnum == leader->core.nr_members) { for_each_group_member (ev2, leader) {perf_stat__print_shadow_stats (&stat_config, ev2, -evsel_script (ev2)->val, - sample->cpu, + evsel->stats->aggr[0 ].counts.val, +0 , &ctx,NULL ); }
@@ -2325,6 +2336,20 @@ static void process_event(struct perf_script *script,fflush (fp); } +static void check_metric_conflict (void ) +{ +int i; +/* +* Avoid conflict with the aggregation mode used for the metric printing. +*/ +for (i =0 ; i <OUTPUT_TYPE_MAX ; i++) { +if (output[i].fields &PERF_OUTPUT_METRIC ) { +fprintf (stderr,"perf stat record files are not supported with -F metric\n" ); +exit (1 ); + } + } +} +static struct scripting_ops *scripting_ops;static void __process_stat (struct evsel *counter,u64 tstamp)
@@ -2334,6 +2359,8 @@ static void __process_stat(struct evsel *counter, u64 tstamp)struct perf_cpu cpu;static int header_printed; +check_metric_conflict (); +if (!header_printed) {printf ("%3s %8s %15s %15s %15s %15s %s\n" ,"CPU" ,"THREAD" ,"VAL" ,"ENA" ,"RUN" ,"TIME" ,"EVENT" );
@@ -3725,6 +3752,8 @@ static int process_stat_config_event(struct perf_session *session __maybe_unused {perf_event__read_stat_config (&stat_config, &event->stat_config); +check_metric_conflict (); +/* * Aggregation modes are not used since post-processing scripts are * supposed to take care of such requirements
@@ -4088,6 +4117,17 @@ int cmd_script(int argc, const char **argv) argc =parse_options_subcommand (argc, argv, options, script_subcommands, script_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +for (i =0 ; i <OUTPUT_TYPE_MAX ; i++) { +if (output[i].fields &PERF_OUTPUT_METRIC ) { + stat_config.aggr_map =cpu_aggr_map__empty_new (1 ); + err = -ENOMEM ; +if (!stat_config.aggr_map) + goto out; + err =0 ; + stat_config.aggr_map->nr =1 ; +break ; + } + } if (symbol_conf.guestmount || symbol_conf.default_guest_vmlinux_name ||
Signature · 1 line
2.45.2
Namhyung Kim
maintainerRe: [PATCH v8 3/4] perf script: Fix perf script -F +metric
Quoted 68 lines · Andi Kleen
This fixes a regression with perf script -F +metric originally caused by :
commit
37cc8ad77cf81f3ffd226856c367b0e15333a738Author: Ian Rogers <[email protected]> Date: Sun Feb 19 01:28:46 2023 -0800perf metric: Directly use counts rather than
saved_valueIn the perf script environment the evsel wouldn't allocate an aggr values array, which led to a -1 reference because the metric evaluation would try to reference NULL - 1 (for
aggr_idx)Give the perf script evsels a single CPU aggr setup. That's enough because the groups are always contiguous, so no need to store more than one CPU's worth of values.
Before
% perf record -e '{cycles,instructions}:S' perf bench mem memcpy % perf script -F +metric
Segmentation fault (core dumped)After:
% perf record -e '{cycles,instructions}:S' perf bench mem memcpy
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.028 MB perf.data (90 samples) ] % perf script -F +metricperf-exec 1847557 264658.180789: 3009 cycles: ffffffff990a579a native_write_msr+0xa ([kernel.kallsyms]) perf-exec 1847557 264658.180789: 382 instructions: ffffffff990a579a native_write_msr+0xa ([kernel.kallsyms]) perf-exec 1847557 264658.180789: metric: 0.13 insn per cycle...
Fixes 37cc8ad77cf8 ("perf metric: Directly use counts rather ...")Signed-off-by Andi Kleen <[email protected]>----
v2: Reformat code
v3: Work around bogus warning
v4: Set up aggr map only for metrics case to keep perf stat record working
v5: Broken version
v6: Only set up limited aggregation mode with -F +metric. Add conflict checks with perf stat record files.
v7: Remove some unnecessary conflict checks. Fix buffer overflow. Minor cleanups.
v8: Add check for leader sampling. Update some comments.--- tools/perf/builtin-script.c | 50 +++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-)tools/perf/builtin-script.c +0
-1@@ -335,7 +335,6 @@ struct evsel_script {FILE *fp;u64 samples;/* For metric output */ -u64 val;int gnum; };@@ -2127,18 +2126,30 @@ static void perf_sample__fprint_metric(struct perf_script *script, };struct evsel *ev2;u64 val;
tools/perf/builtin-script.c static void perf_sample__fprint_metric(struct perf_script *script, · line 2136
Quoted 10 lines
tools/perf/builtin-script.c +3
-2+static int printed;if (!evsel->stats)evlist__alloc_stats (&stat_config, script->session->evlist,/*alloc_raw=*/ false);if (evsel_script (leader)->gnum++ ==0 )perf_stat__reset_shadow_stats (); - val = sample->period * evsel->scale; -evsel_script (evsel)->val = val; + val = sample->period; +if (!(evsel->core.attr.sample_type &PERF_SAMPLE_READ )) {
The correct check would need PERF_FORMAT_GROUP in attr.read_format for the leader event, and no attr.sample_period for member events. But I guess practically it's enough to check the leader only.
Thanks,
Namhyung
Quoted 81 lines
tools/perf/builtin-script.c +42
-2+if (!printed) +fprintf (stderr,"perf script: -F metric requires {}:S for groups leader sampling\n" ); + printed =1 ; +return ; + } +/* +* Always use the first entry as storage because the leader sampling +* groups are contiguous and there's no need to handle multiple indexes +* for anything. +*/ + evsel->stats->aggr[0 ].counts.val = val;if (evsel_script (leader)->gnum == leader->core.nr_members) { for_each_group_member (ev2, leader) {perf_stat__print_shadow_stats (&stat_config, ev2, -evsel_script (ev2)->val, - sample->cpu, + evsel->stats->aggr[0 ].counts.val, +0 , &ctx,NULL ); }@@ -2325,6 +2336,20 @@ static void process_event(struct perf_script *script,fflush (fp); } +static void check_metric_conflict (void ) +{ +int i; +/* +* Avoid conflict with the aggregation mode used for the metric printing. +*/ +for (i =0 ; i <OUTPUT_TYPE_MAX ; i++) { +if (output[i].fields &PERF_OUTPUT_METRIC ) { +fprintf (stderr,"perf stat record files are not supported with -F metric\n" ); +exit (1 ); + } + } +} +static struct scripting_ops *scripting_ops;static void __process_stat (struct evsel *counter,u64 tstamp)@@ -2334,6 +2359,8 @@ static void __process_stat(struct evsel *counter, u64 tstamp)struct perf_cpu cpu;static int header_printed; +check_metric_conflict (); +if (!header_printed) {printf ("%3s %8s %15s %15s %15s %15s %s\n" ,"CPU" ,"THREAD" ,"VAL" ,"ENA" ,"RUN" ,"TIME" ,"EVENT" );@@ -3725,6 +3752,8 @@ static int process_stat_config_event(struct perf_session *session __maybe_unused {perf_event__read_stat_config (&stat_config, &event->stat_config); +check_metric_conflict (); +/* * Aggregation modes are not used since post-processing scripts are * supposed to take care of such requirements @@ -4088,6 +4117,17 @@ int cmd_script(int argc, const char **argv) argc =parse_options_subcommand (argc, argv, options, script_subcommands, script_usage,PARSE_OPT_STOP_AT_NON_OPTION ); +for (i =0 ; i <OUTPUT_TYPE_MAX ; i++) { +if (output[i].fields &PERF_OUTPUT_METRIC ) { + stat_config.aggr_map =cpu_aggr_map__empty_new (1 ); + err = -ENOMEM ; +if (!stat_config.aggr_map) + goto out; + err =0 ; + stat_config.aggr_map->nr =1 ; +break ; + } + } if (symbol_conf.guestmount || symbol_conf.default_guest_vmlinux_name ||Signature · 1 line
2.45.2
PATCH v8 4/4 Add a test case for perf script -F +metric
In reply to Andi KleenJust a simple test
----
v2: Avoid bashisms. Use noploop
v3: Avoid false positive in shellcheck
v4: Use :S in the test (Ian, Namhyung)
--- tools/perf/tests/shell/script.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
tools/perf/tests/shell/script.sh +14 -0
@@ -7,6 +7,7 @@ set -e temp_dir=$(mktemp -d /tmp/perf-test-script.XXXXXXXXXX) perfdatafile="${temp_dir}/perf.data" +scriptoutput="${temp_dir}/script" db_test="${temp_dir}/db_test.py" err=0
@@ -88,8 +89,21 @@ test_parallel_perf() echo "parallel-perf test [Success]" } +test_metric() +{ + echo "script metric test" + if ! perf list | grep -q cycles ; then return ; fi + if ! perf list | grep -q instructions ; then return ; fi + perf record -e '{cycles,instructions}:S' -o "${perfdatafile}" perf test -w noploop + perf script -i "${perfdatafile}" -F +metric > $scriptoutput + test "`grep -c metric $scriptoutput`" -gt 5 + grep metric $scriptoutput | head + echo "script metric test [Success]" +} + test_db test_parallel_perf +test_metric cleanup
Signature · 1 line
2.45.2
Namhyung Kim
maintainerRe: [PATCH v8 4/4] Add a test case for perf script -F +metric
Quoted 1 line · Andi Kleen
Just a simple test
Can you please briefly describe what the test does? Ideally with an example output with 'perf test -vv'.
Thanks,
Namhyung
Quoted 49 lines
Signed-off-by Andi Kleen <[email protected]>----
v2: Avoid bashisms. Use noploop
v3: Avoid false positive in shellcheck
v4: Use :S in the test (Ian, Namhyung)--- tools/perf/tests/shell/script.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+)tools/perf/tests/shell/script.sh +14
-0@@ -7,6 +7,7 @@ set -e temp_dir=$(mktemp -d /tmp/perf-test-script.XXXXXXXXXX) perfdatafile="${temp_dir}/perf.data" +scriptoutput="${temp_dir}/script" db_test="${temp_dir}/db_test.py" err=0@@ -88,8 +89,21 @@ test_parallel_perf() echo "parallel-perf test [Success]" } +test_metric() +{ + echo "script metric test" + if ! perf list | grep -q cycles ; then return ; fi + if ! perf list | grep -q instructions ; then return ; fi + perf record -e '{cycles,instructions}:S' -o "${perfdatafile}" perf test -w noploop + perf script -i "${perfdatafile}" -F +metric > $scriptoutput + test "`grep -c metric $scriptoutput`" -gt 5 + grep metric $scriptoutput | head + echo "script metric test [Success]" +} + test_db test_parallel_perf +test_metric cleanupSignature · 1 line
2.45.2