[PATCH v6 0/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[PATCH v6 0/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Yang Jihong]

User space tasks can migrate between CPUs, track sideband events for all
CPUs.

The specific scenarios are as follows:

         CPU0                                 CPU1
  perf record -C 0 start
                              taskA starts to be created and executed
                                -> PERF_RECORD_COMM and PERF_RECORD_MMAP
                                   events only deliver to CPU1
                              ......
                                |
                          migrate to CPU0
                                |
  Running on CPU0    <----------/
  ...

  perf record -C 0 stop

Now perf samples the PC of taskA. However, perf does not record the
PERF_RECORD_COMM and PERF_RECORD_COMM events of taskA.
Therefore, the comm and symbols of taskA cannot be parsed.

The sys_perf_event_open invoked is as follows:

  # perf --debug verbose=3 record -e cpu-clock -C 1 true
  <SNIP>
  Opening: cpu-clock
  ------------------------------------------------------------
  perf_event_attr:
    type                             1 (PERF_TYPE_SOFTWARE)
    size                             136
    config                           0 (PERF_COUNT_SW_CPU_CLOCK)
    { sample_period, sample_freq }   4000
    sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
    read_format                      ID|LOST
    disabled                         1
    inherit                          1
    freq                             1
    sample_id_all                    1
    exclude_guest                    1
  ------------------------------------------------------------
  sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
  Opening: dummy:u
  ------------------------------------------------------------
  perf_event_attr:
    type                             1 (PERF_TYPE_SOFTWARE)
    size                             136
    config                           0x9 (PERF_COUNT_SW_DUMMY)
    { sample_period, sample_freq }   1
    sample_type                      IP|TID|TIME|CPU|IDENTIFIER
    read_format                      ID|LOST
    inherit                          1
    exclude_kernel                   1
    exclude_hv                       1
    mmap                             1
    comm                             1
    task                             1
    sample_id_all                    1
    exclude_guest                    1
    mmap2                            1
    comm_exec                        1
    ksymbol                          1
    bpf_event                        1
  ------------------------------------------------------------
  sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
  sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
  sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
  sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
  sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
  sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
  sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
  sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
  <SNIP>

Changes since_v5:
 - No code changes.
 - Detailed commit message of patch3.
 - Add Acked-by and Tested-by tags from Adrian Hunter.

Changes since_v4:
 - Simplify check code for record__tracking_system_wide().
 - Add perf attr test result to commit message for patch 7.

Changes since_v3:
 - Check fall_kernel, all_user, and dummy or exclude_user when determining
   whether system wide is required.

Changes since_v2:
 - Rename record_tracking.sh to record_sideband.sh in tools/perf/tests/shell.
 - Remove "perf evlist: Skip dummy event sample_type check for evlist_config" patch.
 - Add opts->all_kernel check in record__config_tracking_events().
 - Add perf_event_attr test for record selected CPUs exclude_user.
 - Update base-record & system-wide-dummy sample_type attr expected values for test-record-C0.

Changes since v1:
 - Add perf_evlist__go_system_wide() via internal/evlist.h instead of
   exporting perf_evlist__propagate_maps().
 - Use evlist__add_aux_dummy() instead of evlist__add_dummy() in
   evlist__findnew_tracking_event().
 - Add a parameter in evlist__findnew_tracking_event() to deal with
   system_wide inside.
 - Add sideband for all CPUs when tracing selected CPUs comments on
   the perf record man page.
 - Use "sideband events" instead of "tracking events".
 - Adjust the patches Sequence.
 - Add patch5 to skip dummy event sample_type check for evlist_config.
 - Add patch6 to update system-wide-dummy attr values for perf test.

Yang Jihong (7):
  perf evlist: Add perf_evlist__go_system_wide() helper
  perf evlist: Add evlist__findnew_tracking_event() helper
  perf record: Move setting dummy tracking before
    record__init_thread_masks()
  perf record: Track sideband events for all CPUs when tracing selected
    CPUs
  perf test: Update base-record & system-wide-dummy attr expected values
    for test-record-C0
  perf test: Add test case for record sideband events
  perf test: Add perf_event_attr test for record selected CPUs
    exclude_user

 tools/lib/perf/evlist.c                       |   9 ++
 tools/lib/perf/include/internal/evlist.h      |   2 +
 tools/perf/Documentation/perf-record.txt      |   3 +
 tools/perf/builtin-record.c                   | 106 +++++++++++++-----
 tools/perf/tests/attr/system-wide-dummy       |  14 ++-
 tools/perf/tests/attr/test-record-C0          |   4 +-
 .../perf/tests/attr/test-record-C0-all-kernel |  32 ++++++
 tools/perf/tests/shell/record_sideband.sh     |  44 ++++++++
 tools/perf/util/evlist.c                      |  18 +++
 tools/perf/util/evlist.h                      |   1 +
 10 files changed, 198 insertions(+), 35 deletions(-)
 create mode 100644 tools/perf/tests/attr/test-record-C0-all-kernel
 create mode 100755 tools/perf/tests/shell/record_sideband.sh

-- 
2.30.GIT

[PATCH v6 1/7] perf evlist: Add perf_evlist__go_system_wide() helper

[Yang Jihong]

For dummy events that keep tracking, we may need to modify its cpu_maps.
For example, change the cpu_maps to record sideband events for all CPUS.
Add perf_evlist__go_system_wide() helper to support this scenario.

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/lib/perf/evlist.c                  | 9 +++++++++
 tools/lib/perf/include/internal/evlist.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index b8b066d0dc5e..3acbbccc1901 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -738,3 +738,12 @@ int perf_evlist__nr_groups(struct perf_evlist *evlist)
 	}
 	return nr_groups;
 }
+
+void perf_evlist__go_system_wide(struct perf_evlist *evlist, struct perf_evsel *evsel)
+{
+	if (!evsel->system_wide) {
+		evsel->system_wide = true;
+		if (evlist->needs_map_propagation)
+			__perf_evlist__propagate_maps(evlist, evsel);
+	}
+}
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index 3339bc2f1765..d86ffe8ed483 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -135,4 +135,6 @@ int perf_evlist__id_add_fd(struct perf_evlist *evlist,
 void perf_evlist__reset_id_hash(struct perf_evlist *evlist);
 
 void __perf_evlist__set_leader(struct list_head *list, struct perf_evsel *leader);
+
+void perf_evlist__go_system_wide(struct perf_evlist *evlist, struct perf_evsel *evsel);
 #endif /* __LIBPERF_INTERNAL_EVLIST_H */
-- 
2.30.GIT

[PATCH v6 2/7] perf evlist: Add evlist__findnew_tracking_event() helper

[Yang Jihong]

Currently, intel-bts, intel-pt, and arm-spe may add a dummy event for
tracking to the evlist. We may need to search for the dummy event for
some settings. Therefore, add evlist__findnew_tracking_event() helper.

evlist__findnew_tracking_event() also deal with system_wide maps if
system_wide is true.

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 11 +++--------
 tools/perf/util/evlist.c    | 18 ++++++++++++++++++
 tools/perf/util/evlist.h    |  1 +
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 34bb31f08bb5..12edad8392cc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1293,14 +1293,9 @@ static int record__open(struct record *rec)
 	 */
 	if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
 	    perf_pmus__num_core_pmus() > 1) {
-		pos = evlist__get_tracking_event(evlist);
-		if (!evsel__is_dummy_event(pos)) {
-			/* Set up dummy event. */
-			if (evlist__add_dummy(evlist))
-				return -ENOMEM;
-			pos = evlist__last(evlist);
-			evlist__set_tracking_event(evlist, pos);
-		}
+		pos = evlist__findnew_tracking_event(evlist, false);
+		if (!pos)
+			return -ENOMEM;
 
 		/*
 		 * Enable the dummy event when the process is forked for
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 7ef43f72098e..25c3ebe2c2f5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1694,6 +1694,24 @@ void evlist__set_tracking_event(struct evlist *evlist, struct evsel *tracking_ev
 	tracking_evsel->tracking = true;
 }
 
+struct evsel *evlist__findnew_tracking_event(struct evlist *evlist, bool system_wide)
+{
+	struct evsel *evsel;
+
+	evsel = evlist__get_tracking_event(evlist);
+	if (!evsel__is_dummy_event(evsel)) {
+		evsel = evlist__add_aux_dummy(evlist, system_wide);
+		if (!evsel)
+			return NULL;
+
+		evlist__set_tracking_event(evlist, evsel);
+	} else if (system_wide) {
+		perf_evlist__go_system_wide(&evlist->core, &evsel->core);
+	}
+
+	return evsel;
+}
+
 struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str)
 {
 	struct evsel *evsel;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 664c6bf7b3e0..98e7ddb2bd30 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -387,6 +387,7 @@ bool evlist_cpu_iterator__end(const struct evlist_cpu_iterator *evlist_cpu_itr);
 
 struct evsel *evlist__get_tracking_event(struct evlist *evlist);
 void evlist__set_tracking_event(struct evlist *evlist, struct evsel *tracking_evsel);
+struct evsel *evlist__findnew_tracking_event(struct evlist *evlist, bool system_wide);
 
 struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str);
 
-- 
2.30.GIT

[PATCH v6 3/7] perf record: Move setting dummy tracking before record__init_thread_masks()

[Yang Jihong]

User space tasks can migrate between CPUs, so when tracing selected CPUs,
sideband for all CPUs is needed. In this case set the cpu map of the evsel
to all online CPUs. This may modify the original cpu map of the evlist.
Therefore, need to check whether the preceding scenario exists before
record__init_thread_masks().
Dummy tracking has been set in record__open(), move it before
record__init_thread_masks() and add a helper for unified processing.

The sys_perf_event_open invoked is as follows:

  # perf --debug verbose=3 record -e cpu-clock -D 100 true
  <SNIP>
  Opening: cpu-clock
  ------------------------------------------------------------
  perf_event_attr:
    type                             1 (PERF_TYPE_SOFTWARE)
    size                             136
    config                           0 (PERF_COUNT_SW_CPU_CLOCK)
    { sample_period, sample_freq }   4000
    sample_type                      IP|TID|TIME|PERIOD|IDENTIFIER
    read_format                      ID|LOST
    disabled                         1
    inherit                          1
    freq                             1
    sample_id_all                    1
    exclude_guest                    1
  ------------------------------------------------------------
  sys_perf_event_open: pid 10318  cpu 0  group_fd -1  flags 0x8 = 5
  sys_perf_event_open: pid 10318  cpu 1  group_fd -1  flags 0x8 = 6
  sys_perf_event_open: pid 10318  cpu 2  group_fd -1  flags 0x8 = 7
  sys_perf_event_open: pid 10318  cpu 3  group_fd -1  flags 0x8 = 9
  sys_perf_event_open: pid 10318  cpu 4  group_fd -1  flags 0x8 = 10
  sys_perf_event_open: pid 10318  cpu 5  group_fd -1  flags 0x8 = 11
  sys_perf_event_open: pid 10318  cpu 6  group_fd -1  flags 0x8 = 12
  sys_perf_event_open: pid 10318  cpu 7  group_fd -1  flags 0x8 = 13
  Opening: dummy:u
  ------------------------------------------------------------
  perf_event_attr:
    type                             1 (PERF_TYPE_SOFTWARE)
    size                             136
    config                           0x9 (PERF_COUNT_SW_DUMMY)
    { sample_period, sample_freq }   1
    sample_type                      IP|TID|TIME|IDENTIFIER
    read_format                      ID|LOST
    disabled                         1
    inherit                          1
    exclude_kernel                   1
    exclude_hv                       1
    mmap                             1
    comm                             1
    enable_on_exec                   1
    task                             1
    sample_id_all                    1
    exclude_guest                    1
    mmap2                            1
    comm_exec                        1
    ksymbol                          1
    bpf_event                        1
  ------------------------------------------------------------
  sys_perf_event_open: pid 10318  cpu 0  group_fd -1  flags 0x8 = 14
  sys_perf_event_open: pid 10318  cpu 1  group_fd -1  flags 0x8 = 15
  sys_perf_event_open: pid 10318  cpu 2  group_fd -1  flags 0x8 = 16
  sys_perf_event_open: pid 10318  cpu 3  group_fd -1  flags 0x8 = 17
  sys_perf_event_open: pid 10318  cpu 4  group_fd -1  flags 0x8 = 18
  sys_perf_event_open: pid 10318  cpu 5  group_fd -1  flags 0x8 = 19
  sys_perf_event_open: pid 10318  cpu 6  group_fd -1  flags 0x8 = 20
  sys_perf_event_open: pid 10318  cpu 7  group_fd -1  flags 0x8 = 21
  <SNIP>

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 59 +++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 12edad8392cc..4ee94058028f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -906,6 +906,37 @@ static int record__config_off_cpu(struct record *rec)
 	return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
 }
 
+static int record__config_tracking_events(struct record *rec)
+{
+	struct record_opts *opts = &rec->opts;
+	struct evlist *evlist = rec->evlist;
+	struct evsel *evsel;
+
+	/*
+	 * For initial_delay, system wide or a hybrid system, we need to add a
+	 * dummy event so that we can track PERF_RECORD_MMAP to cover the delay
+	 * of waiting or event synthesis.
+	 */
+	if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
+	    perf_pmus__num_core_pmus() > 1) {
+		evsel = evlist__findnew_tracking_event(evlist, false);
+		if (!evsel)
+			return -ENOMEM;
+
+		/*
+		 * Enable the dummy event when the process is forked for
+		 * initial_delay, immediately for system wide.
+		 */
+		if (opts->target.initial_delay && !evsel->immediate &&
+		    !target__has_cpu(&opts->target))
+			evsel->core.attr.enable_on_exec = 1;
+		else
+			evsel->immediate = 1;
+	}
+
+	return 0;
+}
+
 static bool record__kcore_readable(struct machine *machine)
 {
 	char kcore[PATH_MAX];
@@ -1286,28 +1317,6 @@ static int record__open(struct record *rec)
 	struct record_opts *opts = &rec->opts;
 	int rc = 0;
 
-	/*
-	 * For initial_delay, system wide or a hybrid system, we need to add a
-	 * dummy event so that we can track PERF_RECORD_MMAP to cover the delay
-	 * of waiting or event synthesis.
-	 */
-	if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
-	    perf_pmus__num_core_pmus() > 1) {
-		pos = evlist__findnew_tracking_event(evlist, false);
-		if (!pos)
-			return -ENOMEM;
-
-		/*
-		 * Enable the dummy event when the process is forked for
-		 * initial_delay, immediately for system wide.
-		 */
-		if (opts->target.initial_delay && !pos->immediate &&
-		    !target__has_cpu(&opts->target))
-			pos->core.attr.enable_on_exec = 1;
-		else
-			pos->immediate = 1;
-	}
-
 	evlist__config(evlist, opts, &callchain_param);
 
 	evlist__for_each_entry(evlist, pos) {
@@ -4190,6 +4199,12 @@ int cmd_record(int argc, const char **argv)
 		goto out;
 	}
 
