All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] perf cgroup: simplify arguments if track multiple events for a cgroup
@ 2018-01-28  5:31 ` weiping zhang
  0 siblings, 0 replies; 4+ messages in thread
From: weiping zhang @ 2018-01-28  5:31 UTC (permalink / raw)
  To: peterz, mingo, acme, alexander.shishkin, jolsa, namhyung
  Cc: linux-kernel, linux-perf-users

if use -G with one cgroup and -e with multiple events, only the first
event has correct cgroup setting, all events from the second will track
system-wide events.

if user want track multiple events for a specific cgroup, user must give
parameters like follow:
$ perf stat -e e1 -e e2 -e e3 -G test,test,test
this patch simplify this case, just type one cgroup, like following:
$ perf stat -e e1 -e e2 -e e3 -G test

reproduce:
$ mkdir -p /sys/fs/cgroup/perf_event/test
$ perf stat -e cycles -e cache-misses  -a -I 1000 -G test

before:
     1.001007226      <not counted>      cycles                    test
     1.001007226              7,506      cache-misses

after:
     1.000834097      <not counted>      cycles                    test
     1.000834097      <not counted>      cache-misses              test

Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
---
 tools/perf/util/cgroup.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index d9ffc1e..38aaeb2 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -154,9 +154,11 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
 		  int unset __maybe_unused)
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
+	struct perf_evsel *counter;
+	struct cgroup_sel *cgrp = NULL;
 	const char *p, *e, *eos = str + strlen(str);
 	char *s;
-	int ret;
+	int ret, i;
 
 	if (list_empty(&evlist->entries)) {
 		fprintf(stderr, "must define events before cgroups\n");
@@ -185,5 +187,18 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
 			break;
 		str = p+1;
 	}
+	/* for the case one cgroup combine to multiple events */
+	i = 0;
+	if (nr_cgroups == 1) {
+		evlist__for_each_entry(evlist, counter) {
+			if (i == 0)
+				cgrp = counter->cgrp;
+			else {
+				counter->cgrp = cgrp;
+				refcount_inc(&cgrp->refcnt);
+			}
+			i++;
+		}
+	}
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [RFC PATCH] perf cgroup: simplify arguments if track multiple events for a cgroup
@ 2018-01-28  5:31 ` weiping zhang
  0 siblings, 0 replies; 4+ messages in thread
From: weiping zhang @ 2018-01-28  5:31 UTC (permalink / raw)
  To: peterz, mingo, acme, alexander.shishkin, jolsa, namhyung
  Cc: linux-kernel, linux-perf-users

if use -G with one cgroup and -e with multiple events, only the first
event has correct cgroup setting, all events from the second will track
system-wide events.

if user want track multiple events for a specific cgroup, user must give
parameters like follow:
$ perf stat -e e1 -e e2 -e e3 -G test,test,test
this patch simplify this case, just type one cgroup, like following:
$ perf stat -e e1 -e e2 -e e3 -G test

reproduce:
$ mkdir -p /sys/fs/cgroup/perf_event/test
$ perf stat -e cycles -e cache-misses  -a -I 1000 -G test

before:
     1.001007226      <not counted>      cycles                    test
     1.001007226              7,506      cache-misses

