| Message ID | 20230104064402.1551516-5-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | perf bpf_counter: A set of random fixes (v1) | expand |
Em Tue, Jan 03, 2023 at 10:44:02PM -0800, Namhyung Kim escreveu: > The --for-each-cgroup can have the same cgroup multiple times, but it makes > bpf counters confused (since they have the same cgroup id), and the last > cgroup events are counted only. Let's check the cgroup name before adding > a new entry. > > Before: > $ sudo ./perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1 > > Performance counter stats for 'system wide': > > <not counted> msec cpu-clock / > <not counted> context-switches / > <not counted> cpu-migrations / > <not counted> page-faults / > <not counted> cycles / > <not counted> instructions / > <not counted> branches / > <not counted> branch-misses / > 8,016.04 msec cpu-clock / # 7.998 CPUs utilized > 6,152 context-switches / # 767.461 /sec > 250 cpu-migrations / # 31.187 /sec > 442 page-faults / # 55.139 /sec > 613,111,487 cycles / # 0.076 GHz > 280,599,604 instructions / # 0.46 insn per cycle > 57,692,724 branches / # 7.197 M/sec > 3,385,168 branch-misses / # 5.87% of all branches > > 1.002220125 seconds time elapsed > > After: > $ sudo ./perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1 > > Performance counter stats for 'system wide': > > 8,013.38 msec cpu-clock / # 7.998 CPUs utilized > 6,859 context-switches / # 855.944 /sec > 334 cpu-migrations / # 41.680 /sec > 345 page-faults / # 43.053 /sec > 782,326,119 cycles / # 0.098 GHz > 471,645,724 instructions / # 0.60 insn per cycle > 94,963,430 branches / # 11.851 M/sec > 3,685,511 branch-misses / # 3.88% of all branches > > 1.001864539 seconds time elapsed > > Fixes: bb1c15b60b981 ("perf stat: Support regex pattern in --for-each-cgroup") > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Tested and appied. - Arnaldo > --- > tools/perf/util/cgroup.c | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c > index e99b41f9be45..cd978c240e0d 100644 > --- a/tools/perf/util/cgroup.c > +++ b/tools/perf/util/cgroup.c > @@ -224,6 +224,19 @@ static int add_cgroup_name(const char *fpath, const struct stat *sb __maybe_unus > return 0; > } > > +static int check_and_add_cgroup_name(const char *fpath) > +{ > + struct cgroup_name *cn; > + > + list_for_each_entry(cn, &cgroup_list, list) { > + if (!strcmp(cn->name, fpath)) > + return 0; > + } > + > + /* pretend if it's added by ftw() */ > + return add_cgroup_name(fpath, NULL, FTW_D, NULL); > +} > + > static void release_cgroup_list(void) > { > struct cgroup_name *cn; > @@ -242,7 +255,7 @@ static int list_cgroups(const char *str) > struct cgroup_name *cn; > char *s; > > - /* use given name as is - for testing purpose */ > + /* use given name as is when no regex is given */ > for (;;) { > p = strchr(str, ','); > e = p ? p : eos; > @@ -253,13 +266,13 @@ static int list_cgroups(const char *str) > s = strndup(str, e - str); > if (!s) > return -1; > - /* pretend if it's added by ftw() */ > - ret = add_cgroup_name(s, NULL, FTW_D, NULL); > + > + ret = check_and_add_cgroup_name(s); > free(s); > - if (ret) > + if (ret < 0) > return -1; > } else { > - if (add_cgroup_name("", NULL, FTW_D, NULL) < 0) > + if (check_and_add_cgroup_name("/") < 0) > return -1; > } > > -- > 2.39.0.314.g84b9a713c41-goog
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c index e99b41f9be45..cd978c240e0d 100644 --- a/tools/perf/util/cgroup.c +++ b/tools/perf/util/cgroup.c @@ -224,6 +224,19 @@ static int add_cgroup_name(const char *fpath, const struct stat *sb __maybe_unus return 0; } +static int check_and_add_cgroup_name(const char *fpath) +{ + struct cgroup_name *cn; + + list_for_each_entry(cn, &cgroup_list, list) { + if (!strcmp(cn->name, fpath)) + return 0; + } + + /* pretend if it's added by ftw() */ + return add_cgroup_name(fpath, NULL, FTW_D, NULL); +} + static void release_cgroup_list(void) { struct cgroup_name *cn; @@ -242,7 +255,7 @@ static int list_cgroups(const char *str) struct cgroup_name *cn; char *s; - /* use given name as is - for testing purpose */ + /* use given name as is when no regex is given */ for (;;) { p = strchr(str, ','); e = p ? p : eos; @@ -253,13 +266,13 @@ static int list_cgroups(const char *str) s = strndup(str, e - str); if (!s) return -1; - /* pretend if it's added by ftw() */ - ret = add_cgroup_name(s, NULL, FTW_D, NULL); + + ret = check_and_add_cgroup_name(s); free(s); - if (ret) + if (ret < 0) return -1; } else { - if (add_cgroup_name("", NULL, FTW_D, NULL) < 0) + if (check_and_add_cgroup_name("/") < 0) return -1; }
The --for-each-cgroup can have the same cgroup multiple times, but it makes bpf counters confused (since they have the same cgroup id), and the last cgroup events are counted only. Let's check the cgroup name before adding a new entry. Before: $ sudo ./perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1 Performance counter stats for 'system wide': <not counted> msec cpu-clock / <not counted> context-switches / <not counted> cpu-migrations / <not counted> page-faults / <not counted> cycles / <not counted> instructions / <not counted> branches / <not counted> branch-misses / 8,016.04 msec cpu-clock / # 7.998 CPUs utilized 6,152 context-switches / # 767.461 /sec 250 cpu-migrations / # 31.187 /sec 442 page-faults / # 55.139 /sec 613,111,487 cycles / # 0.076 GHz 280,599,604 instructions / # 0.46 insn per cycle 57,692,724 branches / # 7.197 M/sec 3,385,168 branch-misses / # 5.87% of all branches 1.002220125 seconds time elapsed After: $ sudo ./perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1 Performance counter stats for 'system wide': 8,013.38 msec cpu-clock / # 7.998 CPUs utilized 6,859 context-switches / # 855.944 /sec 334 cpu-migrations / # 41.680 /sec 345 page-faults / # 43.053 /sec 782,326,119 cycles / # 0.098 GHz 471,645,724 instructions / # 0.60 insn per cycle 94,963,430 branches / # 11.851 M/sec 3,685,511 branch-misses / # 3.88% of all branches 1.001864539 seconds time elapsed Fixes: bb1c15b60b981 ("perf stat: Support regex pattern in --for-each-cgroup") Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/cgroup.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)