+	err = record__config_tracking_events(rec);
+	if (err) {
+		pr_err("record__config_tracking_events failed, error %d\n", err);
+		goto out;
+	}
+
 	err = record__init_thread_masks(rec);
 	if (err) {
 		pr_err("Failed to initialize parallel data streaming masks\n");
-- 
2.30.GIT

[PATCH v6 4/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Yang Jihong]

User space tasks can migrate between CPUs, we need to track side-band
events for all CPUs.

The specific scenarios are as follows:

         CPU0                                 CPU1
  perf record -C 0 start
                              taskA starts to be created and executed
                                -> PERF_RECORD_COMM and PERF_RECORD_MMAP
                                   events only deliver to CPU1
                              ......
                                |
                          migrate to CPU0
                                |
  Running on CPU0    <----------/
  ...

  perf record -C 0 stop

Now perf samples the PC of taskA. However, perf does not record the
PERF_RECORD_COMM and PERF_RECORD_MMAP events of taskA.
Therefore, the comm and symbols of taskA cannot be parsed.

The solution is to record sideband events for all CPUs when tracing
selected CPUs. Because this modifies the default behavior, add related
comments to the perf record man page.

The sys_perf_event_open invoked is as follows:

  # perf --debug verbose=3 record -e cpu-clock -C 1 true
  <SNIP>
  Opening: cpu-clock
  ------------------------------------------------------------
  perf_event_attr:
    type                             1 (PERF_TYPE_SOFTWARE)
    size                             136
    config                           0 (PERF_COUNT_SW_CPU_CLOCK)
    { sample_period, sample_freq }   4000
    sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
    read_format                      ID|LOST
    disabled                         1
    inherit                          1
    freq                             1
    sample_id_all                    1
    exclude_guest                    1
  ------------------------------------------------------------
  sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
  Opening: dummy:u
  ------------------------------------------------------------
  perf_event_attr:
    type                             1 (PERF_TYPE_SOFTWARE)
    size                             136
    config                           0x9 (PERF_COUNT_SW_DUMMY)
    { sample_period, sample_freq }   1
    sample_type                      IP|TID|TIME|CPU|IDENTIFIER
    read_format                      ID|LOST
    inherit                          1
    exclude_kernel                   1
    exclude_hv                       1
    mmap                             1
    comm                             1
    task                             1
    sample_id_all                    1
    exclude_guest                    1
    mmap2                            1
    comm_exec                        1
    ksymbol                          1
    bpf_event                        1
  ------------------------------------------------------------
  sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
  sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
  sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
  sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
  sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
  sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
  sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
  sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
  <SNIP>

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt |  3 ++
 tools/perf/builtin-record.c              | 44 +++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d5217be012d7..1889f66addf2 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -374,6 +374,9 @@ comma-separated list with no space: 0,1. Ranges of CPUs are specified with -: 0-
 In per-thread mode with inheritance mode on (default), samples are captured only when
 the thread executes on the designated CPUs. Default is to monitor all CPUs.
 
+User space tasks can migrate between CPUs, so when tracing selected CPUs,
+a dummy event is created to track sideband for all CPUs.
+
 -B::
 --no-buildid::
 Do not save the build ids of binaries in the perf.data files. This skips
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4ee94058028f..ae2e21b945fa 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -906,10 +906,44 @@ static int record__config_off_cpu(struct record *rec)
 	return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
 }
 
