linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf tools: Move probing cgroup sampling support
@ 2021-05-27 18:28 Namhyung Kim
  2021-05-30 22:15 ` Jiri Olsa
  2021-06-01 13:30 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2021-05-27 18:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
	LKML, Andi Kleen, Ian Rogers

I found that checking cgroup sampling support using the missing
features doesn't work on old kernels.  Because it added both
attr.cgroup bit and PERF_SAMPLE_CGROUP bit, it needs to check
whichever comes first (usually the actual event, not dummy).

But it only checks the attr.cgroup bit which is set only in the dummy
event so cannot detect failtures due the sample bits.  Also we don't
ignore the missing feature and retry, it'd be better checking it with
the API probing logic.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c      |  6 ++++++
 tools/perf/util/evsel.c          |  6 +-----
 tools/perf/util/evsel.h          |  1 -
 tools/perf/util/perf_api_probe.c | 10 ++++++++++
 tools/perf/util/perf_api_probe.h |  1 +
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bc3dd379eb67..71efe6573ee7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2733,6 +2733,12 @@ int cmd_record(int argc, const char **argv)
 		rec->no_buildid = true;
 	}
 
+	if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
+		pr_err("Kernel has no cgroup sampling support.\n");
+		err = -EINVAL;
+		goto out_opts;
+	}
+
 	if (rec->opts.kcore)
 		rec->data.is_dir = true;
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4a3cd1b5bb33..2462584d0ee5 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1217,7 +1217,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
 		attr->namespaces  = track;
 
 	if (opts->record_cgroup) {
-		attr->cgroup = track && !perf_missing_features.cgroup;
+		attr->cgroup = track;
 		evsel__set_sample_bit(evsel, CGROUP);
 	}
 
@@ -1933,10 +1933,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 		perf_missing_features.data_page_size = true;
 		pr_debug2_peo("Kernel has no PERF_SAMPLE_DATA_PAGE_SIZE support, bailing out\n");
 		goto out_close;
-	} else if (!perf_missing_features.cgroup && evsel->core.attr.cgroup) {
-		perf_missing_features.cgroup = true;
-		pr_debug2_peo("Kernel has no cgroup sampling support, bailing out\n");
-		goto out_close;
         } else if (!perf_missing_features.branch_hw_idx &&
 	    (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_HW_INDEX)) {
 		perf_missing_features.branch_hw_idx = true;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 75cf5dbfe208..fecf13c0e4da 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -168,7 +168,6 @@ struct perf_missing_features {
 	bool bpf;
 	bool aux_output;
 	bool branch_hw_idx;
-	bool cgroup;
 	bool data_page_size;
 	bool code_page_size;
 	bool weight_struct;
diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c
index 829af17a0867..020411682a3c 100644
--- a/tools/perf/util/perf_api_probe.c
+++ b/tools/perf/util/perf_api_probe.c
@@ -103,6 +103,11 @@ static void perf_probe_build_id(struct evsel *evsel)
 	evsel->core.attr.build_id = 1;
 }
 
+static void perf_probe_cgroup(struct evsel *evsel)
+{
+	evsel->core.attr.cgroup = 1;
+}
+
 bool perf_can_sample_identifier(void)
 {
 	return perf_probe_api(perf_probe_sample_identifier);
@@ -182,3 +187,8 @@ bool perf_can_record_build_id(void)
 {
 	return perf_probe_api(perf_probe_build_id);
 }
+
+bool perf_can_record_cgroup(void)
+{
+	return perf_probe_api(perf_probe_cgroup);
+}
diff --git a/tools/perf/util/perf_api_probe.h b/tools/perf/util/perf_api_probe.h
index f12ca55f509a..b104168efb15 100644
--- a/tools/perf/util/perf_api_probe.h
+++ b/tools/perf/util/perf_api_probe.h
@@ -12,5 +12,6 @@ bool perf_can_record_switch_events(void);
 bool perf_can_record_text_poke_events(void);
 bool perf_can_sample_identifier(void);
 bool perf_can_record_build_id(void);
+bool perf_can_record_cgroup(void);
 
 #endif // __PERF_API_PROBE_H
-- 
2.32.0.rc0.204.g9fa02ecfa5-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf tools: Move probing cgroup sampling support
  2021-05-27 18:28 [PATCH] perf tools: Move probing cgroup sampling support Namhyung Kim
@ 2021-05-30 22:15 ` Jiri Olsa
  2021-06-01 13:30 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2021-05-30 22:15 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
	Mark Rutland, Alexander Shishkin, LKML, Andi Kleen, Ian Rogers

