Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1569268 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH REPOST 2/2] cgroup, perf_event: make perf_event controller  work on cgroup2 hierarchy Tejun Heo <tj@kernel.org> - 2017-01-29 20:40 +0100
    Re: [PATCH REPOST 2/2] cgroup, perf_event: make perf_event  controller work on cgroup2 hierarchy Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-30 15:10 +0100
      Re: [PATCH REPOST 2/2] cgroup, perf_event: make perf_event  controller work on cgroup2 hierarchy Tejun Heo <tj@kernel.org> - 2017-01-30 16:20 +0100
    Re: [PATCH REPOST 2/2] cgroup, perf_event: make perf_event  controller work on cgroup2 hierarchy Tejun Heo <tj@kernel.org> - 2017-02-02 19:50 +0100

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

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@kernel.org>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
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?

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;

[toc] | [next] | [standalone]

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

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@kernel.org>
> Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> ---
> 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;

[toc] | [prev] | [next] | [standalone]

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

Hello,

On Mon, Jan 30, 2017 at 11:05:41AM -0300, Arnaldo Carvalho de Melo wrote:
> > 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/.

Alright, thanks.  Will apply it to cgroup/for-4.11 in a couple days.
If there's any objection, please holler.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

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

On Sun, Jan 29, 2017 at 02:35:20PM -0500, Tejun Heo wrote:
> 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@kernel.org>
> Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

Applied to cgroup/for-4.11.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web