+static bool record__tracking_system_wide(struct record *rec)
+{
+	struct record_opts *opts = &rec->opts;
+	struct evlist *evlist = rec->evlist;
+	struct evsel *evsel;
+
+	/*
+	 * If all (non-dummy) evsel have exclude_user,
+	 * system_wide is not needed.
+	 *
+	 * all_kernel and all_user will overwrite exclude_kernel and
+	 * exclude_user of attr in evsel__config(), here need to check
+	 * all the three items.
+	 *
+	 * Sideband system wide if one of the following conditions is met:
+	 *
+	 *   - all_user is set, and there is a non-dummy event
+	 *   - all_user and all_kernel are not set, and there is
+	 *     a non-dummy event without exclude_user
+	 */
+	if (opts->all_kernel)
+		return false;
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (!evsel__is_dummy_event(evsel)) {
+			if (opts->all_user || !evsel->core.attr.exclude_user)
+				return true;
+		}
+	}
+
+	return false;
+}
+
 static int record__config_tracking_events(struct record *rec)
 {
 	struct record_opts *opts = &rec->opts;
 	struct evlist *evlist = rec->evlist;
+	bool system_wide = false;
 	struct evsel *evsel;
 
 	/*
@@ -919,7 +953,15 @@ static int record__config_tracking_events(struct record *rec)
 	 */
 	if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
 	    perf_pmus__num_core_pmus() > 1) {
-		evsel = evlist__findnew_tracking_event(evlist, false);
+
+		/*
+		 * User space tasks can migrate between CPUs, so when tracing
+		 * selected CPUs, sideband for all CPUs is still needed.
+		 */
+		if (!!opts->target.cpu_list && record__tracking_system_wide(rec))
+			system_wide = true;
+
+		evsel = evlist__findnew_tracking_event(evlist, system_wide);
 		if (!evsel)
 			return -ENOMEM;
 
-- 
2.30.GIT

[PATCH v6 5/7] perf test: Update base-record & system-wide-dummy attr expected values for test-record-C0

[Yang Jihong]

1. Because a dummy sideband event is added to the sampling of specified
   CPUs. When evlist contains evsel of different sample_type,
   evlist__config() will change the default PERF_SAMPLE_ID bit to
   PERF_SAMPLE_IDENTIFICATION bit.
   The attr sample_type expected value of base-record and system-wide-dummy
   in test-record-C0 needs to be updated.

2. The perf record uses evlist__add_aux_dummy() instead of
   evlist__add_dummy() to add a dummy event.
   The expected value of system-wide-dummy attr needs to be updated.

The perf test result is as follows:

  # ./perf test list  2>&1 | grep 'Setup struct perf_event_attr'
   17: Setup struct perf_event_attr
  # ./perf test 17
   17: Setup struct perf_event_attr                                    : Ok

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Tested-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/attr/system-wide-dummy | 14 ++++++++------
 tools/perf/tests/attr/test-record-C0    |  4 ++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/attr/system-wide-dummy b/tools/perf/tests/attr/system-wide-dummy
index 2f3e3eb728eb..a1e1d6a263bf 100644
--- a/tools/perf/tests/attr/system-wide-dummy
+++ b/tools/perf/tests/attr/system-wide-dummy
@@ -9,8 +9,10 @@ flags=8
 type=1
 size=136
 config=9
-sample_period=4000
-sample_type=455
+sample_period=1
+# PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
+# PERF_SAMPLE_CPU | PERF_SAMPLE_IDENTIFIER
+sample_type=65671
 read_format=4|20
 # Event will be enabled right away.
 disabled=0
@@ -18,12 +20,12 @@ inherit=1
 pinned=0
 exclusive=0
 exclude_user=0
-exclude_kernel=0
-exclude_hv=0
+exclude_kernel=1
+exclude_hv=1
 exclude_idle=0
 mmap=1
 comm=1
-freq=1
+freq=0
 inherit_stat=0
 enable_on_exec=0
 task=1
@@ -32,7 +34,7 @@ precise_ip=0
 mmap_data=0
 sample_id_all=1
 exclude_host=0
-exclude_guest=0
+exclude_guest=1
 exclude_callchain_kernel=0
 exclude_callchain_user=0
 mmap2=1
diff --git a/tools/perf/tests/attr/test-record-C0 b/tools/perf/tests/attr/test-record-C0
index 317730b906dd..198e8429a1bf 100644
--- a/tools/perf/tests/attr/test-record-C0
+++ b/tools/perf/tests/attr/test-record-C0
@@ -10,9 +10,9 @@ cpu=0
 enable_on_exec=0
 
 # PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
-# PERF_SAMPLE_ID | PERF_SAMPLE_PERIOD
+# PERF_SAMPLE_PERIOD | PERF_SAMPLE_IDENTIFIER
 # + PERF_SAMPLE_CPU added by -C 0
-sample_type=455
+sample_type=65927
 
 # Dummy event handles mmaps, comm and task.
 mmap=0
-- 
2.30.GIT

[PATCH v6 6/7] perf test: Add test case for record sideband events

[Yang Jihong]

Add a new test case to record sideband events for all CPUs when tracing
selected CPUs

Test result:

  # ./perf test list 2>&1 | grep 'perf record sideband tests'
   95: perf record sideband tests
  # ./perf test 95
   95: perf record sideband tests                                      : Ok

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Tested-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/record_sideband.sh | 44 +++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100755 tools/perf/tests/shell/record_sideband.sh

diff --git a/tools/perf/tests/shell/record_sideband.sh b/tools/perf/tests/shell/record_sideband.sh
new file mode 100755
index 000000000000..2ecf00011cb1
--- /dev/null
+++ b/tools/perf/tests/shell/record_sideband.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+# perf record sideband tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+err=0
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+
+can_cpu_wide()
+{
+    if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
+    then
+        echo "record sideband test [Skipped cannot record cpu$1]"
+        err=2
+    fi
+
+    rm -f ${perfdata}
+    return $err
+}
+
+test_system_wide_tracking()
+{
+    # Need CPU 0 and CPU 1
+    can_cpu_wide 0 || return 0
+    can_cpu_wide 1 || return 0
+
+    # Record on CPU 0 a task running on CPU 1
+    perf record -BN --no-bpf-event -o ${perfdata} -C 0 -- taskset --cpu-list 1 true
+
+    # Should get MMAP events from CPU 1
+    mmap_cnt=`perf script -i ${perfdata} --show-mmap-events -C 1 2>/dev/null | grep MMAP | wc -l`
+
+    rm -f ${perfdata}
+
+    if [ ${mmap_cnt} -gt 0 ] ; then
+        return 0
+    fi
+
+    echo "Failed to record MMAP events on CPU 1 when tracing CPU 0"
+    return 1
+}
+
+test_system_wide_tracking
-- 
2.30.GIT

[PATCH v6 7/7] perf test: Add perf_event_attr test for record selected CPUs exclude_user

[Yang Jihong]

If all (non-dummy) evsel have exclude_user, system_wide sideband is not
needed. Add this test scenario.

Test result:

  # ./perf test list 2>&1 | grep 'Setup struct perf_event_attr'
   17: Setup struct perf_event_attr
  # ./perf test 17 -v
   17: Setup struct perf_event_attr                                    :
  --- start ---
  test child forked, pid 720198
  <SNIP>
  running './tests/attr/test-record-C0-all-kernel'
  <SNIP>
  test child finished with 0
  ---- end ----
  Setup struct perf_event_attr: Ok

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
Tested-by: Adrian Hunter <adrian.hunter@intel.com>
---
 .../perf/tests/attr/test-record-C0-all-kernel | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 tools/perf/tests/attr/test-record-C0-all-kernel

diff --git a/tools/perf/tests/attr/test-record-C0-all-kernel b/tools/perf/tests/attr/test-record-C0-all-kernel
new file mode 100644
index 000000000000..2d7549277c1e
--- /dev/null
+++ b/tools/perf/tests/attr/test-record-C0-all-kernel
@@ -0,0 +1,32 @@
+[config]
+command = record
+args    = --no-bpf-event --all-kernel -C 0 kill >/dev/null 2>&1
+ret     = 1
+
+[event:base-record]
+cpu=0
+
+# no enable on exec for CPU attached
+enable_on_exec=0
+
+# PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
+# PERF_SAMPLE_PERIOD | PERF_SAMPLE_IDENTIFIER
+# + PERF_SAMPLE_CPU added by -C 0
+sample_type=65927
+
+# Dummy event handles mmaps, comm and task.
+mmap=0
+comm=0
+task=0
+
+# exclude_user for all-kernel option
+exclude_user=1
+
+[event:system-wide-dummy]
+
+# system_wide is not need for all (non-dummy) events have exclude_user
+cpu=0
+
+# exclude_user for all-kernel option
+exclude_user=1
+exclude_kernel=0
-- 
2.30.GIT

Re: [PATCH v6 0/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Yang Jihong]

Hi Arnaldo,

Can you consider applying this patchset ?
Please let me know if there is anything that needs to be fixed.

Yang,
Thanks

On 2023/8/21 9:27, Yang Jihong wrote:
> User space tasks can migrate between CPUs, track sideband events for all
> CPUs.
> 
> The specific scenarios are as follows:
> 
>           CPU0                                 CPU1
>    perf record -C 0 start
>                                taskA starts to be created and executed
>                                  -> PERF_RECORD_COMM and PERF_RECORD_MMAP
>                                     events only deliver to CPU1
>                                ......
>                                  |
>                            migrate to CPU0
>                                  |
>    Running on CPU0    <----------/
>    ...
> 
>    perf record -C 0 stop
> 
> Now perf samples the PC of taskA. However, perf does not record the
> PERF_RECORD_COMM and PERF_RECORD_COMM events of taskA.
> Therefore, the comm and symbols of taskA cannot be parsed.
> 
> The sys_perf_event_open invoked is as follows:
> 
>    # perf --debug verbose=3 record -e cpu-clock -C 1 true
>    <SNIP>
>    Opening: cpu-clock
>    ------------------------------------------------------------
>    perf_event_attr:
>      type                             1 (PERF_TYPE_SOFTWARE)
>      size                             136
>      config                           0 (PERF_COUNT_SW_CPU_CLOCK)
>      { sample_period, sample_freq }   4000
>      sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
>      read_format                      ID|LOST
>      disabled                         1
>      inherit                          1
>      freq                             1
>      sample_id_all                    1
>      exclude_guest                    1
>    ------------------------------------------------------------
>    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
>    Opening: dummy:u
>    ------------------------------------------------------------
>    perf_event_attr:
>      type                             1 (PERF_TYPE_SOFTWARE)
>      size                             136
>      config                           0x9 (PERF_COUNT_SW_DUMMY)
>      { sample_period, sample_freq }   1
>      sample_type                      IP|TID|TIME|CPU|IDENTIFIER
>      read_format                      ID|LOST
>      inherit                          1
>      exclude_kernel                   1
>      exclude_hv                       1
>      mmap                             1
>      comm                             1
>      task                             1
>      sample_id_all                    1
>      exclude_guest                    1
>      mmap2                            1
>      comm_exec                        1
>      ksymbol                          1
>      bpf_event                        1
>    ------------------------------------------------------------
>    sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
>    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
>    sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
>    sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
>    sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
>    sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
>    sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
>    sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
>    <SNIP>
> 
> Changes since_v5:
>   - No code changes.
>   - Detailed commit message of patch3.
>   - Add Acked-by and Tested-by tags from Adrian Hunter.
> 
> Changes since_v4:
>   - Simplify check code for record__tracking_system_wide().
>   - Add perf attr test result to commit message for patch 7.
> 
> Changes since_v3:
>   - Check fall_kernel, all_user, and dummy or exclude_user when determining
>     whether system wide is required.
> 
> Changes since_v2:
>   - Rename record_tracking.sh to record_sideband.sh in tools/perf/tests/shell.
>   - Remove "perf evlist: Skip dummy event sample_type check for evlist_config" patch.
>   - Add opts->all_kernel check in record__config_tracking_events().
>   - Add perf_event_attr test for record selected CPUs exclude_user.
>   - Update base-record & system-wide-dummy sample_type attr expected values for test-record-C0.
> 
> Changes since v1:
>   - Add perf_evlist__go_system_wide() via internal/evlist.h instead of
>     exporting perf_evlist__propagate_maps().
>   - Use evlist__add_aux_dummy() instead of evlist__add_dummy() in
>     evlist__findnew_tracking_event().
>   - Add a parameter in evlist__findnew_tracking_event() to deal with
>     system_wide inside.
>   - Add sideband for all CPUs when tracing selected CPUs comments on
>     the perf record man page.
>   - Use "sideband events" instead of "tracking events".
>   - Adjust the patches Sequence.
>   - Add patch5 to skip dummy event sample_type check for evlist_config.
>   - Add patch6 to update system-wide-dummy attr values for perf test.
> 
> Yang Jihong (7):
>    perf evlist: Add perf_evlist__go_system_wide() helper
>    perf evlist: Add evlist__findnew_tracking_event() helper
>    perf record: Move setting dummy tracking before
>      record__init_thread_masks()
>    perf record: Track sideband events for all CPUs when tracing selected
>      CPUs
>    perf test: Update base-record & system-wide-dummy attr expected values
>      for test-record-C0
>    perf test: Add test case for record sideband events
>    perf test: Add perf_event_attr test for record selected CPUs
>      exclude_user
> 
>   tools/lib/perf/evlist.c                       |   9 ++
>   tools/lib/perf/include/internal/evlist.h      |   2 +
>   tools/perf/Documentation/perf-record.txt      |   3 +
>   tools/perf/builtin-record.c                   | 106 +++++++++++++-----
>   tools/perf/tests/attr/system-wide-dummy       |  14 ++-
>   tools/perf/tests/attr/test-record-C0          |   4 +-
>   .../perf/tests/attr/test-record-C0-all-kernel |  32 ++++++
>   tools/perf/tests/shell/record_sideband.sh     |  44 ++++++++
>   tools/perf/util/evlist.c                      |  18 +++
>   tools/perf/util/evlist.h                      |   1 +
>   10 files changed, 198 insertions(+), 35 deletions(-)
>   create mode 100644 tools/perf/tests/attr/test-record-C0-all-kernel
>   create mode 100755 tools/perf/tests/shell/record_sideband.sh
> 

Re: [PATCH v6 0/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Arnaldo Carvalho de Melo]

Em Wed, Aug 23, 2023 at 09:17:56AM +0800, Yang Jihong escreveu:
> Hi Arnaldo,
> 
> Can you consider applying this patchset ?
> Please let me know if there is anything that needs to be fixed.

I'm just giving Ian some more time since he's been busy lately and tried
to review this patchset,

- Arnaldo
 
> Yang,
> Thanks
> 
> On 2023/8/21 9:27, Yang Jihong wrote:
> > User space tasks can migrate between CPUs, track sideband events for all
> > CPUs.
> > 
> > The specific scenarios are as follows:
> > 
> >           CPU0                                 CPU1
> >    perf record -C 0 start
> >                                taskA starts to be created and executed
> >                                  -> PERF_RECORD_COMM and PERF_RECORD_MMAP
> >                                     events only deliver to CPU1
> >                                ......
> >                                  |
> >                            migrate to CPU0
> >                                  |
> >    Running on CPU0    <----------/
> >    ...
> > 
> >    perf record -C 0 stop
> > 
> > Now perf samples the PC of taskA. However, perf does not record the
> > PERF_RECORD_COMM and PERF_RECORD_COMM events of taskA.
> > Therefore, the comm and symbols of taskA cannot be parsed.
> > 
> > The sys_perf_event_open invoked is as follows:
> > 
> >    # perf --debug verbose=3 record -e cpu-clock -C 1 true
> >    <SNIP>
> >    Opening: cpu-clock
> >    ------------------------------------------------------------
> >    perf_event_attr:
> >      type                             1 (PERF_TYPE_SOFTWARE)
> >      size                             136
> >      config                           0 (PERF_COUNT_SW_CPU_CLOCK)
> >      { sample_period, sample_freq }   4000
> >      sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
> >      read_format                      ID|LOST
> >      disabled                         1
> >      inherit                          1
> >      freq                             1
> >      sample_id_all                    1
> >      exclude_guest                    1
> >    ------------------------------------------------------------
> >    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
> >    Opening: dummy:u
> >    ------------------------------------------------------------
> >    perf_event_attr:
> >      type                             1 (PERF_TYPE_SOFTWARE)
> >      size                             136
> >      config                           0x9 (PERF_COUNT_SW_DUMMY)
> >      { sample_period, sample_freq }   1
> >      sample_type                      IP|TID|TIME|CPU|IDENTIFIER
> >      read_format                      ID|LOST
> >      inherit                          1
> >      exclude_kernel                   1
> >      exclude_hv                       1
> >      mmap                             1
> >      comm                             1
> >      task                             1
> >      sample_id_all                    1
> >      exclude_guest                    1
> >      mmap2                            1
> >      comm_exec                        1
> >      ksymbol                          1
> >      bpf_event                        1
> >    ------------------------------------------------------------
> >    sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
> >    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
> >    sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
> >    sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
> >    sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
> >    sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
> >    sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
> >    sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
> >    <SNIP>
> > 
> > Changes since_v5:
> >   - No code changes.
> >   - Detailed commit message of patch3.
> >   - Add Acked-by and Tested-by tags from Adrian Hunter.
> > 
> > Changes since_v4:
> >   - Simplify check code for record__tracking_system_wide().
> >   - Add perf attr test result to commit message for patch 7.
> > 
> > Changes since_v3:
> >   - Check fall_kernel, all_user, and dummy or exclude_user when determining
> >     whether system wide is required.
> > 
> > Changes since_v2:
> >   - Rename record_tracking.sh to record_sideband.sh in tools/perf/tests/shell.
> >   - Remove "perf evlist: Skip dummy event sample_type check for evlist_config" patch.
> >   - Add opts->all_kernel check in record__config_tracking_events().
> >   - Add perf_event_attr test for record selected CPUs exclude_user.
> >   - Update base-record & system-wide-dummy sample_type attr expected values for test-record-C0.
> > 
> > Changes since v1:
> >   - Add perf_evlist__go_system_wide() via internal/evlist.h instead of
> >     exporting perf_evlist__propagate_maps().
> >   - Use evlist__add_aux_dummy() instead of evlist__add_dummy() in
> >     evlist__findnew_tracking_event().
> >   - Add a parameter in evlist__findnew_tracking_event() to deal with
> >     system_wide inside.
> >   - Add sideband for all CPUs when tracing selected CPUs comments on
> >     the perf record man page.
> >   - Use "sideband events" instead of "tracking events".
> >   - Adjust the patches Sequence.
> >   - Add patch5 to skip dummy event sample_type check for evlist_config.
> >   - Add patch6 to update system-wide-dummy attr values for perf test.
> > 
> > Yang Jihong (7):
> >    perf evlist: Add perf_evlist__go_system_wide() helper
> >    perf evlist: Add evlist__findnew_tracking_event() helper
> >    perf record: Move setting dummy tracking before
> >      record__init_thread_masks()
> >    perf record: Track sideband events for all CPUs when tracing selected
> >      CPUs
> >    perf test: Update base-record & system-wide-dummy attr expected values
> >      for test-record-C0
> >    perf test: Add test case for record sideband events
> >    perf test: Add perf_event_attr test for record selected CPUs
> >      exclude_user
> > 
> >   tools/lib/perf/evlist.c                       |   9 ++
> >   tools/lib/perf/include/internal/evlist.h      |   2 +
> >   tools/perf/Documentation/perf-record.txt      |   3 +
> >   tools/perf/builtin-record.c                   | 106 +++++++++++++-----
> >   tools/perf/tests/attr/system-wide-dummy       |  14 ++-
> >   tools/perf/tests/attr/test-record-C0          |   4 +-
> >   .../perf/tests/attr/test-record-C0-all-kernel |  32 ++++++
> >   tools/perf/tests/shell/record_sideband.sh     |  44 ++++++++
> >   tools/perf/util/evlist.c                      |  18 +++
> >   tools/perf/util/evlist.h                      |   1 +
> >   10 files changed, 198 insertions(+), 35 deletions(-)
> >   create mode 100644 tools/perf/tests/attr/test-record-C0-all-kernel
> >   create mode 100755 tools/perf/tests/shell/record_sideband.sh
> > 

-- 

- Arnaldo

Re: [PATCH v6 1/7] perf evlist: Add perf_evlist__go_system_wide() helper

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> For dummy events that keep tracking, we may need to modify its cpu_maps.
> For example, change the cpu_maps to record sideband events for all CPUS.
> Add perf_evlist__go_system_wide() helper to support this scenario.
>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/lib/perf/evlist.c                  | 9 +++++++++
>  tools/lib/perf/include/internal/evlist.h | 2 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index b8b066d0dc5e..3acbbccc1901 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -738,3 +738,12 @@ int perf_evlist__nr_groups(struct perf_evlist *evlist)
>         }
>         return nr_groups;
>  }
> +
> +void perf_evlist__go_system_wide(struct perf_evlist *evlist, struct perf_evsel *evsel)
> +{
> +       if (!evsel->system_wide) {
> +               evsel->system_wide = true;
> +               if (evlist->needs_map_propagation)
> +                       __perf_evlist__propagate_maps(evlist, evsel);
> +       }
> +}

