diff mbox series

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

Message ID 20230804020741.99806-4-yangjihong1@huawei.com
State New
Headers
Series perf record: Track sideband events for all CPUs when tracing selected CPUs |

Commit Message

Yang Jihong Aug. 4, 2023, 2:07 a.m. UTC 
  When dummy tracking go system wide, the mmap cpu mask is changed.
Therefore, needs to be placed 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>
---
 tools/perf/builtin-record.c | 59 +++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 22 deletions(-)

Comments

Adrian Hunter Aug. 4, 2023, 6:58 a.m. UTC | #1 
On 4/08/23 05:07, Yang Jihong wrote:
> When dummy tracking go system wide, the mmap cpu mask is changed.
> Therefore, needs to be placed 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 ca83599cc50c..3ff9d972225e 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -908,6 +908,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];
> @@ -1288,28 +1319,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) {
> @@ -4235,6 +4244,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");
Yang Jihong Aug. 15, 2023, 1:57 a.m. UTC | #2 
Hello,

On 2023/8/15 4:29, Ian Rogers wrote:
> On Thu, Aug 3, 2023 at 11:58 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 4/08/23 05:07, Yang Jihong wrote:
>>> When dummy tracking go system wide, the mmap cpu mask is changed.
> 
> As previously commented, can we improve the quality of the function
> names and commit messages? This sentence is particularly difficult to
> understand, I don't understand it.

OK. The commit messages will be modified. Please check whether the 
following description is clear:

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.

Thanks,
Yang
Yang Jihong Aug. 17, 2023, 12:27 p.m. UTC | #3 
Hello,

On 2023/8/15 9:57, Yang Jihong wrote:
> Hello,
> 
> On 2023/8/15 4:29, Ian Rogers wrote:
>> On Thu, Aug 3, 2023 at 11:58 PM Adrian Hunter 
>> <adrian.hunter@intel.com> wrote:
>>>
>>> On 4/08/23 05:07, Yang Jihong wrote:
>>>> When dummy tracking go system wide, the mmap cpu mask is changed.
>>
>> As previously commented, can we improve the quality of the function
>> names and commit messages? This sentence is particularly difficult to
>> understand, I don't understand it.
> 
> OK. The commit messages will be modified. Please check whether the 
> following description is clear:
> 
> 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.
> 
Ian, do you have any questions about the commit message above? If it's 
okay, I'll send the next version as above.

Thanks,
Yang
diff mbox series

Patch

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ca83599cc50c..3ff9d972225e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -908,6 +908,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];
@@ -1288,28 +1319,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) {
@@ -4235,6 +4244,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");