Re: [PATCH REPOST 2/2] cgroup, perf_event: make perf_event controller work on cgroup2 hierarchy

From: Arnaldo Carvalho de Melo
Date: Mon Jan 30 2017 - 09:08:18 EST


Em Sun, Jan 29, 2017 at 02:35:20PM -0500, Tejun Heo escreveu:
> perf_event is a utility controller whose primary role is identifying
> cgroup membership to filter perf events; however, because it also
> tracks some per-css state, it can't be replaced by pure cgroup
> membership test. Mark the controller as implicitly enabled on the
> default hierarchy so that perf events can always be filtered based on
> cgroup v2 path as long as the controller is not mounted on a legacy
> hierarchy.
>
> "perf record" is updated accordingly so that it searches for both v1
> and v2 hierarchies. A v1 hierarchy is used if perf_event is mounted
> on it; otherwise, it uses the v2 hierarchy.
>
> v2: Doc updated to reflect more flexible rebinding behavior.
>
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> Acked-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
> Hello,
>
> This was posted months ago and acked by Peter. I thought it was
> applied but apparently wasn't. Peter asked Arnaldo whether the
> userspace part looked which didn't get replied and that probably was
> how it slipped through the crack.
>
> Peter, Arnanldo, how should we proceed? Should I route this through
> the cgroup tree?

It looks ok, haven't tested it tho. I don't have a problem for it to go
thru the cgroup tree. The change is small and restrictred to cgroup
glue in tools/perf/.

- Arnaldo

> Thanks.
>
> Documentation/cgroup-v2.txt | 12 ++++++++++++
> kernel/events/core.c | 6 ++++++
> tools/perf/util/cgroup.c | 26 +++++++++++++++++++-------
> 3 files changed, 37 insertions(+), 7 deletions(-)
>
> --- a/Documentation/cgroup-v2.txt
> +++ b/Documentation/cgroup-v2.txt
> @@ -49,6 +49,8 @@ CONTENTS
> 5-3-2. Writeback
> 5-4. PID
> 5-4-1. PID Interface Files
> + 5-5. Misc
> + 5-5-1. perf_event
> 6. Namespace
> 6-1. Basics
> 6-2. The Root and Views
> @@ -1160,6 +1162,16 @@ through fork() or clone(). These will re
> of a new process would cause a cgroup policy to be violated.
>
>
> +5-5. Misc
> +
> +5-5-1. perf_event
> +
> +perf_event controller, if not mounted on a legacy hierarchy, is
> +automatically enabled on the v2 hierarchy so that perf events can
> +always be filtered by cgroup v2 path. The controller can still be
> +moved to a legacy hierarchy after v2 hierarchy is populated.
> +
> +
> 6. Namespace
>
> 6-1. Basics
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10792,5 +10792,11 @@ struct cgroup_subsys perf_event_cgrp_sub
> .css_alloc = perf_cgroup_css_alloc,
> .css_free = perf_cgroup_css_free,
> .attach = perf_cgroup_attach,
> + /*
> + * Implicitly enable on dfl hierarchy so that perf events can
> + * always be filtered by cgroup2 path as long as perf_event
> + * controller is not mounted on a legacy hierarchy.
> + */
> + .implicit_on_dfl = true,
> };
> #endif /* CONFIG_CGROUP_PERF */
> --- a/tools/perf/util/cgroup.c
> +++ b/tools/perf/util/cgroup.c
> @@ -12,8 +12,8 @@ cgroupfs_find_mountpoint(char *buf, size
> {
> FILE *fp;
> char mountpoint[PATH_MAX + 1], tokens[PATH_MAX + 1], type[PATH_MAX + 1];
> + char path_v1[PATH_MAX + 1], path_v2[PATH_MAX + 2], *path;
> char *token, *saved_ptr = NULL;
> - int found = 0;
>
> fp = fopen("/proc/mounts", "r");
> if (!fp)
> @@ -24,31 +24,43 @@ cgroupfs_find_mountpoint(char *buf, size
> * and inspect every cgroupfs mount point to find one that has
> * perf_event subsystem
> */
> + path_v1[0] = '\0';
> + path_v2[0] = '\0';
> +
> while (fscanf(fp, "%*s %"STR(PATH_MAX)"s %"STR(PATH_MAX)"s %"
> STR(PATH_MAX)"s %*d %*d\n",
> mountpoint, type, tokens) == 3) {
>
> - if (!strcmp(type, "cgroup")) {
> + if (!path_v1[0] && !strcmp(type, "cgroup")) {
>
> token = strtok_r(tokens, ",", &saved_ptr);
>
> while (token != NULL) {
> if (!strcmp(token, "perf_event")) {
> - found = 1;
> + strcpy(path_v1, mountpoint);
> break;
> }
> token = strtok_r(NULL, ",", &saved_ptr);
> }
> }
> - if (found)
> +
> + if (!path_v2[0] && !strcmp(type, "cgroup2"))
> + strcpy(path_v2, mountpoint);
> +
> + if (path_v1[0] && path_v2[0])
> break;
> }
> fclose(fp);
> - if (!found)
> +
> + if (path_v1[0])
> + path = path_v1;
> + else if (path_v2[0])
> + path = path_v2;
> + else
> return -1;
>
> - if (strlen(mountpoint) < maxlen) {
> - strcpy(buf, mountpoint);
> + if (strlen(path) < maxlen) {
> + strcpy(buf, path);
> return 0;
> }
> return -1;