On Thu, May 27, 2021 at 11:28:35AM -0700, Namhyung Kim wrote:
> I found that checking cgroup sampling support using the missing
> features doesn't work on old kernels.  Because it added both
> attr.cgroup bit and PERF_SAMPLE_CGROUP bit, it needs to check
> whichever comes first (usually the actual event, not dummy).
> 
> But it only checks the attr.cgroup bit which is set only in the dummy
> event so cannot detect failtures due the sample bits.  Also we don't
> ignore the missing feature and retry, it'd be better checking it with
> the API probing logic.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> ---
>  tools/perf/builtin-record.c      |  6 ++++++
>  tools/perf/util/evsel.c          |  6 +-----
>  tools/perf/util/evsel.h          |  1 -
>  tools/perf/util/perf_api_probe.c | 10 ++++++++++
>  tools/perf/util/perf_api_probe.h |  1 +
>  5 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index bc3dd379eb67..71efe6573ee7 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2733,6 +2733,12 @@ int cmd_record(int argc, const char **argv)
>  		rec->no_buildid = true;
>  	}
>  
> +	if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
> +		pr_err("Kernel has no cgroup sampling support.\n");
> +		err = -EINVAL;
> +		goto out_opts;
> +	}
> +
>  	if (rec->opts.kcore)
>  		rec->data.is_dir = true;
>  
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 4a3cd1b5bb33..2462584d0ee5 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1217,7 +1217,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
>  		attr->namespaces  = track;
>  
>  	if (opts->record_cgroup) {
> -		attr->cgroup = track && !perf_missing_features.cgroup;
> +		attr->cgroup = track;
>  		evsel__set_sample_bit(evsel, CGROUP);
>  	}
>  
> @@ -1933,10 +1933,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
>  		perf_missing_features.data_page_size = true;
>  		pr_debug2_peo("Kernel has no PERF_SAMPLE_DATA_PAGE_SIZE support, bailing out\n");
>  		goto out_close;
> -	} else if (!perf_missing_features.cgroup && evsel->core.attr.cgroup) {
> -		perf_missing_features.cgroup = true;
> -		pr_debug2_peo("Kernel has no cgroup sampling support, bailing out\n");
> -		goto out_close;
>          } else if (!perf_missing_features.branch_hw_idx &&
>  	    (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_HW_INDEX)) {
>  		perf_missing_features.branch_hw_idx = true;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 75cf5dbfe208..fecf13c0e4da 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -168,7 +168,6 @@ struct perf_missing_features {
>  	bool bpf;
>  	bool aux_output;
>  	bool branch_hw_idx;
> -	bool cgroup;
>  	bool data_page_size;
>  	bool code_page_size;
>  	bool weight_struct;
> diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c
> index 829af17a0867..020411682a3c 100644
> --- a/tools/perf/util/perf_api_probe.c
> +++ b/tools/perf/util/perf_api_probe.c
> @@ -103,6 +103,11 @@ static void perf_probe_build_id(struct evsel *evsel)
>  	evsel->core.attr.build_id = 1;
>  }
>  
> +static void perf_probe_cgroup(struct evsel *evsel)
> +{
> +	evsel->core.attr.cgroup = 1;
> +}
> +
>  bool perf_can_sample_identifier(void)
>  {
>  	return perf_probe_api(perf_probe_sample_identifier);
> @@ -182,3 +187,8 @@ bool perf_can_record_build_id(void)
>  {
>  	return perf_probe_api(perf_probe_build_id);
>  }
> +
> +bool perf_can_record_cgroup(void)
> +{
> +	return perf_probe_api(perf_probe_cgroup);
> +}
> diff --git a/tools/perf/util/perf_api_probe.h b/tools/perf/util/perf_api_probe.h
> index f12ca55f509a..b104168efb15 100644
> --- a/tools/perf/util/perf_api_probe.h
> +++ b/tools/perf/util/perf_api_probe.h
> @@ -12,5 +12,6 @@ bool perf_can_record_switch_events(void);
>  bool perf_can_record_text_poke_events(void);
>  bool perf_can_sample_identifier(void);
>  bool perf_can_record_build_id(void);
> +bool perf_can_record_cgroup(void);
>  
>  #endif // __PERF_API_PROBE_H
> -- 
> 2.32.0.rc0.204.g9fa02ecfa5-goog
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf tools: Move probing cgroup sampling support
  2021-05-27 18:28 [PATCH] perf tools: Move probing cgroup sampling support Namhyung Kim
  2021-05-30 22:15 ` Jiri Olsa
@ 2021-06-01 13:30 ` Arnaldo Carvalho de Melo
  2021-06-01 13:34   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-01 13:30 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, Mark Rutland,
	Alexander Shishkin, LKML, Andi Kleen, Ian Rogers

Em Thu, May 27, 2021 at 11:28:35AM -0700, Namhyung Kim escreveu:
> I found that checking cgroup sampling support using the missing
> features doesn't work on old kernels.  Because it added both
> attr.cgroup bit and PERF_SAMPLE_CGROUP bit, it needs to check
> whichever comes first (usually the actual event, not dummy).
> 
> But it only checks the attr.cgroup bit which is set only in the dummy
> event so cannot detect failtures due the sample bits.  Also we don't
> ignore the missing feature and retry, it'd be better checking it with
> the API probing logic.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-record.c      |  6 ++++++
>  tools/perf/util/evsel.c          |  6 +-----
>  tools/perf/util/evsel.h          |  1 -
>  tools/perf/util/perf_api_probe.c | 10 ++++++++++
>  tools/perf/util/perf_api_probe.h |  1 +
>  5 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c
> index 829af17a0867..020411682a3c 100644
> --- a/tools/perf/util/perf_api_probe.c
> +++ b/tools/perf/util/perf_api_probe.c
> @@ -103,6 +103,11 @@ static void perf_probe_build_id(struct evsel *evsel)
>  	evsel->core.attr.build_id = 1;
>  }
>  
> +static void perf_probe_cgroup(struct evsel *evsel)
> +{
> +	evsel->core.attr.cgroup = 1;
> +}
> +
>  bool perf_can_sample_identifier(void)
>  {
>  	return perf_probe_api(perf_probe_sample_identifier);
> @@ -182,3 +187,8 @@ bool perf_can_record_build_id(void)
>  {
>  	return perf_probe_api(perf_probe_build_id);
>  }
> +
> +bool perf_can_record_cgroup(void)
> +{
> +	return perf_probe_api(perf_probe_cgroup);
> +}
> diff --git a/tools/perf/util/perf_api_probe.h b/tools/perf/util/perf_api_probe.h
> index f12ca55f509a..b104168efb15 100644
> --- a/tools/perf/util/perf_api_probe.h
> +++ b/tools/perf/util/perf_api_probe.h
> @@ -12,5 +12,6 @@ bool perf_can_record_switch_events(void);
>  bool perf_can_record_text_poke_events(void);
>  bool perf_can_sample_identifier(void);
>  bool perf_can_record_build_id(void);
> +bool perf_can_record_cgroup(void);
>  
>  #endif // __PERF_API_PROBE_H
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index bc3dd379eb67..71efe6573ee7 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2733,6 +2733,12 @@ int cmd_record(int argc, const char **argv)
>  		rec->no_buildid = true;
>  	}
>  
> +	if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
> +		pr_err("Kernel has no cgroup sampling support.\n");
> +		err = -EINVAL;
> +		goto out_opts;
> +	}
> +
>  	if (rec->opts.kcore)
>  		rec->data.is_dir = true;
>  