after:
     1.000834097      <not counted>      cycles                    test
     1.000834097      <not counted>      cache-misses              test

Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
---
 tools/perf/util/cgroup.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index d9ffc1e..38aaeb2 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -154,9 +154,11 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
 		  int unset __maybe_unused)
 {
 	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
+	struct perf_evsel *counter;
+	struct cgroup_sel *cgrp = NULL;
 	const char *p, *e, *eos = str + strlen(str);
 	char *s;
-	int ret;
+	int ret, i;
 
 	if (list_empty(&evlist->entries)) {
 		fprintf(stderr, "must define events before cgroups\n");
@@ -185,5 +187,18 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
 			break;
 		str = p+1;
 	}
+	/* for the case one cgroup combine to multiple events */
+	i = 0;
+	if (nr_cgroups == 1) {
+		evlist__for_each_entry(evlist, counter) {
+			if (i == 0)
+				cgrp = counter->cgrp;
+			else {
+				counter->cgrp = cgrp;
+				refcount_inc(&cgrp->refcnt);
+			}
+			i++;
+		}
+	}
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [RFC PATCH] perf cgroup: simplify arguments if track multiple events for a cgroup
  2018-01-28  5:31 ` weiping zhang
  (?)
@ 2018-01-29 10:39 ` Jiri Olsa
  2018-01-29 15:19   ` weiping zhang
  -1 siblings, 1 reply; 4+ messages in thread
From: Jiri Olsa @ 2018-01-29 10:39 UTC (permalink / raw)
  To: peterz, mingo, acme, alexander.shishkin, namhyung, linux-kernel,
	linux-perf-users

On Sun, Jan 28, 2018 at 01:31:48PM +0800, weiping zhang wrote:
> if use -G with one cgroup and -e with multiple events, only the first
> event has correct cgroup setting, all events from the second will track
> system-wide events.
> 
> if user want track multiple events for a specific cgroup, user must give
> parameters like follow:
> $ perf stat -e e1 -e e2 -e e3 -G test,test,test
> this patch simplify this case, just type one cgroup, like following:
> $ perf stat -e e1 -e e2 -e e3 -G test
> 
> reproduce:
> $ mkdir -p /sys/fs/cgroup/perf_event/test
> $ perf stat -e cycles -e cache-misses  -a -I 1000 -G test
> 
> before:
>      1.001007226      <not counted>      cycles                    test
>      1.001007226              7,506      cache-misses
> 
> after:
>      1.000834097      <not counted>      cycles                    test
>      1.000834097      <not counted>      cache-misses              test
> 
> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
> ---
>  tools/perf/util/cgroup.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

please update the -G option documentation

> 
> diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
> index d9ffc1e..38aaeb2 100644
> --- a/tools/perf/util/cgroup.c
> +++ b/tools/perf/util/cgroup.c
> @@ -154,9 +154,11 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
>  		  int unset __maybe_unused)
>  {
>  	struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
> +	struct perf_evsel *counter;
> +	struct cgroup_sel *cgrp = NULL;
>  	const char *p, *e, *eos = str + strlen(str);
>  	char *s;
> -	int ret;
> +	int ret, i;
>  
>  	if (list_empty(&evlist->entries)) {
>  		fprintf(stderr, "must define events before cgroups\n");
> @@ -185,5 +187,18 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
>  			break;
>  		str = p+1;
>  	}
> +	/* for the case one cgroup combine to multiple events */
> +	i = 0;
> +	if (nr_cgroups == 1) {
> +		evlist__for_each_entry(evlist, counter) {
> +			if (i == 0)
> +				cgrp = counter->cgrp;

what if the only group is not defined for the first event? like  -G ,,krava

thanks,
jirka

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [RFC PATCH] perf cgroup: simplify arguments if track multiple events for a cgroup
  2018-01-29 10:39 ` Jiri Olsa
@ 2018-01-29 15:19   ` weiping zhang
  0 siblings, 0 replies; 4+ messages in thread
From: weiping zhang @ 2018-01-29 15:19 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: peterz, mingo, acme, alexander.shishkin, namhyung,
	Linux Kernel Mailing List, linux-perf-users

2018-01-29 18:39 GMT+08:00 Jiri Olsa <jolsa@redhat.com>:
> On Sun, Jan 28, 2018 at 01:31:48PM +0800, weiping zhang wrote:
>> if use -G with one cgroup and -e with multiple events, only the first
>> event has correct cgroup setting, all events from the second will track
>> system-wide events.
>>
>> if user want track multiple events for a specific cgroup, user must give
>> parameters like follow:
>> $ perf stat -e e1 -e e2 -e e3 -G test,test,test
>> this patch simplify this case, just type one cgroup, like following:
>> $ perf stat -e e1 -e e2 -e e3 -G test
>>
>> reproduce:
>> $ mkdir -p /sys/fs/cgroup/perf_event/test
>> $ perf stat -e cycles -e cache-misses  -a -I 1000 -G test
>>
>> before:
>>      1.001007226      <not counted>      cycles                    test
>>      1.001007226              7,506      cache-misses
>>
>> after:
>>      1.000834097      <not counted>      cycles                    test
>>      1.000834097      <not counted>      cache-misses              test
>>
>> Signed-off-by: weiping zhang <zhangweiping@didichuxing.com>
>> ---
>>  tools/perf/util/cgroup.c | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> please update the -G option documentation
>
OK, I'll add that at V2.
>>
>> diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
>> index d9ffc1e..38aaeb2 100644
>> --- a/tools/perf/util/cgroup.c
>> +++ b/tools/perf/util/cgroup.c
>> @@ -154,9 +154,11 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
>>                 int unset __maybe_unused)
>>  {
>>       struct perf_evlist *evlist = *(struct perf_evlist **)opt->value;
>> +     struct perf_evsel *counter;
>> +     struct cgroup_sel *cgrp = NULL;
>>       const char *p, *e, *eos = str + strlen(str);
>>       char *s;
>> -     int ret;
>> +     int ret, i;
>>
>>       if (list_empty(&evlist->entries)) {
>>               fprintf(stderr, "must define events before cgroups\n");
>> @@ -185,5 +187,18 @@ int parse_cgroups(const struct option *opt __maybe_unused, const char *str,
>>                       break;
>>               str = p+1;
>>       }
>> +     /* for the case one cgroup combine to multiple events */
>> +     i = 0;
>> +     if (nr_cgroups == 1) {
>> +             evlist__for_each_entry(evlist, counter) {
>> +                     if (i == 0)
>> +                             cgrp = counter->cgrp;
>
> what if the only group is not defined for the first event? like  -G ,,krava
>
if there is a comma, the nr_cgrous will > 1, so it will keep original
behavior track system-wide events.
> thanks,
> jirka

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-29 15:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-28  5:31 [RFC PATCH] perf cgroup: simplify arguments if track multiple events for a cgroup weiping zhang
2018-01-28  5:31 ` weiping zhang
2018-01-29 10:39 ` Jiri Olsa
2018-01-29 15:19   ` weiping zhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.