I think this should be:

void evsel__set_system_wide(struct evsel *evsel)
{
        if (evsel->system_wide)
               return;
        evsel->system_wide = true;
        if (evsel->evlist->core.needs_map_propagation)
...

The API being on evlist makes it look like all evsels are affected.

Thanks,
Ian

> diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
> index 3339bc2f1765..d86ffe8ed483 100644
> --- a/tools/lib/perf/include/internal/evlist.h
> +++ b/tools/lib/perf/include/internal/evlist.h
> @@ -135,4 +135,6 @@ int perf_evlist__id_add_fd(struct perf_evlist *evlist,
>  void perf_evlist__reset_id_hash(struct perf_evlist *evlist);
>
>  void __perf_evlist__set_leader(struct list_head *list, struct perf_evsel *leader);
> +
> +void perf_evlist__go_system_wide(struct perf_evlist *evlist, struct perf_evsel *evsel);
>  #endif /* __LIBPERF_INTERNAL_EVLIST_H */
> --
> 2.30.GIT
>

Re: [PATCH v6 2/7] perf evlist: Add evlist__findnew_tracking_event() helper

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> Currently, intel-bts, intel-pt, and arm-spe may add a dummy event for
> tracking to the evlist. We may need to search for the dummy event for
> some settings. Therefore, add evlist__findnew_tracking_event() helper.

Given the first two sentences I don't understand why this is
evlist__findnew_tracking_event and not evlist__findnew_dummy_event?
Are you setting tracking on other events other than dummy? If so, then
the commit message isn't right. If not then I'd prefer not to use
tracking event in the function name.

>
> evlist__findnew_tracking_event() also deal with system_wide maps if
> system_wide is true.

Could you fix the explanation here, what does "deal with system_wide"
mean? A kerneldoc comment and explanation of the system_wide argument
would be useful.

Thanks,
Ian

> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/builtin-record.c | 11 +++--------
>  tools/perf/util/evlist.c    | 18 ++++++++++++++++++
>  tools/perf/util/evlist.h    |  1 +
>  3 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 34bb31f08bb5..12edad8392cc 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1293,14 +1293,9 @@ static int record__open(struct record *rec)
>          */
>         if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
>             perf_pmus__num_core_pmus() > 1) {
> -               pos = evlist__get_tracking_event(evlist);
> -               if (!evsel__is_dummy_event(pos)) {
> -                       /* Set up dummy event. */
> -                       if (evlist__add_dummy(evlist))
> -                               return -ENOMEM;
> -                       pos = evlist__last(evlist);
> -                       evlist__set_tracking_event(evlist, pos);
> -               }
> +               pos = evlist__findnew_tracking_event(evlist, false);
> +               if (!pos)
> +                       return -ENOMEM;
>
>                 /*
>                  * Enable the dummy event when the process is forked for
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 7ef43f72098e..25c3ebe2c2f5 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1694,6 +1694,24 @@ void evlist__set_tracking_event(struct evlist *evlist, struct evsel *tracking_ev
>         tracking_evsel->tracking = true;
>  }
>
> +struct evsel *evlist__findnew_tracking_event(struct evlist *evlist, bool system_wide)
> +{
> +       struct evsel *evsel;
> +
> +       evsel = evlist__get_tracking_event(evlist);
> +       if (!evsel__is_dummy_event(evsel)) {
> +               evsel = evlist__add_aux_dummy(evlist, system_wide);
> +               if (!evsel)
> +                       return NULL;
> +
> +               evlist__set_tracking_event(evlist, evsel);
> +       } else if (system_wide) {
> +               perf_evlist__go_system_wide(&evlist->core, &evsel->core);
> +       }
> +
> +       return evsel;
> +}
> +
>  struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str)
>  {
>         struct evsel *evsel;
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 664c6bf7b3e0..98e7ddb2bd30 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -387,6 +387,7 @@ bool evlist_cpu_iterator__end(const struct evlist_cpu_iterator *evlist_cpu_itr);
>
>  struct evsel *evlist__get_tracking_event(struct evlist *evlist);
>  void evlist__set_tracking_event(struct evlist *evlist, struct evsel *tracking_evsel);
> +struct evsel *evlist__findnew_tracking_event(struct evlist *evlist, bool system_wide);
>
>  struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str);
>
> --
> 2.30.GIT
>

Re: [PATCH v6 3/7] perf record: Move setting dummy tracking before record__init_thread_masks()

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> User space tasks can migrate between CPUs, so when tracing selected CPUs,
> sideband for all CPUs is needed. In this case set the cpu map of the evsel
> to all online CPUs. This may modify the original cpu map of the evlist.
> Therefore, need to check whether the preceding scenario exists before
> record__init_thread_masks().
> Dummy tracking has been set in record__open(), move it before
> record__init_thread_masks() and add a helper for unified processing.

I have a hard time understanding this commit message. Let me have a go:

In order to set cpu maps correctly on dummy events they need to set up
later. Factor out the dummy event initialization from record__open and
move it to its own function record__setup_dummy_event. Move the call
to the record__setup_dummy to before the call to
record__init_thread_masks in cmd_record.

> The sys_perf_event_open invoked is as follows:
>
>   # perf --debug verbose=3 record -e cpu-clock -D 100 true
>   <SNIP>
>   Opening: cpu-clock
>   ------------------------------------------------------------
>   perf_event_attr:
>     type                             1 (PERF_TYPE_SOFTWARE)
>     size                             136
>     config                           0 (PERF_COUNT_SW_CPU_CLOCK)
>     { sample_period, sample_freq }   4000
>     sample_type                      IP|TID|TIME|PERIOD|IDENTIFIER
>     read_format                      ID|LOST
>     disabled                         1
>     inherit                          1
>     freq                             1
>     sample_id_all                    1
>     exclude_guest                    1
>   ------------------------------------------------------------
>   sys_perf_event_open: pid 10318  cpu 0  group_fd -1  flags 0x8 = 5
>   sys_perf_event_open: pid 10318  cpu 1  group_fd -1  flags 0x8 = 6
>   sys_perf_event_open: pid 10318  cpu 2  group_fd -1  flags 0x8 = 7
>   sys_perf_event_open: pid 10318  cpu 3  group_fd -1  flags 0x8 = 9
>   sys_perf_event_open: pid 10318  cpu 4  group_fd -1  flags 0x8 = 10
>   sys_perf_event_open: pid 10318  cpu 5  group_fd -1  flags 0x8 = 11
>   sys_perf_event_open: pid 10318  cpu 6  group_fd -1  flags 0x8 = 12
>   sys_perf_event_open: pid 10318  cpu 7  group_fd -1  flags 0x8 = 13
>   Opening: dummy:u
>   ------------------------------------------------------------
>   perf_event_attr:
>     type                             1 (PERF_TYPE_SOFTWARE)
>     size                             136
>     config                           0x9 (PERF_COUNT_SW_DUMMY)
>     { sample_period, sample_freq }   1
>     sample_type                      IP|TID|TIME|IDENTIFIER
>     read_format                      ID|LOST
>     disabled                         1
>     inherit                          1
>     exclude_kernel                   1
>     exclude_hv                       1
>     mmap                             1
>     comm                             1
>     enable_on_exec                   1
>     task                             1
>     sample_id_all                    1
>     exclude_guest                    1
>     mmap2                            1
>     comm_exec                        1
>     ksymbol                          1
>     bpf_event                        1
>   ------------------------------------------------------------
>   sys_perf_event_open: pid 10318  cpu 0  group_fd -1  flags 0x8 = 14
>   sys_perf_event_open: pid 10318  cpu 1  group_fd -1  flags 0x8 = 15
>   sys_perf_event_open: pid 10318  cpu 2  group_fd -1  flags 0x8 = 16
>   sys_perf_event_open: pid 10318  cpu 3  group_fd -1  flags 0x8 = 17
>   sys_perf_event_open: pid 10318  cpu 4  group_fd -1  flags 0x8 = 18
>   sys_perf_event_open: pid 10318  cpu 5  group_fd -1  flags 0x8 = 19
>   sys_perf_event_open: pid 10318  cpu 6  group_fd -1  flags 0x8 = 20
>   sys_perf_event_open: pid 10318  cpu 7  group_fd -1  flags 0x8 = 21
>   <SNIP>
>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/builtin-record.c | 59 +++++++++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 12edad8392cc..4ee94058028f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -906,6 +906,37 @@ static int record__config_off_cpu(struct record *rec)
>         return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
>  }
>
> +static int record__config_tracking_events(struct record *rec)
> +{
> +       struct record_opts *opts = &rec->opts;
> +       struct evlist *evlist = rec->evlist;
> +       struct evsel *evsel;
> +
> +       /*
> +        * For initial_delay, system wide or a hybrid system, we need to add a
> +        * dummy event so that we can track PERF_RECORD_MMAP to cover the delay

This code is explicitly talking about dummy events but the function is
now renamed to call it a tracking event. I think the code should be
consistent.

> +        * of waiting or event synthesis.
> +        */
> +       if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
> +           perf_pmus__num_core_pmus() > 1) {
> +               evsel = evlist__findnew_tracking_event(evlist, false);
> +               if (!evsel)
> +                       return -ENOMEM;
> +
> +               /*
> +                * Enable the dummy event when the process is forked for

Dummy again.

Thanks,
Ian

> +                * initial_delay, immediately for system wide.
> +                */
> +               if (opts->target.initial_delay && !evsel->immediate &&
> +                   !target__has_cpu(&opts->target))
> +                       evsel->core.attr.enable_on_exec = 1;
> +               else
> +                       evsel->immediate = 1;
> +       }
> +
> +       return 0;
> +}
> +
>  static bool record__kcore_readable(struct machine *machine)
>  {
>         char kcore[PATH_MAX];
> @@ -1286,28 +1317,6 @@ static int record__open(struct record *rec)
>         struct record_opts *opts = &rec->opts;
>         int rc = 0;
>
> -       /*
> -        * For initial_delay, system wide or a hybrid system, we need to add a
> -        * dummy event so that we can track PERF_RECORD_MMAP to cover the delay
> -        * of waiting or event synthesis.
> -        */
> -       if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
> -           perf_pmus__num_core_pmus() > 1) {
> -               pos = evlist__findnew_tracking_event(evlist, false);
> -               if (!pos)
> -                       return -ENOMEM;
> -
> -               /*
> -                * Enable the dummy event when the process is forked for
> -                * initial_delay, immediately for system wide.
> -                */
> -               if (opts->target.initial_delay && !pos->immediate &&
> -                   !target__has_cpu(&opts->target))
> -                       pos->core.attr.enable_on_exec = 1;
> -               else
> -                       pos->immediate = 1;
> -       }
> -
>         evlist__config(evlist, opts, &callchain_param);
>
>         evlist__for_each_entry(evlist, pos) {
> @@ -4190,6 +4199,12 @@ int cmd_record(int argc, const char **argv)
>                 goto out;
>         }
>
> +       err = record__config_tracking_events(rec);
> +       if (err) {
> +               pr_err("record__config_tracking_events failed, error %d\n", err);
> +               goto out;
> +       }
> +
>         err = record__init_thread_masks(rec);
>         if (err) {
>                 pr_err("Failed to initialize parallel data streaming masks\n");
> --
> 2.30.GIT
>

Re: [PATCH v6 4/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> User space tasks can migrate between CPUs, we need to track side-band
> events for all CPUs.
>
> The specific scenarios are as follows:
>
>          CPU0                                 CPU1
>   perf record -C 0 start
>                               taskA starts to be created and executed
>                                 -> PERF_RECORD_COMM and PERF_RECORD_MMAP
>                                    events only deliver to CPU1
>                               ......
>                                 |
>                           migrate to CPU0
>                                 |
>   Running on CPU0    <----------/
>   ...
>
>   perf record -C 0 stop
>
> Now perf samples the PC of taskA. However, perf does not record the
> PERF_RECORD_COMM and PERF_RECORD_MMAP events of taskA.
> Therefore, the comm and symbols of taskA cannot be parsed.
>
> The solution is to record sideband events for all CPUs when tracing
> selected CPUs. Because this modifies the default behavior, add related
> comments to the perf record man page.
>
> The sys_perf_event_open invoked is as follows:
>
>   # perf --debug verbose=3 record -e cpu-clock -C 1 true
>   <SNIP>
>   Opening: cpu-clock
>   ------------------------------------------------------------
>   perf_event_attr:
>     type                             1 (PERF_TYPE_SOFTWARE)
>     size                             136
>     config                           0 (PERF_COUNT_SW_CPU_CLOCK)
>     { sample_period, sample_freq }   4000
>     sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
>     read_format                      ID|LOST
>     disabled                         1
>     inherit                          1
>     freq                             1
>     sample_id_all                    1
>     exclude_guest                    1
>   ------------------------------------------------------------
>   sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
>   Opening: dummy:u
>   ------------------------------------------------------------
>   perf_event_attr:
>     type                             1 (PERF_TYPE_SOFTWARE)
>     size                             136
>     config                           0x9 (PERF_COUNT_SW_DUMMY)
>     { sample_period, sample_freq }   1
>     sample_type                      IP|TID|TIME|CPU|IDENTIFIER
>     read_format                      ID|LOST
>     inherit                          1
>     exclude_kernel                   1
>     exclude_hv                       1
>     mmap                             1
>     comm                             1
>     task                             1
>     sample_id_all                    1
>     exclude_guest                    1
>     mmap2                            1
>     comm_exec                        1
>     ksymbol                          1
>     bpf_event                        1
>   ------------------------------------------------------------
>   sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
>   sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
>   sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
>   sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
>   sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
>   sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
>   sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
>   sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
>   <SNIP>
>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/Documentation/perf-record.txt |  3 ++
>  tools/perf/builtin-record.c              | 44 +++++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index d5217be012d7..1889f66addf2 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -374,6 +374,9 @@ comma-separated list with no space: 0,1. Ranges of CPUs are specified with -: 0-
>  In per-thread mode with inheritance mode on (default), samples are captured only when
>  the thread executes on the designated CPUs. Default is to monitor all CPUs.
>
> +User space tasks can migrate between CPUs, so when tracing selected CPUs,
> +a dummy event is created to track sideband for all CPUs.
> +
>  -B::
>  --no-buildid::
>  Do not save the build ids of binaries in the perf.data files. This skips
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 4ee94058028f..ae2e21b945fa 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -906,10 +906,44 @@ static int record__config_off_cpu(struct record *rec)
>         return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
>  }
>
> +static bool record__tracking_system_wide(struct record *rec)

I think this would be better named something like:
record__need_system_wide_dummy_event

Thanks,
Ian

> +{
> +       struct record_opts *opts = &rec->opts;
> +       struct evlist *evlist = rec->evlist;
> +       struct evsel *evsel;
> +
> +       /*
> +        * If all (non-dummy) evsel have exclude_user,
> +        * system_wide is not needed.
> +        *
> +        * all_kernel and all_user will overwrite exclude_kernel and
> +        * exclude_user of attr in evsel__config(), here need to check
> +        * all the three items.
> +        *
> +        * Sideband system wide if one of the following conditions is met:
> +        *
> +        *   - all_user is set, and there is a non-dummy event
> +        *   - all_user and all_kernel are not set, and there is
> +        *     a non-dummy event without exclude_user
> +        */
> +       if (opts->all_kernel)
> +               return false;
> +
> +       evlist__for_each_entry(evlist, evsel) {
> +               if (!evsel__is_dummy_event(evsel)) {
> +                       if (opts->all_user || !evsel->core.attr.exclude_user)
> +                               return true;
> +               }
> +       }
> +
> +       return false;
> +}
> +
>  static int record__config_tracking_events(struct record *rec)
>  {
>         struct record_opts *opts = &rec->opts;
>         struct evlist *evlist = rec->evlist;
> +       bool system_wide = false;
>         struct evsel *evsel;
>
>         /*
> @@ -919,7 +953,15 @@ static int record__config_tracking_events(struct record *rec)
>          */
>         if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
>             perf_pmus__num_core_pmus() > 1) {
> -               evsel = evlist__findnew_tracking_event(evlist, false);
> +
> +               /*
> +                * User space tasks can migrate between CPUs, so when tracing
> +                * selected CPUs, sideband for all CPUs is still needed.
> +                */
> +               if (!!opts->target.cpu_list && record__tracking_system_wide(rec))
> +                       system_wide = true;
> +
> +               evsel = evlist__findnew_tracking_event(evlist, system_wide);
>                 if (!evsel)
>                         return -ENOMEM;
>
> --
> 2.30.GIT
>

Re: [PATCH v6 5/7] perf test: Update base-record & system-wide-dummy attr expected values for test-record-C0

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> 1. Because a dummy sideband event is added to the sampling of specified
>    CPUs. When evlist contains evsel of different sample_type,
>    evlist__config() will change the default PERF_SAMPLE_ID bit to
>    PERF_SAMPLE_IDENTIFICATION bit.
>    The attr sample_type expected value of base-record and system-wide-dummy
>    in test-record-C0 needs to be updated.
>
> 2. The perf record uses evlist__add_aux_dummy() instead of
>    evlist__add_dummy() to add a dummy event.
>    The expected value of system-wide-dummy attr needs to be updated.
>
> The perf test result is as follows:
>
>   # ./perf test list  2>&1 | grep 'Setup struct perf_event_attr'
>    17: Setup struct perf_event_attr
>   # ./perf test 17
>    17: Setup struct perf_event_attr                                    : Ok

Will the kernel test robot complain about the previous patches
breaking this test? It may be best to update the test while changing
the behavior in those patches.

Thanks,
Ian


> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Tested-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/tests/attr/system-wide-dummy | 14 ++++++++------
>  tools/perf/tests/attr/test-record-C0    |  4 ++--
>  2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/tests/attr/system-wide-dummy b/tools/perf/tests/attr/system-wide-dummy
> index 2f3e3eb728eb..a1e1d6a263bf 100644
> --- a/tools/perf/tests/attr/system-wide-dummy
> +++ b/tools/perf/tests/attr/system-wide-dummy
> @@ -9,8 +9,10 @@ flags=8
>  type=1
>  size=136
>  config=9
> -sample_period=4000
> -sample_type=455
> +sample_period=1
> +# PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
> +# PERF_SAMPLE_CPU | PERF_SAMPLE_IDENTIFIER
> +sample_type=65671
>  read_format=4|20
>  # Event will be enabled right away.
>  disabled=0
> @@ -18,12 +20,12 @@ inherit=1
>  pinned=0
>  exclusive=0
>  exclude_user=0
> -exclude_kernel=0
> -exclude_hv=0
> +exclude_kernel=1
> +exclude_hv=1
>  exclude_idle=0
>  mmap=1
>  comm=1
> -freq=1
> +freq=0
>  inherit_stat=0
>  enable_on_exec=0
>  task=1
> @@ -32,7 +34,7 @@ precise_ip=0
>  mmap_data=0
>  sample_id_all=1
>  exclude_host=0
> -exclude_guest=0
> +exclude_guest=1
>  exclude_callchain_kernel=0
>  exclude_callchain_user=0
>  mmap2=1
> diff --git a/tools/perf/tests/attr/test-record-C0 b/tools/perf/tests/attr/test-record-C0
> index 317730b906dd..198e8429a1bf 100644
> --- a/tools/perf/tests/attr/test-record-C0
> +++ b/tools/perf/tests/attr/test-record-C0
> @@ -10,9 +10,9 @@ cpu=0
>  enable_on_exec=0
>
>  # PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
> -# PERF_SAMPLE_ID | PERF_SAMPLE_PERIOD
> +# PERF_SAMPLE_PERIOD | PERF_SAMPLE_IDENTIFIER
>  # + PERF_SAMPLE_CPU added by -C 0
> -sample_type=455
> +sample_type=65927
>
>  # Dummy event handles mmaps, comm and task.
>  mmap=0
> --
> 2.30.GIT
>

Re: [PATCH v6 7/7] perf test: Add perf_event_attr test for record selected CPUs exclude_user

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> If all (non-dummy) evsel have exclude_user, system_wide sideband is not
> needed. Add this test scenario.
>
> Test result:
>
>   # ./perf test list 2>&1 | grep 'Setup struct perf_event_attr'
>    17: Setup struct perf_event_attr
>   # ./perf test 17 -v
>    17: Setup struct perf_event_attr                                    :
>   --- start ---
>   test child forked, pid 720198
>   <SNIP>
>   running './tests/attr/test-record-C0-all-kernel'
>   <SNIP>
>   test child finished with 0
>   ---- end ----
>   Setup struct perf_event_attr: Ok
>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Tested-by: Adrian Hunter <adrian.hunter@intel.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  .../perf/tests/attr/test-record-C0-all-kernel | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 tools/perf/tests/attr/test-record-C0-all-kernel
>
> diff --git a/tools/perf/tests/attr/test-record-C0-all-kernel b/tools/perf/tests/attr/test-record-C0-all-kernel
> new file mode 100644
> index 000000000000..2d7549277c1e
> --- /dev/null
> +++ b/tools/perf/tests/attr/test-record-C0-all-kernel
> @@ -0,0 +1,32 @@
> +[config]
> +command = record
> +args    = --no-bpf-event --all-kernel -C 0 kill >/dev/null 2>&1
> +ret     = 1
> +
> +[event:base-record]
> +cpu=0
> +
> +# no enable on exec for CPU attached
> +enable_on_exec=0
> +
> +# PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
> +# PERF_SAMPLE_PERIOD | PERF_SAMPLE_IDENTIFIER
> +# + PERF_SAMPLE_CPU added by -C 0
> +sample_type=65927
> +
> +# Dummy event handles mmaps, comm and task.
> +mmap=0
> +comm=0
> +task=0
> +
> +# exclude_user for all-kernel option
> +exclude_user=1
> +
> +[event:system-wide-dummy]
> +
> +# system_wide is not need for all (non-dummy) events have exclude_user
> +cpu=0
> +
> +# exclude_user for all-kernel option
> +exclude_user=1
> +exclude_kernel=0
> --
> 2.30.GIT
>

Re: [PATCH v6 6/7] perf test: Add test case for record sideband events

[Ian Rogers]

On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>
> Add a new test case to record sideband events for all CPUs when tracing
> selected CPUs
>
> Test result:
>
>   # ./perf test list 2>&1 | grep 'perf record sideband tests'
>    95: perf record sideband tests
>   # ./perf test 95
>    95: perf record sideband tests                                      : Ok
>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> Tested-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/tests/shell/record_sideband.sh | 44 +++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100755 tools/perf/tests/shell/record_sideband.sh
>
> diff --git a/tools/perf/tests/shell/record_sideband.sh b/tools/perf/tests/shell/record_sideband.sh
> new file mode 100755
> index 000000000000..2ecf00011cb1
> --- /dev/null
> +++ b/tools/perf/tests/shell/record_sideband.sh
> @@ -0,0 +1,44 @@
> +#!/bin/sh
> +# perf record sideband tests
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +err=0
> +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)

Could you add some kind of cleanup on trap function like:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/record.sh?h=perf-tools-next#n26
It'd be okay to do this as a follow up patch.

Thanks,
Ian

> +
> +can_cpu_wide()
> +{
> +    if ! perf record -o ${perfdata} -BN --no-bpf-event -C $1 true 2>&1 >/dev/null
> +    then
> +        echo "record sideband test [Skipped cannot record cpu$1]"
> +        err=2
> +    fi
> +
> +    rm -f ${perfdata}
> +    return $err
> +}
> +
> +test_system_wide_tracking()
> +{
> +    # Need CPU 0 and CPU 1
> +    can_cpu_wide 0 || return 0
> +    can_cpu_wide 1 || return 0
> +
> +    # Record on CPU 0 a task running on CPU 1
> +    perf record -BN --no-bpf-event -o ${perfdata} -C 0 -- taskset --cpu-list 1 true
> +
> +    # Should get MMAP events from CPU 1
> +    mmap_cnt=`perf script -i ${perfdata} --show-mmap-events -C 1 2>/dev/null | grep MMAP | wc -l`
> +
> +    rm -f ${perfdata}
> +
> +    if [ ${mmap_cnt} -gt 0 ] ; then
> +        return 0
> +    fi
> +
> +    echo "Failed to record MMAP events on CPU 1 when tracing CPU 0"
> +    return 1
> +}
> +
> +test_system_wide_tracking
> --
> 2.30.GIT
>

Re: [PATCH v6 1/7] perf evlist: Add perf_evlist__go_system_wide() helper

[Yang Jihong]

Hello,

On 2023/8/25 12:51, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> For dummy events that keep tracking, we may need to modify its cpu_maps.
>> For example, change the cpu_maps to record sideband events for all CPUS.
>> Add perf_evlist__go_system_wide() helper to support this scenario.
>>
>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>   tools/lib/perf/evlist.c                  | 9 +++++++++
>>   tools/lib/perf/include/internal/evlist.h | 2 ++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
>> index b8b066d0dc5e..3acbbccc1901 100644
>> --- a/tools/lib/perf/evlist.c
>> +++ b/tools/lib/perf/evlist.c
>> @@ -738,3 +738,12 @@ int perf_evlist__nr_groups(struct perf_evlist *evlist)
>>          }
>>          return nr_groups;
>>   }
>> +
>> +void perf_evlist__go_system_wide(struct perf_evlist *evlist, struct perf_evsel *evsel)
>> +{
>> +       if (!evsel->system_wide) {
>> +               evsel->system_wide = true;
>> +               if (evlist->needs_map_propagation)
>> +                       __perf_evlist__propagate_maps(evlist, evsel);
>> +       }
>> +}
> 
> I think this should be:
> 
> void evsel__set_system_wide(struct evsel *evsel)
> {
>          if (evsel->system_wide)
>                 return;
>          evsel->system_wide = true;
>          if (evsel->evlist->core.needs_map_propagation)
> ...
> 
> The API being on evlist makes it look like all evsels are affected.
> 
This part of the code is implemented according to Adrian's suggestion.
Refer to:

https://lore.kernel.org/linux-perf-users/206972a3-d44d-1c75-3fbc-426427614543@intel.com/

The logic of both is the same, and either is OK for me.
If really want to change it, please let me know.

Thanks,
Yang

Re: [PATCH v6 2/7] perf evlist: Add evlist__findnew_tracking_event() helper

[Yang Jihong]

Hello,

On 2023/8/25 12:55, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> Currently, intel-bts, intel-pt, and arm-spe may add a dummy event for
>> tracking to the evlist. We may need to search for the dummy event for
>> some settings. Therefore, add evlist__findnew_tracking_event() helper.
> 
> Given the first two sentences I don't understand why this is
> evlist__findnew_tracking_event and not evlist__findnew_dummy_event?
> Are you setting tracking on other events other than dummy? If so, then
> the commit message isn't right. If not then I'd prefer not to use
> tracking event in the function name.
> 
Yes, add this helper to find tracking event for some setting, will 
modify commit message, like as follows:

Currently, intel-bts, intel-pt, and arm-spe may add tracking event to 
the evlist. We may need to search for the tracking event for
some settings. Therefore, add evlist__findnew_tracking_event() helper.

>>
>> evlist__findnew_tracking_event() also deal with system_wide maps if
>> system_wide is true.
> 
> Could you fix the explanation here, what does "deal with system_wide"
> mean? A kerneldoc comment and explanation of the system_wide argument
> would be useful.
> 
In the next version, details will be described as follows:

If system_wide is true, evlist__findnew_tracking_event() set the cpu map 
of the evsel to all online CPUs.

Is such an explanation okay?

Thanks,
Yang

Re: [PATCH v6 3/7] perf record: Move setting dummy tracking before record__init_thread_masks()

[Yang Jihong]

Hello,

On 2023/8/25 13:10, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> User space tasks can migrate between CPUs, so when tracing selected CPUs,
>> sideband for all CPUs is needed. In this case set the cpu map of the evsel
>> to all online CPUs. This may modify the original cpu map of the evlist.
>> Therefore, need to check whether the preceding scenario exists before
>> record__init_thread_masks().
>> Dummy tracking has been set in record__open(), move it before
>> record__init_thread_masks() and add a helper for unified processing.
> 
> I have a hard time understanding this commit message. Let me have a go:
> 
> In order to set cpu maps correctly on dummy events they need to set up
> later. Factor out the dummy event initialization from record__open and
> move it to its own function record__setup_dummy_event. Move the call
> to the record__setup_dummy to before the call to
> record__init_thread_masks in cmd_record.
Yes, as you said above, for that purpose

> 
>> The sys_perf_event_open invoked is as follows:
>>
>>    # perf --debug verbose=3 record -e cpu-clock -D 100 true
>>    <SNIP>
>>    Opening: cpu-clock
>>    ------------------------------------------------------------
>>    perf_event_attr:
>>      type                             1 (PERF_TYPE_SOFTWARE)
>>      size                             136
>>      config                           0 (PERF_COUNT_SW_CPU_CLOCK)
>>      { sample_period, sample_freq }   4000
>>      sample_type                      IP|TID|TIME|PERIOD|IDENTIFIER
>>      read_format                      ID|LOST
>>      disabled                         1
>>      inherit                          1
>>      freq                             1
>>      sample_id_all                    1
>>      exclude_guest                    1
>>    ------------------------------------------------------------
>>    sys_perf_event_open: pid 10318  cpu 0  group_fd -1  flags 0x8 = 5
>>    sys_perf_event_open: pid 10318  cpu 1  group_fd -1  flags 0x8 = 6
>>    sys_perf_event_open: pid 10318  cpu 2  group_fd -1  flags 0x8 = 7
>>    sys_perf_event_open: pid 10318  cpu 3  group_fd -1  flags 0x8 = 9
>>    sys_perf_event_open: pid 10318  cpu 4  group_fd -1  flags 0x8 = 10
>>    sys_perf_event_open: pid 10318  cpu 5  group_fd -1  flags 0x8 = 11
>>    sys_perf_event_open: pid 10318  cpu 6  group_fd -1  flags 0x8 = 12
>>    sys_perf_event_open: pid 10318  cpu 7  group_fd -1  flags 0x8 = 13
>>    Opening: dummy:u
>>    ------------------------------------------------------------
>>    perf_event_attr:
>>      type                             1 (PERF_TYPE_SOFTWARE)
>>      size                             136
>>      config                           0x9 (PERF_COUNT_SW_DUMMY)
>>      { sample_period, sample_freq }   1
>>      sample_type                      IP|TID|TIME|IDENTIFIER
>>      read_format                      ID|LOST
>>      disabled                         1
>>      inherit                          1
>>      exclude_kernel                   1
>>      exclude_hv                       1
>>      mmap                             1
>>      comm                             1
>>      enable_on_exec                   1
>>      task                             1
>>      sample_id_all                    1
>>      exclude_guest                    1
>>      mmap2                            1
>>      comm_exec                        1
>>      ksymbol                          1
>>      bpf_event                        1
>>    ------------------------------------------------------------
>>    sys_perf_event_open: pid 10318  cpu 0  group_fd -1  flags 0x8 = 14
>>    sys_perf_event_open: pid 10318  cpu 1  group_fd -1  flags 0x8 = 15
>>    sys_perf_event_open: pid 10318  cpu 2  group_fd -1  flags 0x8 = 16
>>    sys_perf_event_open: pid 10318  cpu 3  group_fd -1  flags 0x8 = 17
>>    sys_perf_event_open: pid 10318  cpu 4  group_fd -1  flags 0x8 = 18
>>    sys_perf_event_open: pid 10318  cpu 5  group_fd -1  flags 0x8 = 19
>>    sys_perf_event_open: pid 10318  cpu 6  group_fd -1  flags 0x8 = 20
>>    sys_perf_event_open: pid 10318  cpu 7  group_fd -1  flags 0x8 = 21
>>    <SNIP>
>>
>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>   tools/perf/builtin-record.c | 59 +++++++++++++++++++++++--------------
>>   1 file changed, 37 insertions(+), 22 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 12edad8392cc..4ee94058028f 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -906,6 +906,37 @@ static int record__config_off_cpu(struct record *rec)
>>          return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
>>   }
>>
>> +static int record__config_tracking_events(struct record *rec)
>> +{
>> +       struct record_opts *opts = &rec->opts;
>> +       struct evlist *evlist = rec->evlist;
>> +       struct evsel *evsel;
>> +
>> +       /*
>> +        * For initial_delay, system wide or a hybrid system, we need to add a
>> +        * dummy event so that we can track PERF_RECORD_MMAP to cover the delay
> 
> This code is explicitly talking about dummy events but the function is
> now renamed to call it a tracking event. I think the code should be
> consistent.
> 
>> +        * of waiting or event synthesis.
>> +        */
>> +       if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
>> +           perf_pmus__num_core_pmus() > 1) {
>> +               evsel = evlist__findnew_tracking_event(evlist, false);
>> +               if (!evsel)
>> +                       return -ENOMEM;
>> +
>> +               /*
>> +                * Enable the dummy event when the process is forked for
> 
> Dummy again.
> 
This comment directly copies the original content.
I can change to "tracking event" in the next version.

Thanks,
Yang

Re: [PATCH v6 4/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Yang Jihong]

Hello,

On 2023/8/25 13:17, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> User space tasks can migrate between CPUs, we need to track side-band
>> events for all CPUs.
>>
>> The specific scenarios are as follows:
>>
>>           CPU0                                 CPU1
>>    perf record -C 0 start
>>                                taskA starts to be created and executed
>>                                  -> PERF_RECORD_COMM and PERF_RECORD_MMAP
>>                                     events only deliver to CPU1
>>                                ......
>>                                  |
>>                            migrate to CPU0
>>                                  |
>>    Running on CPU0    <----------/
>>    ...
>>
>>    perf record -C 0 stop
>>
>> Now perf samples the PC of taskA. However, perf does not record the
>> PERF_RECORD_COMM and PERF_RECORD_MMAP events of taskA.
>> Therefore, the comm and symbols of taskA cannot be parsed.
>>
>> The solution is to record sideband events for all CPUs when tracing
>> selected CPUs. Because this modifies the default behavior, add related
>> comments to the perf record man page.
>>
>> The sys_perf_event_open invoked is as follows:
>>
>>    # perf --debug verbose=3 record -e cpu-clock -C 1 true
>>    <SNIP>
>>    Opening: cpu-clock
>>    ------------------------------------------------------------
>>    perf_event_attr:
>>      type                             1 (PERF_TYPE_SOFTWARE)
>>      size                             136
>>      config                           0 (PERF_COUNT_SW_CPU_CLOCK)
>>      { sample_period, sample_freq }   4000
>>      sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
>>      read_format                      ID|LOST
>>      disabled                         1
>>      inherit                          1
>>      freq                             1
>>      sample_id_all                    1
>>      exclude_guest                    1
>>    ------------------------------------------------------------
>>    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
>>    Opening: dummy:u
>>    ------------------------------------------------------------
>>    perf_event_attr:
>>      type                             1 (PERF_TYPE_SOFTWARE)
>>      size                             136
>>      config                           0x9 (PERF_COUNT_SW_DUMMY)
>>      { sample_period, sample_freq }   1
>>      sample_type                      IP|TID|TIME|CPU|IDENTIFIER
>>      read_format                      ID|LOST
>>      inherit                          1
>>      exclude_kernel                   1
>>      exclude_hv                       1
>>      mmap                             1
>>      comm                             1
>>      task                             1
>>      sample_id_all                    1
>>      exclude_guest                    1
>>      mmap2                            1
>>      comm_exec                        1
>>      ksymbol                          1
>>      bpf_event                        1
>>    ------------------------------------------------------------
>>    sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
>>    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
>>    sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
>>    sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
>>    sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
>>    sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
>>    sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
>>    sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
>>    <SNIP>
>>
>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>   tools/perf/Documentation/perf-record.txt |  3 ++
>>   tools/perf/builtin-record.c              | 44 +++++++++++++++++++++++-
>>   2 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
>> index d5217be012d7..1889f66addf2 100644
>> --- a/tools/perf/Documentation/perf-record.txt
>> +++ b/tools/perf/Documentation/perf-record.txt
>> @@ -374,6 +374,9 @@ comma-separated list with no space: 0,1. Ranges of CPUs are specified with -: 0-
>>   In per-thread mode with inheritance mode on (default), samples are captured only when
>>   the thread executes on the designated CPUs. Default is to monitor all CPUs.
>>
>> +User space tasks can migrate between CPUs, so when tracing selected CPUs,
>> +a dummy event is created to track sideband for all CPUs.
>> +
>>   -B::
>>   --no-buildid::
>>   Do not save the build ids of binaries in the perf.data files. This skips
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 4ee94058028f..ae2e21b945fa 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -906,10 +906,44 @@ static int record__config_off_cpu(struct record *rec)
>>          return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
>>   }
>>
>> +static bool record__tracking_system_wide(struct record *rec)
> 
> I think this would be better named something like:
> record__need_system_wide_dummy_event
> 
Okay, it'll be modified in the next version.

Thanks,
Yang

Re: [PATCH v6 5/7] perf test: Update base-record & system-wide-dummy attr expected values for test-record-C0

[Yang Jihong]

Hello,

On 2023/8/25 13:22, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> 1. Because a dummy sideband event is added to the sampling of specified
>>     CPUs. When evlist contains evsel of different sample_type,
>>     evlist__config() will change the default PERF_SAMPLE_ID bit to
>>     PERF_SAMPLE_IDENTIFICATION bit.
>>     The attr sample_type expected value of base-record and system-wide-dummy
>>     in test-record-C0 needs to be updated.
>>
>> 2. The perf record uses evlist__add_aux_dummy() instead of
>>     evlist__add_dummy() to add a dummy event.
>>     The expected value of system-wide-dummy attr needs to be updated.
>>
>> The perf test result is as follows:
>>
>>    # ./perf test list  2>&1 | grep 'Setup struct perf_event_attr'
>>     17: Setup struct perf_event_attr
>>    # ./perf test 17
>>     17: Setup struct perf_event_attr                                    : Ok
> 
> Will the kernel test robot complain about the previous patches
> breaking this test? It may be best to update the test while changing
> the behavior in those patches.
> 
Okay, it'll be modified in the next version.

Thanks,
Yang

Re: [PATCH v6 6/7] perf test: Add test case for record sideband events

[Yang Jihong]

Hello,

On 2023/8/25 13:28, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> Add a new test case to record sideband events for all CPUs when tracing
>> selected CPUs
>>
>> Test result:
>>
>>    # ./perf test list 2>&1 | grep 'perf record sideband tests'
>>     95: perf record sideband tests
>>    # ./perf test 95
>>     95: perf record sideband tests                                      : Ok
>>
>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>> Tested-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>   tools/perf/tests/shell/record_sideband.sh | 44 +++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>   create mode 100755 tools/perf/tests/shell/record_sideband.sh
>>
>> diff --git a/tools/perf/tests/shell/record_sideband.sh b/tools/perf/tests/shell/record_sideband.sh
>> new file mode 100755
>> index 000000000000..2ecf00011cb1
>> --- /dev/null
>> +++ b/tools/perf/tests/shell/record_sideband.sh
>> @@ -0,0 +1,44 @@
>> +#!/bin/sh
>> +# perf record sideband tests
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +set -e
>> +
>> +err=0
>> +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> 
> Could you add some kind of cleanup on trap function like:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/record.sh?h=perf-tools-next#n26
> It'd be okay to do this as a follow up patch.
> 
Okay, it'll be modified in the next version.

Thanks,
Yang

Re: [PATCH v6 4/7] perf record: Track sideband events for all CPUs when tracing selected CPUs

[Adrian Hunter]

On 25/08/23 08:17, Ian Rogers wrote:
> On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong <yangjihong1@huawei.com> wrote:
>>
>> User space tasks can migrate between CPUs, we need to track side-band
>> events for all CPUs.
>>
>> The specific scenarios are as follows:
>>
>>          CPU0                                 CPU1
>>   perf record -C 0 start
>>                               taskA starts to be created and executed
>>                                 -> PERF_RECORD_COMM and PERF_RECORD_MMAP
>>                                    events only deliver to CPU1
>>                               ......
>>                                 |
>>                           migrate to CPU0
>>                                 |
>>   Running on CPU0    <----------/
>>   ...
>>
>>   perf record -C 0 stop
>>
>> Now perf samples the PC of taskA. However, perf does not record the
>> PERF_RECORD_COMM and PERF_RECORD_MMAP events of taskA.
>> Therefore, the comm and symbols of taskA cannot be parsed.
>>
>> The solution is to record sideband events for all CPUs when tracing
>> selected CPUs. Because this modifies the default behavior, add related
>> comments to the perf record man page.
>>
>> The sys_perf_event_open invoked is as follows:
>>
>>   # perf --debug verbose=3 record -e cpu-clock -C 1 true
>>   <SNIP>
>>   Opening: cpu-clock
>>   ------------------------------------------------------------
>>   perf_event_attr:
>>     type                             1 (PERF_TYPE_SOFTWARE)
>>     size                             136
>>     config                           0 (PERF_COUNT_SW_CPU_CLOCK)
>>     { sample_period, sample_freq }   4000
>>     sample_type                      IP|TID|TIME|CPU|PERIOD|IDENTIFIER
>>     read_format                      ID|LOST
>>     disabled                         1
>>     inherit                          1
>>     freq                             1
>>     sample_id_all                    1
>>     exclude_guest                    1
>>   ------------------------------------------------------------
>>   sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 5
>>   Opening: dummy:u
>>   ------------------------------------------------------------
>>   perf_event_attr:
>>     type                             1 (PERF_TYPE_SOFTWARE)
>>     size                             136
>>     config                           0x9 (PERF_COUNT_SW_DUMMY)
>>     { sample_period, sample_freq }   1
>>     sample_type                      IP|TID|TIME|CPU|IDENTIFIER
>>     read_format                      ID|LOST
>>     inherit                          1
>>     exclude_kernel                   1
>>     exclude_hv                       1
>>     mmap                             1
>>     comm                             1
>>     task                             1
>>     sample_id_all                    1
>>     exclude_guest                    1
>>     mmap2                            1
>>     comm_exec                        1
>>     ksymbol                          1
>>     bpf_event                        1
>>   ------------------------------------------------------------
>>   sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 6
>>   sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 7
>>   sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 9
>>   sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 10
>>   sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 11
>>   sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 12
>>   sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 13
>>   sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 14
>>   <SNIP>
>>
>> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/Documentation/perf-record.txt |  3 ++
>>  tools/perf/builtin-record.c              | 44 +++++++++++++++++++++++-
>>  2 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
>> index d5217be012d7..1889f66addf2 100644
>> --- a/tools/perf/Documentation/perf-record.txt
>> +++ b/tools/perf/Documentation/perf-record.txt
>> @@ -374,6 +374,9 @@ comma-separated list with no space: 0,1. Ranges of CPUs are specified with -: 0-
>>  In per-thread mode with inheritance mode on (default), samples are captured only when
>>  the thread executes on the designated CPUs. Default is to monitor all CPUs.
>>
>> +User space tasks can migrate between CPUs, so when tracing selected CPUs,
>> +a dummy event is created to track sideband for all CPUs.
>> +
>>  -B::
>>  --no-buildid::
>>  Do not save the build ids of binaries in the perf.data files. This skips
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 4ee94058028f..ae2e21b945fa 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -906,10 +906,44 @@ static int record__config_off_cpu(struct record *rec)
>>         return off_cpu_prepare(rec->evlist, &rec->opts.target, &rec->opts);
>>  }
>>
>> +static bool record__tracking_system_wide(struct record *rec)
> 
> I think this would be better named something like:
> record__need_system_wide_dummy_event

Please no.  The tracking event is a perf tool invention identified
by evsel->tracking and used in evsel__config(). Please don't
confuse it with dummy events.  Just because the tracking event
*might* be a dummy event does not mean a dummy event is the
tracking event.

> 
> Thanks,
> Ian
> 
>> +{
>> +       struct record_opts *opts = &rec->opts;
>> +       struct evlist *evlist = rec->evlist;
>> +       struct evsel *evsel;
>> +
>> +       /*
>> +        * If all (non-dummy) evsel have exclude_user,
>> +        * system_wide is not needed.
>> +        *
>> +        * all_kernel and all_user will overwrite exclude_kernel and
>> +        * exclude_user of attr in evsel__config(), here need to check
>> +        * all the three items.
>> +        *
>> +        * Sideband system wide if one of the following conditions is met:
>> +        *
>> +        *   - all_user is set, and there is a non-dummy event
>> +        *   - all_user and all_kernel are not set, and there is
>> +        *     a non-dummy event without exclude_user
>> +        */
>> +       if (opts->all_kernel)
>> +               return false;
>> +
>> +       evlist__for_each_entry(evlist, evsel) {
>> +               if (!evsel__is_dummy_event(evsel)) {
>> +                       if (opts->all_user || !evsel->core.attr.exclude_user)
>> +                               return true;
>> +               }
>> +       }
>> +
>> +       return false;
>> +}
>> +
>>  static int record__config_tracking_events(struct record *rec)
>>  {
>>         struct record_opts *opts = &rec->opts;
>>         struct evlist *evlist = rec->evlist;
>> +       bool system_wide = false;
>>         struct evsel *evsel;
>>
>>         /*
>> @@ -919,7 +953,15 @@ static int record__config_tracking_events(struct record *rec)
>>          */
>>         if (opts->target.initial_delay || target__has_cpu(&opts->target) ||
>>             perf_pmus__num_core_pmus() > 1) {
>> -               evsel = evlist__findnew_tracking_event(evlist, false);
>> +
>> +               /*
>> +                * User space tasks can migrate between CPUs, so when tracing
>> +                * selected CPUs, sideband for all CPUs is still needed.
>> +                */
>> +               if (!!opts->target.cpu_list && record__tracking_system_wide(rec))
>> +                       system_wide = true;
>> +
>> +               evsel = evlist__findnew_tracking_event(evlist, system_wide);
>>                 if (!evsel)
>>                         return -ENOMEM;
>>
>> --
>> 2.30.GIT
>>

Re: [PATCH v6 1/7] perf evlist: Add perf_evlist__go_system_wide() helper

[Yang Jihong]

Hello,

On 2023/8/25 13:45, Ian Rogers wrote:
> 
> 
> On Thu, Aug 24, 2023, 10:41 PM Yang Jihong <yangjihong1@huawei.com 
> <mailto:yangjihong1@huawei.com>> wrote:
> 
>     Hello,
> 
>     On 2023/8/25 12:51, Ian Rogers wrote:
>      > On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong
>     <yangjihong1@huawei.com <mailto:yangjihong1@huawei.com>> wrote:
>      >>
>      >> For dummy events that keep tracking, we may need to modify its
>     cpu_maps.
>      >> For example, change the cpu_maps to record sideband events for
>     all CPUS.
>      >> Add perf_evlist__go_system_wide() helper to support this scenario.
>      >>
>      >> Signed-off-by: Yang Jihong <yangjihong1@huawei.com
>     <mailto:yangjihong1@huawei.com>>
>      >> Acked-by: Adrian Hunter <adrian.hunter@intel.com
>     <mailto:adrian.hunter@intel.com>>
>      >> ---
>      >>   tools/lib/perf/evlist.c                  | 9 +++++++++
>      >>   tools/lib/perf/include/internal/evlist.h | 2 ++
>      >>   2 files changed, 11 insertions(+)
>      >>
>      >> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
>      >> index b8b066d0dc5e..3acbbccc1901 100644
>      >> --- a/tools/lib/perf/evlist.c
>      >> +++ b/tools/lib/perf/evlist.c
>      >> @@ -738,3 +738,12 @@ int perf_evlist__nr_groups(struct
>     perf_evlist *evlist)
>      >>          }
>      >>          return nr_groups;
>      >>   }
>      >> +
>      >> +void perf_evlist__go_system_wide(struct perf_evlist *evlist,
>     struct perf_evsel *evsel)
>      >> +{
>      >> +       if (!evsel->system_wide) {
>      >> +               evsel->system_wide = true;
>      >> +               if (evlist->needs_map_propagation)
>      >> +                       __perf_evlist__propagate_maps(evlist,
>     evsel);
>      >> +       }
>      >> +}
>      >
>      > I think this should be:
>      >
>      > void evsel__set_system_wide(struct evsel *evsel)
>      > {
>      >          if (evsel->system_wide)
>      >                 return;
>      >          evsel->system_wide = true;
>      >          if (evsel->evlist->core.needs_map_propagation)
>      > ...
>      >
>      > The API being on evlist makes it look like all evsels are affected.
>      >
>     This part of the code is implemented according to Adrian's suggestion.
>     Refer to:
> 
>     https://lore.kernel.org/linux-perf-users/206972a3-d44d-1c75-3fbc-426427614543@intel.com/
>     <https://lore.kernel.org/linux-perf-users/206972a3-d44d-1c75-3fbc-426427614543@intel.com/>
> 
>     The logic of both is the same, and either is OK for me.
>     If really want to change it, please let me know.
> 
> 
> Yes, I think the naming isn't correct and the function being on evlist 
> is misleading.
> 
Okay, I'll follow the above changes in the next version.

Thanks,
Yang

Re: [PATCH v6 1/7] perf evlist: Add perf_evlist__go_system_wide() helper

[Yang Jihong]

Hello,

On 2023/8/25 13:45, Ian Rogers wrote:
> 
> 
> On Thu, Aug 24, 2023, 10:41 PM Yang Jihong <yangjihong1@huawei.com 
> <mailto:yangjihong1@huawei.com>> wrote:
> 
>     Hello,
> 
>     On 2023/8/25 12:51, Ian Rogers wrote:
>      > On Sun, Aug 20, 2023 at 6:30 PM Yang Jihong
>     <yangjihong1@huawei.com <mailto:yangjihong1@huawei.com>> wrote:
>      >>
>      >> For dummy events that keep tracking, we may need to modify its
>     cpu_maps.
>      >> For example, change the cpu_maps to record sideband events for
>     all CPUS.
>      >> Add perf_evlist__go_system_wide() helper to support this scenario.
>      >>
>      >> Signed-off-by: Yang Jihong <yangjihong1@huawei.com
>     <mailto:yangjihong1@huawei.com>>
>      >> Acked-by: Adrian Hunter <adrian.hunter@intel.com
>     <mailto:adrian.hunter@intel.com>>
>      >> ---
>      >>   tools/lib/perf/evlist.c                  | 9 +++++++++
>      >>   tools/lib/perf/include/internal/evlist.h | 2 ++
>      >>   2 files changed, 11 insertions(+)
>      >>
>      >> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
>      >> index b8b066d0dc5e..3acbbccc1901 100644
>      >> --- a/tools/lib/perf/evlist.c
>      >> +++ b/tools/lib/perf/evlist.c
>      >> @@ -738,3 +738,12 @@ int perf_evlist__nr_groups(struct
>     perf_evlist *evlist)
>      >>          }
>      >>          return nr_groups;
>      >>   }
>      >> +
>      >> +void perf_evlist__go_system_wide(struct perf_evlist *evlist,
>     struct perf_evsel *evsel)
>      >> +{
>      >> +       if (!evsel->system_wide) {
>      >> +               evsel->system_wide = true;
>      >> +               if (evlist->needs_map_propagation)
>      >> +                       __perf_evlist__propagate_maps(evlist,
>     evsel);
>      >> +       }
>      >> +}
>      >
>      > I think this should be:
>      >
>      > void evsel__set_system_wide(struct evsel *evsel)
>      > {
>      >          if (evsel->system_wide)
>      >                 return;
>      >          evsel->system_wide = true;
>      >          if (evsel->evlist->core.needs_map_propagation)
>      > ...
>      >
>      > The API being on evlist makes it look like all evsels are affected.
>      >
>     This part of the code is implemented according to Adrian's suggestion.
>     Refer to:
> 
>     https://lore.kernel.org/linux-perf-users/206972a3-d44d-1c75-3fbc-426427614543@intel.com/
>     <https://lore.kernel.org/linux-perf-users/206972a3-d44d-1c75-3fbc-426427614543@intel.com/>
> 
>     The logic of both is the same, and either is OK for me.
>     If really want to change it, please let me know.
> 
> 
> Yes, I think the naming isn't correct and the function being on evlist 
> is misleading.
Uh, I have a little problem here, too.

Because perf_evlist__go_system_wide() needs to invoke the 
__perf_evlist__propagate_maps(), which is a local function and is 
located in the evlist.c file in tools/lib/perf/.
So perf_evlist__go_system_wide() can only be located in this file. The 
prefixes of all funcstions in this file are "perf_evlist__". Therefore, 
it is better to use the original names.

In addition, __perf_evlist__propagate_maps affects the evlist, so it is 
not misleading.

Thanks,
Yang