The above is perf/urgent material and should fix your issue, right?

The part below is a separate patch and can be left for later, or maybe
remain in the codebase, as simple tools that use just one evsel and
request a cgroup will continue probing the kernel, etc. I.e. it
shouldn't get in the way for cases with dummies, etc.

Simple tools then won't have to get that !perf_can_record_cgroup() call.

- Arnaldo

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 4a3cd1b5bb33..2462584d0ee5 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1217,7 +1217,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
>  		attr->namespaces  = track;
>  
>  	if (opts->record_cgroup) {
> -		attr->cgroup = track && !perf_missing_features.cgroup;
> +		attr->cgroup = track;
>  		evsel__set_sample_bit(evsel, CGROUP);
>  	}
>  
> @@ -1933,10 +1933,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
>  		perf_missing_features.data_page_size = true;
>  		pr_debug2_peo("Kernel has no PERF_SAMPLE_DATA_PAGE_SIZE support, bailing out\n");
>  		goto out_close;
> -	} else if (!perf_missing_features.cgroup && evsel->core.attr.cgroup) {
> -		perf_missing_features.cgroup = true;
> -		pr_debug2_peo("Kernel has no cgroup sampling support, bailing out\n");
> -		goto out_close;
>          } else if (!perf_missing_features.branch_hw_idx &&
>  	    (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_HW_INDEX)) {
>  		perf_missing_features.branch_hw_idx = true;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 75cf5dbfe208..fecf13c0e4da 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -168,7 +168,6 @@ struct perf_missing_features {
>  	bool bpf;
>  	bool aux_output;
>  	bool branch_hw_idx;
> -	bool cgroup;
>  	bool data_page_size;
>  	bool code_page_size;
>  	bool weight_struct;
> -- 
> 2.32.0.rc0.204.g9fa02ecfa5-goog
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf tools: Move probing cgroup sampling support
  2021-06-01 13:30 ` Arnaldo Carvalho de Melo
@ 2021-06-01 13:34   ` Arnaldo Carvalho de Melo
  2021-06-01 22:46     ` Namhyung Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-01 13:34 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, Mark Rutland,
	Alexander Shishkin, LKML, Andi Kleen, Ian Rogers

Em Tue, Jun 01, 2021 at 10:30:25AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, May 27, 2021 at 11:28:35AM -0700, Namhyung Kim escreveu:
> > I found that checking cgroup sampling support using the missing
> > features doesn't work on old kernels.  Because it added both
> > attr.cgroup bit and PERF_SAMPLE_CGROUP bit, it needs to check
> > whichever comes first (usually the actual event, not dummy).
> > 
> > But it only checks the attr.cgroup bit which is set only in the dummy
> > event so cannot detect failtures due the sample bits.  Also we don't
> > ignore the missing feature and retry, it'd be better checking it with
> > the API probing logic.
> > 
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-record.c      |  6 ++++++
> >  tools/perf/util/evsel.c          |  6 +-----
> >  tools/perf/util/evsel.h          |  1 -
> >  tools/perf/util/perf_api_probe.c | 10 ++++++++++
> >  tools/perf/util/perf_api_probe.h |  1 +
> >  5 files changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c
> > index 829af17a0867..020411682a3c 100644
> > --- a/tools/perf/util/perf_api_probe.c
> > +++ b/tools/perf/util/perf_api_probe.c
> > @@ -103,6 +103,11 @@ static void perf_probe_build_id(struct evsel *evsel)
> >  	evsel->core.attr.build_id = 1;
> >  }
> >  
> > +static void perf_probe_cgroup(struct evsel *evsel)
> > +{
> > +	evsel->core.attr.cgroup = 1;
> > +}
> > +
> >  bool perf_can_sample_identifier(void)
> >  {
> >  	return perf_probe_api(perf_probe_sample_identifier);
> > @@ -182,3 +187,8 @@ bool perf_can_record_build_id(void)
> >  {
> >  	return perf_probe_api(perf_probe_build_id);
> >  }
> > +
> > +bool perf_can_record_cgroup(void)
> > +{
> > +	return perf_probe_api(perf_probe_cgroup);
> > +}
> > diff --git a/tools/perf/util/perf_api_probe.h b/tools/perf/util/perf_api_probe.h
> > index f12ca55f509a..b104168efb15 100644
> > --- a/tools/perf/util/perf_api_probe.h
> > +++ b/tools/perf/util/perf_api_probe.h
> > @@ -12,5 +12,6 @@ bool perf_can_record_switch_events(void);
> >  bool perf_can_record_text_poke_events(void);
> >  bool perf_can_sample_identifier(void);
> >  bool perf_can_record_build_id(void);
> > +bool perf_can_record_cgroup(void);
> >  
> >  #endif // __PERF_API_PROBE_H
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index bc3dd379eb67..71efe6573ee7 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -2733,6 +2733,12 @@ int cmd_record(int argc, const char **argv)
> >  		rec->no_buildid = true;
> >  	}
> >  
> > +	if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
> > +		pr_err("Kernel has no cgroup sampling support.\n");
> > +		err = -EINVAL;
> > +		goto out_opts;
> > +	}
> > +
> >  	if (rec->opts.kcore)
> >  		rec->data.is_dir = true;
> >  
> 
> The above is perf/urgent material and should fix your issue, right?
> 
> The part below is a separate patch and can be left for later, or maybe
> remain in the codebase, as simple tools that use just one evsel and
> request a cgroup will continue probing the kernel, etc. I.e. it
> shouldn't get in the way for cases with dummies, etc.
> 
> Simple tools then won't have to get that !perf_can_record_cgroup() call.

I did it tentatively in my local branch, i.e. removed the removal of the
fallback part, thus is just for it to be tested by the containers setup,
etc, we can change this before it hits acme/perf/core externally.

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf tools: Move probing cgroup sampling support
  2021-06-01 13:34   ` Arnaldo Carvalho de Melo
@ 2021-06-01 22:46     ` Namhyung Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2021-06-01 22:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, Mark Rutland,
	Alexander Shishkin, LKML, Andi Kleen, Ian Rogers

Hi Arnaldo,

On Tue, Jun 1, 2021 at 6:34 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Tue, Jun 01, 2021 at 10:30:25AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, May 27, 2021 at 11:28:35AM -0700, Namhyung Kim escreveu:
> > > I found that checking cgroup sampling support using the missing
> > > features doesn't work on old kernels.  Because it added both
> > > attr.cgroup bit and PERF_SAMPLE_CGROUP bit, it needs to check
> > > whichever comes first (usually the actual event, not dummy).
> > >
> > > But it only checks the attr.cgroup bit which is set only in the dummy
> > > event so cannot detect failtures due the sample bits.  Also we don't
> > > ignore the missing feature and retry, it'd be better checking it with
> > > the API probing logic.
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/builtin-record.c      |  6 ++++++
> > >  tools/perf/util/evsel.c          |  6 +-----
> > >  tools/perf/util/evsel.h          |  1 -
> > >  tools/perf/util/perf_api_probe.c | 10 ++++++++++
> > >  tools/perf/util/perf_api_probe.h |  1 +
> > >  5 files changed, 18 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c
> > > index 829af17a0867..020411682a3c 100644
> > > --- a/tools/perf/util/perf_api_probe.c
> > > +++ b/tools/perf/util/perf_api_probe.c
> > > @@ -103,6 +103,11 @@ static void perf_probe_build_id(struct evsel *evsel)
> > >     evsel->core.attr.build_id = 1;
> > >  }
> > >
> > > +static void perf_probe_cgroup(struct evsel *evsel)
> > > +{
> > > +   evsel->core.attr.cgroup = 1;
> > > +}
> > > +
> > >  bool perf_can_sample_identifier(void)
> > >  {
> > >     return perf_probe_api(perf_probe_sample_identifier);
> > > @@ -182,3 +187,8 @@ bool perf_can_record_build_id(void)
> > >  {
> > >     return perf_probe_api(perf_probe_build_id);
> > >  }
> > > +
> > > +bool perf_can_record_cgroup(void)
> > > +{
> > > +   return perf_probe_api(perf_probe_cgroup);
> > > +}
> > > diff --git a/tools/perf/util/perf_api_probe.h b/tools/perf/util/perf_api_probe.h
> > > index f12ca55f509a..b104168efb15 100644
> > > --- a/tools/perf/util/perf_api_probe.h
> > > +++ b/tools/perf/util/perf_api_probe.h
> > > @@ -12,5 +12,6 @@ bool perf_can_record_switch_events(void);
> > >  bool perf_can_record_text_poke_events(void);
> > >  bool perf_can_sample_identifier(void);
> > >  bool perf_can_record_build_id(void);
> > > +bool perf_can_record_cgroup(void);
> > >
> > >  #endif // __PERF_API_PROBE_H
> > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > > index bc3dd379eb67..71efe6573ee7 100644
> > > --- a/tools/perf/builtin-record.c
> > > +++ b/tools/perf/builtin-record.c
> > > @@ -2733,6 +2733,12 @@ int cmd_record(int argc, const char **argv)
> > >             rec->no_buildid = true;
> > >     }
> > >
> > > +   if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
> > > +           pr_err("Kernel has no cgroup sampling support.\n");
> > > +           err = -EINVAL;
> > > +           goto out_opts;
> > > +   }
> > > +
> > >     if (rec->opts.kcore)
> > >             rec->data.is_dir = true;
> > >
> >
> > The above is perf/urgent material and should fix your issue, right?
> >
> > The part below is a separate patch and can be left for later, or maybe
> > remain in the codebase, as simple tools that use just one evsel and
> > request a cgroup will continue probing the kernel, etc. I.e. it
> > shouldn't get in the way for cases with dummies, etc.
> >
> > Simple tools then won't have to get that !perf_can_record_cgroup() call.
>
> I did it tentatively in my local branch, i.e. removed the removal of the
> fallback part, thus is just for it to be tested by the containers setup,
> etc, we can change this before it hits acme/perf/core externally.

Sure, thanks for doing this!

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-01 22:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 18:28 [PATCH] perf tools: Move probing cgroup sampling support Namhyung Kim
2021-05-30 22:15 ` Jiri Olsa
2021-06-01 13:30 ` Arnaldo Carvalho de Melo
2021-06-01 13:34   ` Arnaldo Carvalho de Melo
2021-06-01 22:46     ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).