All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint()
@ 2020-01-27 10:00 Namhyung Kim
  2020-01-27 11:12 ` Jiri Olsa
  2020-03-19 14:10 ` [tip: perf/core] tools lib api fs: Move cgroupsfs_find_mountpoint() tip-bot2 for Namhyung Kim
  0 siblings, 2 replies; 8+ messages in thread
From: Namhyung Kim @ 2020-01-27 10:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa; +Cc: LKML, linux-perf-users

Move it from tools/perf/util/cgroup.c as it can be used by other places.
Note that cgroup filesystem is different from others since it's usually
mounted separately (in v1) for each subsystem.

I just copied the code with a little modification to pass a name of
subsystem.

Suggested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/api/fs/Build    |  1 +
 tools/lib/api/fs/cgroup.c | 67 +++++++++++++++++++++++++++++++++++++++
 tools/lib/api/fs/fs.h     |  2 ++
 tools/perf/util/cgroup.c  | 63 ++----------------------------------
 4 files changed, 72 insertions(+), 61 deletions(-)
 create mode 100644 tools/lib/api/fs/cgroup.c

diff --git a/tools/lib/api/fs/Build b/tools/lib/api/fs/Build
index f4ed9629ae85..0f75b28654de 100644
--- a/tools/lib/api/fs/Build
+++ b/tools/lib/api/fs/Build
@@ -1,2 +1,3 @@
 libapi-y += fs.o
 libapi-y += tracing_path.o
+libapi-y += cgroup.o
diff --git a/tools/lib/api/fs/cgroup.c b/tools/lib/api/fs/cgroup.c
new file mode 100644
index 000000000000..889a6eb4aaca
--- /dev/null
+++ b/tools/lib/api/fs/cgroup.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/stringify.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include "fs.h"
+
+int cgroupfs_find_mountpoint(char *buf, size_t maxlen, const char *subsys)
+{
+	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;
+
+	fp = fopen("/proc/mounts", "r");
+	if (!fp)
+		return -1;
+
+	/*
+	 * in order to handle split hierarchy, we need to scan /proc/mounts
+	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
+				__stringify(PATH_MAX)"s %*d %*d\n",
+				mountpoint, type, tokens) == 3) {
+
+		if (!path_v1[0] && !strcmp(type, "cgroup")) {
+
+			token = strtok_r(tokens, ",", &saved_ptr);
+
+			while (token != NULL) {
+				if (subsys && !strcmp(token, subsys)) {
+					strcpy(path_v1, mountpoint);
+					break;
+				}
+				token = strtok_r(NULL, ",", &saved_ptr);
+			}
+		}
+
+		if (!path_v2[0] && !strcmp(type, "cgroup2"))
+			strcpy(path_v2, mountpoint);
+
+		if (path_v1[0] && path_v2[0])
+			break;
+	}
+	fclose(fp);
+
+	if (path_v1[0])
+		path = path_v1;
+	else if (path_v2[0])
+		path = path_v2;
+	else
+		return -1;
+
+	if (strlen(path) < maxlen) {
+		strcpy(buf, path);
+		return 0;
+	}
+	return -1;
+}
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index 92d03b8396b1..936edb95e1f3 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -28,6 +28,8 @@ FS(bpf_fs)
 #undef FS
 
 
+int cgroupfs_find_mountpoint(char *buf, size_t maxlen, const char *subsys);
+
 int filename__read_int(const char *filename, int *value);
 int filename__read_ull(const char *filename, unsigned long long *value);
 int filename__read_xll(const char *filename, unsigned long long *value);
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 4881d4af3381..5bc9d3b01bd9 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -3,75 +3,16 @@
 #include "evsel.h"
 #include "cgroup.h"
 #include "evlist.h"
-#include <linux/stringify.h>
 #include <linux/zalloc.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
+#include <api/fs/fs.h>
 
 int nr_cgroups;
 
-static int
-cgroupfs_find_mountpoint(char *buf, size_t maxlen)
-{
-	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;
-
-	fp = fopen("/proc/mounts", "r");
-	if (!fp)
-		return -1;
-
-	/*
-	 * in order to handle split hierarchy, we need to scan /proc/mounts
-	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
-				__stringify(PATH_MAX)"s %*d %*d\n",
-				mountpoint, type, tokens) == 3) {
-
-		if (!path_v1[0] && !strcmp(type, "cgroup")) {
-
-			token = strtok_r(tokens, ",", &saved_ptr);
-
-			while (token != NULL) {
-				if (!strcmp(token, "perf_event")) {
-					strcpy(path_v1, mountpoint);
-					break;
-				}
-				token = strtok_r(NULL, ",", &saved_ptr);
-			}
-		}
-
-		if (!path_v2[0] && !strcmp(type, "cgroup2"))
-			strcpy(path_v2, mountpoint);
-
-		if (path_v1[0] && path_v2[0])
-			break;
-	}
-	fclose(fp);
-
-	if (path_v1[0])
-		path = path_v1;
-	else if (path_v2[0])
-		path = path_v2;
-	else
-		return -1;
-
-	if (strlen(path) < maxlen) {
-		strcpy(buf, path);
-		return 0;
-	}
-	return -1;
-}
-
 static int open_cgroup(const char *name)
 {
 	char path[PATH_MAX + 1];
@@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
 	int fd;
 
 
-	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
+	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1, "perf_event"))
 		return -1;
 
 	scnprintf(path, PATH_MAX, "%s/%s", mnt, name);
-- 
2.25.0.341.g760bfbb309-goog


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

* Re: [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint()
  2020-01-27 10:00 [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint() Namhyung Kim
@ 2020-01-27 11:12 ` Jiri Olsa
  2020-01-27 14:58   ` Namhyung Kim
  2020-03-19 14:10 ` [tip: perf/core] tools lib api fs: Move cgroupsfs_find_mountpoint() tip-bot2 for Namhyung Kim
  1 sibling, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2020-01-27 11:12 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Arnaldo Carvalho de Melo, LKML, linux-perf-users

On Mon, Jan 27, 2020 at 07:00:31PM +0900, Namhyung Kim wrote:

SNIP

> -
> -	if (strlen(path) < maxlen) {
> -		strcpy(buf, path);
> -		return 0;
> -	}
> -	return -1;
> -}
> -
>  static int open_cgroup(const char *name)
>  {
>  	char path[PATH_MAX + 1];
> @@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
>  	int fd;
>  
>  
> -	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
> +	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1, "perf_event"))

nice, but could you please follow fs API names and change the
name to cgroupfs__mountpoint

I think we don't need to define the rest of the functions now,
if they are not used

	#define FS(name)                                \
		const char *name##__mountpoint(void);   \
		const char *name##__mount(void);        \
		bool name##__configured(void);          \

just follow the function name

thanks,
jirka

>  		return -1;
>  
>  	scnprintf(path, PATH_MAX, "%s/%s", mnt, name);
> -- 
> 2.25.0.341.g760bfbb309-goog
> 


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

* Re: [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint()
  2020-01-27 11:12 ` Jiri Olsa
@ 2020-01-27 14:58   ` Namhyung Kim
  2020-01-27 16:22     ` Jiri Olsa
  0 siblings, 1 reply; 8+ messages in thread
From: Namhyung Kim @ 2020-01-27 14:58 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, LKML, linux-perf-users

Hi Jiri,

On Mon, Jan 27, 2020 at 8:12 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Jan 27, 2020 at 07:00:31PM +0900, Namhyung Kim wrote:
>
> SNIP
>
> > -
> > -     if (strlen(path) < maxlen) {
> > -             strcpy(buf, path);
> > -             return 0;
> > -     }
> > -     return -1;
> > -}
> > -
> >  static int open_cgroup(const char *name)
> >  {
> >       char path[PATH_MAX + 1];
> > @@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
> >       int fd;
> >
> >
> > -     if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
> > +     if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1, "perf_event"))
>
> nice, but could you please follow fs API names and change the
> name to cgroupfs__mountpoint
>
> I think we don't need to define the rest of the functions now,
> if they are not used
>
>         #define FS(name)                                \
>                 const char *name##__mountpoint(void);   \
>                 const char *name##__mount(void);        \
>                 bool name##__configured(void);          \
>
> just follow the function name

I thought about it too, but still it has different arguments.
So I left it as is... Do you really want to rename?

Thanks
Namhyung

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

* Re: [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint()
  2020-01-27 14:58   ` Namhyung Kim
@ 2020-01-27 16:22     ` Jiri Olsa
  2020-01-28  0:49       ` [PATCH v2] tools lib api fs: Move cgroupsfs__mountpoint() Namhyung Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2020-01-27 16:22 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Arnaldo Carvalho de Melo, LKML, linux-perf-users

On Mon, Jan 27, 2020 at 11:58:27PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Mon, Jan 27, 2020 at 8:12 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Mon, Jan 27, 2020 at 07:00:31PM +0900, Namhyung Kim wrote:
> >
> > SNIP
> >
> > > -
> > > -     if (strlen(path) < maxlen) {
> > > -             strcpy(buf, path);
> > > -             return 0;
> > > -     }
> > > -     return -1;
> > > -}
> > > -
> > >  static int open_cgroup(const char *name)
> > >  {
> > >       char path[PATH_MAX + 1];
> > > @@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
> > >       int fd;
> > >
> > >
> > > -     if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
> > > +     if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1, "perf_event"))
> >
> > nice, but could you please follow fs API names and change the
> > name to cgroupfs__mountpoint
> >
> > I think we don't need to define the rest of the functions now,
> > if they are not used
> >
> >         #define FS(name)                                \
> >                 const char *name##__mountpoint(void);   \
> >                 const char *name##__mount(void);        \
> >                 bool name##__configured(void);          \
> >
> > just follow the function name
> 
> I thought about it too, but still it has different arguments.
> So I left it as is... Do you really want to rename?

yea I think we should keep the same name, Arnaldo?

jirka

> 
> Thanks
> Namhyung
> 


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

* [PATCH v2] tools lib api fs: Move cgroupsfs__mountpoint()
  2020-01-27 16:22     ` Jiri Olsa
@ 2020-01-28  0:49       ` Namhyung Kim
  2020-01-28  9:40         ` Jiri Olsa
  0 siblings, 1 reply; 8+ messages in thread
From: Namhyung Kim @ 2020-01-28  0:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa; +Cc: LKML, linux-perf-users

Move it from tools/perf/util/cgroup.c as it can be used by other places.
Note that cgroup filesystem is different from others since it's usually
mounted separately (in v1) for each subsystem.

I just copied the code with a little modification to pass a name of
subsystem and renamed it to follow other APIs.

Suggested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
* rename to cgroupfs__mountpoint()

 tools/lib/api/fs/Build    |  1 +
 tools/lib/api/fs/cgroup.c | 67 +++++++++++++++++++++++++++++++++++++++
 tools/lib/api/fs/fs.h     |  2 ++
 tools/perf/util/cgroup.c  | 63 ++----------------------------------
 4 files changed, 72 insertions(+), 61 deletions(-)
 create mode 100644 tools/lib/api/fs/cgroup.c

diff --git a/tools/lib/api/fs/Build b/tools/lib/api/fs/Build
index f4ed9629ae85..0f75b28654de 100644
--- a/tools/lib/api/fs/Build
+++ b/tools/lib/api/fs/Build
@@ -1,2 +1,3 @@
 libapi-y += fs.o
 libapi-y += tracing_path.o
+libapi-y += cgroup.o
diff --git a/tools/lib/api/fs/cgroup.c b/tools/lib/api/fs/cgroup.c
new file mode 100644
index 000000000000..c7e1cdaa36e1
--- /dev/null
+++ b/tools/lib/api/fs/cgroup.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/stringify.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include "fs.h"
+
+int cgroupfs__mountpoint(char *buf, size_t maxlen, const char *subsys)
+{
+	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;
+
+	fp = fopen("/proc/mounts", "r");
+	if (!fp)
+		return -1;
+
+	/*
+	 * in order to handle split hierarchy, we need to scan /proc/mounts
+	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
+				__stringify(PATH_MAX)"s %*d %*d\n",
+				mountpoint, type, tokens) == 3) {
+
+		if (!path_v1[0] && !strcmp(type, "cgroup")) {
+
+			token = strtok_r(tokens, ",", &saved_ptr);
+
+			while (token != NULL) {
+				if (subsys && !strcmp(token, subsys)) {
+					strcpy(path_v1, mountpoint);
+					break;
+				}
+				token = strtok_r(NULL, ",", &saved_ptr);
+			}
+		}
+
+		if (!path_v2[0] && !strcmp(type, "cgroup2"))
+			strcpy(path_v2, mountpoint);
+
+		if (path_v1[0] && path_v2[0])
+			break;
+	}
+	fclose(fp);
+
+	if (path_v1[0])
+		path = path_v1;
+	else if (path_v2[0])
+		path = path_v2;
+	else
+		return -1;
+
+	if (strlen(path) < maxlen) {
+		strcpy(buf, path);
+		return 0;
+	}
+	return -1;
+}
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index 92d03b8396b1..07591ecbe39f 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -28,6 +28,8 @@ FS(bpf_fs)
 #undef FS
 
 
+int cgroupfs__mountpoint(char *buf, size_t maxlen, const char *subsys);
+
 int filename__read_int(const char *filename, int *value);
 int filename__read_ull(const char *filename, unsigned long long *value);
 int filename__read_xll(const char *filename, unsigned long long *value);
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 4881d4af3381..12e466d1ec3b 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -3,75 +3,16 @@
 #include "evsel.h"
 #include "cgroup.h"
 #include "evlist.h"
-#include <linux/stringify.h>
 #include <linux/zalloc.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
+#include <api/fs/fs.h>
 
 int nr_cgroups;
 
-static int
-cgroupfs_find_mountpoint(char *buf, size_t maxlen)
-{
-	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;
-
-	fp = fopen("/proc/mounts", "r");
-	if (!fp)
-		return -1;
-
-	/*
-	 * in order to handle split hierarchy, we need to scan /proc/mounts
-	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
-				__stringify(PATH_MAX)"s %*d %*d\n",
-				mountpoint, type, tokens) == 3) {
-
-		if (!path_v1[0] && !strcmp(type, "cgroup")) {
-
-			token = strtok_r(tokens, ",", &saved_ptr);
-
-			while (token != NULL) {
-				if (!strcmp(token, "perf_event")) {
-					strcpy(path_v1, mountpoint);
-					break;
-				}
-				token = strtok_r(NULL, ",", &saved_ptr);
-			}
-		}
-
-		if (!path_v2[0] && !strcmp(type, "cgroup2"))
-			strcpy(path_v2, mountpoint);
-
-		if (path_v1[0] && path_v2[0])
-			break;
-	}
-	fclose(fp);
-
-	if (path_v1[0])
-		path = path_v1;
-	else if (path_v2[0])
-		path = path_v2;
-	else
-		return -1;
-
-	if (strlen(path) < maxlen) {
-		strcpy(buf, path);
-		return 0;
-	}
-	return -1;
-}
-
 static int open_cgroup(const char *name)
 {
 	char path[PATH_MAX + 1];
@@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
 	int fd;
 
 
-	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
+	if (cgroupfs__mountpoint(mnt, PATH_MAX + 1, "perf_event"))
 		return -1;
 
 	scnprintf(path, PATH_MAX, "%s/%s", mnt, name);
-- 
2.25.0.341.g760bfbb309-goog


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

* Re: [PATCH v2] tools lib api fs: Move cgroupsfs__mountpoint()
  2020-01-28  0:49       ` [PATCH v2] tools lib api fs: Move cgroupsfs__mountpoint() Namhyung Kim
@ 2020-01-28  9:40         ` Jiri Olsa
  2020-02-10 19:37           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2020-01-28  9:40 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Arnaldo Carvalho de Melo, LKML, linux-perf-users

On Tue, Jan 28, 2020 at 09:49:40AM +0900, Namhyung Kim wrote:
> Move it from tools/perf/util/cgroup.c as it can be used by other places.
> Note that cgroup filesystem is different from others since it's usually
> mounted separately (in v1) for each subsystem.
> 
> I just copied the code with a little modification to pass a name of
> subsystem and renamed it to follow other APIs.
> 
> Suggested-by: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

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

thanks,
jirka

> ---
> * rename to cgroupfs__mountpoint()
> 
>  tools/lib/api/fs/Build    |  1 +
>  tools/lib/api/fs/cgroup.c | 67 +++++++++++++++++++++++++++++++++++++++
>  tools/lib/api/fs/fs.h     |  2 ++
>  tools/perf/util/cgroup.c  | 63 ++----------------------------------
>  4 files changed, 72 insertions(+), 61 deletions(-)
>  create mode 100644 tools/lib/api/fs/cgroup.c
> 
> diff --git a/tools/lib/api/fs/Build b/tools/lib/api/fs/Build
> index f4ed9629ae85..0f75b28654de 100644
> --- a/tools/lib/api/fs/Build
> +++ b/tools/lib/api/fs/Build
> @@ -1,2 +1,3 @@
>  libapi-y += fs.o
>  libapi-y += tracing_path.o
> +libapi-y += cgroup.o
> diff --git a/tools/lib/api/fs/cgroup.c b/tools/lib/api/fs/cgroup.c
> new file mode 100644
> index 000000000000..c7e1cdaa36e1
> --- /dev/null
> +++ b/tools/lib/api/fs/cgroup.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/stringify.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include "fs.h"
> +
> +int cgroupfs__mountpoint(char *buf, size_t maxlen, const char *subsys)
> +{
> +	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;
> +
> +	fp = fopen("/proc/mounts", "r");
> +	if (!fp)
> +		return -1;
> +
> +	/*
> +	 * in order to handle split hierarchy, we need to scan /proc/mounts
> +	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
> +				__stringify(PATH_MAX)"s %*d %*d\n",
> +				mountpoint, type, tokens) == 3) {
> +
> +		if (!path_v1[0] && !strcmp(type, "cgroup")) {
> +
> +			token = strtok_r(tokens, ",", &saved_ptr);
> +
> +			while (token != NULL) {
> +				if (subsys && !strcmp(token, subsys)) {
> +					strcpy(path_v1, mountpoint);
> +					break;
> +				}
> +				token = strtok_r(NULL, ",", &saved_ptr);
> +			}
> +		}
> +
> +		if (!path_v2[0] && !strcmp(type, "cgroup2"))
> +			strcpy(path_v2, mountpoint);
> +
> +		if (path_v1[0] && path_v2[0])
> +			break;
> +	}
> +	fclose(fp);
> +
> +	if (path_v1[0])
> +		path = path_v1;
> +	else if (path_v2[0])
> +		path = path_v2;
> +	else
> +		return -1;
> +
> +	if (strlen(path) < maxlen) {
> +		strcpy(buf, path);
> +		return 0;
> +	}
> +	return -1;
> +}
> diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
> index 92d03b8396b1..07591ecbe39f 100644
> --- a/tools/lib/api/fs/fs.h
> +++ b/tools/lib/api/fs/fs.h
> @@ -28,6 +28,8 @@ FS(bpf_fs)
>  #undef FS
>  
>  
> +int cgroupfs__mountpoint(char *buf, size_t maxlen, const char *subsys);
> +
>  int filename__read_int(const char *filename, int *value);
>  int filename__read_ull(const char *filename, unsigned long long *value);
>  int filename__read_xll(const char *filename, unsigned long long *value);
> diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
> index 4881d4af3381..12e466d1ec3b 100644
> --- a/tools/perf/util/cgroup.c
> +++ b/tools/perf/util/cgroup.c
> @@ -3,75 +3,16 @@
>  #include "evsel.h"
>  #include "cgroup.h"
>  #include "evlist.h"
> -#include <linux/stringify.h>
>  #include <linux/zalloc.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <api/fs/fs.h>
>  
>  int nr_cgroups;
>  
> -static int
> -cgroupfs_find_mountpoint(char *buf, size_t maxlen)
> -{
> -	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;
> -
> -	fp = fopen("/proc/mounts", "r");
> -	if (!fp)
> -		return -1;
> -
> -	/*
> -	 * in order to handle split hierarchy, we need to scan /proc/mounts
> -	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
> -				__stringify(PATH_MAX)"s %*d %*d\n",
> -				mountpoint, type, tokens) == 3) {
> -
> -		if (!path_v1[0] && !strcmp(type, "cgroup")) {
> -
> -			token = strtok_r(tokens, ",", &saved_ptr);
> -
> -			while (token != NULL) {
> -				if (!strcmp(token, "perf_event")) {
> -					strcpy(path_v1, mountpoint);
> -					break;
> -				}
> -				token = strtok_r(NULL, ",", &saved_ptr);
> -			}
> -		}
> -
> -		if (!path_v2[0] && !strcmp(type, "cgroup2"))
> -			strcpy(path_v2, mountpoint);
> -
> -		if (path_v1[0] && path_v2[0])
> -			break;
> -	}
> -	fclose(fp);
> -
> -	if (path_v1[0])
> -		path = path_v1;
> -	else if (path_v2[0])
> -		path = path_v2;
> -	else
> -		return -1;
> -
> -	if (strlen(path) < maxlen) {
> -		strcpy(buf, path);
> -		return 0;
> -	}
> -	return -1;
> -}
> -
>  static int open_cgroup(const char *name)
>  {
>  	char path[PATH_MAX + 1];
> @@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
>  	int fd;
>  
>  
> -	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
> +	if (cgroupfs__mountpoint(mnt, PATH_MAX + 1, "perf_event"))
>  		return -1;
>  
>  	scnprintf(path, PATH_MAX, "%s/%s", mnt, name);
> -- 
> 2.25.0.341.g760bfbb309-goog
> 


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

* Re: [PATCH v2] tools lib api fs: Move cgroupsfs__mountpoint()
  2020-01-28  9:40         ` Jiri Olsa
@ 2020-02-10 19:37           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-10 19:37 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Namhyung Kim, LKML, linux-perf-users

Em Tue, Jan 28, 2020 at 10:40:57AM +0100, Jiri Olsa escreveu:
> On Tue, Jan 28, 2020 at 09:49:40AM +0900, Namhyung Kim wrote:
> > Move it from tools/perf/util/cgroup.c as it can be used by other places.
> > Note that cgroup filesystem is different from others since it's usually
> > mounted separately (in v1) for each subsystem.
> > 
> > I just copied the code with a little modification to pass a name of
> > subsystem and renamed it to follow other APIs.
> > 
> > Suggested-by: Jiri Olsa <jolsa@redhat.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Jiri Olsa <jolsa@redhat.com>

Thanks, applied to perf/core.

- Arnaldo
 
> thanks,
> jirka
> 
> > ---
> > * rename to cgroupfs__mountpoint()
> > 
> >  tools/lib/api/fs/Build    |  1 +
> >  tools/lib/api/fs/cgroup.c | 67 +++++++++++++++++++++++++++++++++++++++
> >  tools/lib/api/fs/fs.h     |  2 ++
> >  tools/perf/util/cgroup.c  | 63 ++----------------------------------
> >  4 files changed, 72 insertions(+), 61 deletions(-)
> >  create mode 100644 tools/lib/api/fs/cgroup.c
> > 
> > diff --git a/tools/lib/api/fs/Build b/tools/lib/api/fs/Build
> > index f4ed9629ae85..0f75b28654de 100644
> > --- a/tools/lib/api/fs/Build
> > +++ b/tools/lib/api/fs/Build
> > @@ -1,2 +1,3 @@
> >  libapi-y += fs.o
> >  libapi-y += tracing_path.o
> > +libapi-y += cgroup.o
> > diff --git a/tools/lib/api/fs/cgroup.c b/tools/lib/api/fs/cgroup.c
> > new file mode 100644
> > index 000000000000..c7e1cdaa36e1
> > --- /dev/null
> > +++ b/tools/lib/api/fs/cgroup.c
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/stringify.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include "fs.h"
> > +
> > +int cgroupfs__mountpoint(char *buf, size_t maxlen, const char *subsys)
> > +{
> > +	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;
> > +
> > +	fp = fopen("/proc/mounts", "r");
> > +	if (!fp)
> > +		return -1;
> > +
> > +	/*
> > +	 * in order to handle split hierarchy, we need to scan /proc/mounts
> > +	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
> > +				__stringify(PATH_MAX)"s %*d %*d\n",
> > +				mountpoint, type, tokens) == 3) {
> > +
> > +		if (!path_v1[0] && !strcmp(type, "cgroup")) {
> > +
> > +			token = strtok_r(tokens, ",", &saved_ptr);
> > +
> > +			while (token != NULL) {
> > +				if (subsys && !strcmp(token, subsys)) {
> > +					strcpy(path_v1, mountpoint);
> > +					break;
> > +				}
> > +				token = strtok_r(NULL, ",", &saved_ptr);
> > +			}
> > +		}
> > +
> > +		if (!path_v2[0] && !strcmp(type, "cgroup2"))
> > +			strcpy(path_v2, mountpoint);
> > +
> > +		if (path_v1[0] && path_v2[0])
> > +			break;
> > +	}
> > +	fclose(fp);
> > +
> > +	if (path_v1[0])
> > +		path = path_v1;
> > +	else if (path_v2[0])
> > +		path = path_v2;
> > +	else
> > +		return -1;
> > +
> > +	if (strlen(path) < maxlen) {
> > +		strcpy(buf, path);
> > +		return 0;
> > +	}
> > +	return -1;
> > +}
> > diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
> > index 92d03b8396b1..07591ecbe39f 100644
> > --- a/tools/lib/api/fs/fs.h
> > +++ b/tools/lib/api/fs/fs.h
> > @@ -28,6 +28,8 @@ FS(bpf_fs)
> >  #undef FS
> >  
> >  
> > +int cgroupfs__mountpoint(char *buf, size_t maxlen, const char *subsys);
> > +
> >  int filename__read_int(const char *filename, int *value);
> >  int filename__read_ull(const char *filename, unsigned long long *value);
> >  int filename__read_xll(const char *filename, unsigned long long *value);
> > diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
> > index 4881d4af3381..12e466d1ec3b 100644
> > --- a/tools/perf/util/cgroup.c
> > +++ b/tools/perf/util/cgroup.c
> > @@ -3,75 +3,16 @@
> >  #include "evsel.h"
> >  #include "cgroup.h"
> >  #include "evlist.h"
> > -#include <linux/stringify.h>
> >  #include <linux/zalloc.h>
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> >  #include <fcntl.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > +#include <api/fs/fs.h>
> >  
> >  int nr_cgroups;
> >  
> > -static int
> > -cgroupfs_find_mountpoint(char *buf, size_t maxlen)
> > -{
> > -	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;
> > -
> > -	fp = fopen("/proc/mounts", "r");
> > -	if (!fp)
> > -		return -1;
> > -
> > -	/*
> > -	 * in order to handle split hierarchy, we need to scan /proc/mounts
> > -	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
> > -				__stringify(PATH_MAX)"s %*d %*d\n",
> > -				mountpoint, type, tokens) == 3) {
> > -
> > -		if (!path_v1[0] && !strcmp(type, "cgroup")) {
> > -
> > -			token = strtok_r(tokens, ",", &saved_ptr);
> > -
> > -			while (token != NULL) {
> > -				if (!strcmp(token, "perf_event")) {
> > -					strcpy(path_v1, mountpoint);
> > -					break;
> > -				}
> > -				token = strtok_r(NULL, ",", &saved_ptr);
> > -			}
> > -		}
> > -
> > -		if (!path_v2[0] && !strcmp(type, "cgroup2"))
> > -			strcpy(path_v2, mountpoint);
> > -
> > -		if (path_v1[0] && path_v2[0])
> > -			break;
> > -	}
> > -	fclose(fp);
> > -
> > -	if (path_v1[0])
> > -		path = path_v1;
> > -	else if (path_v2[0])
> > -		path = path_v2;
> > -	else
> > -		return -1;
> > -
> > -	if (strlen(path) < maxlen) {
> > -		strcpy(buf, path);
> > -		return 0;
> > -	}
> > -	return -1;
> > -}
> > -
> >  static int open_cgroup(const char *name)
> >  {
> >  	char path[PATH_MAX + 1];
> > @@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
> >  	int fd;
> >  
> >  
> > -	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
> > +	if (cgroupfs__mountpoint(mnt, PATH_MAX + 1, "perf_event"))
> >  		return -1;
> >  
> >  	scnprintf(path, PATH_MAX, "%s/%s", mnt, name);
> > -- 
> > 2.25.0.341.g760bfbb309-goog
> > 
> 

-- 

- Arnaldo

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

* [tip: perf/core] tools lib api fs: Move cgroupsfs_find_mountpoint()
  2020-01-27 10:00 [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint() Namhyung Kim
  2020-01-27 11:12 ` Jiri Olsa
@ 2020-03-19 14:10 ` tip-bot2 for Namhyung Kim
  1 sibling, 0 replies; 8+ messages in thread
From: tip-bot2 for Namhyung Kim @ 2020-03-19 14:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Jiri Olsa, Namhyung Kim, Arnaldo Carvalho de Melo, x86, LKML

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     7982a898515064ba180d958bfc89ed22d13905ee
Gitweb:        https://git.kernel.org/tip/7982a898515064ba180d958bfc89ed22d13905ee
Author:        Namhyung Kim <namhyung@kernel.org>
AuthorDate:    Mon, 27 Jan 2020 19:00:31 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 04 Mar 2020 10:34:09 -03:00

tools lib api fs: Move cgroupsfs_find_mountpoint()

Move it from tools/perf/util/cgroup.c as it can be used by other places.
Note that cgroup filesystem is different from others since it's usually
mounted separately (in v1) for each subsystem.

I just copied the code with a little modification to pass a name of
subsystem.

Suggested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lore.kernel.org/lkml/20200127100031.1368732-1-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/Build    |  1 +-
 tools/lib/api/fs/cgroup.c | 67 ++++++++++++++++++++++++++++++++++++++-
 tools/lib/api/fs/fs.h     |  2 +-
 tools/perf/util/cgroup.c  | 63 +-----------------------------------
 4 files changed, 72 insertions(+), 61 deletions(-)
 create mode 100644 tools/lib/api/fs/cgroup.c

diff --git a/tools/lib/api/fs/Build b/tools/lib/api/fs/Build
index f4ed962..0f75b28 100644
--- a/tools/lib/api/fs/Build
+++ b/tools/lib/api/fs/Build
@@ -1,2 +1,3 @@
 libapi-y += fs.o
 libapi-y += tracing_path.o
+libapi-y += cgroup.o
diff --git a/tools/lib/api/fs/cgroup.c b/tools/lib/api/fs/cgroup.c
new file mode 100644
index 0000000..889a6eb
--- /dev/null
+++ b/tools/lib/api/fs/cgroup.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/stringify.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include "fs.h"
+
+int cgroupfs_find_mountpoint(char *buf, size_t maxlen, const char *subsys)
+{
+	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;
+
+	fp = fopen("/proc/mounts", "r");
+	if (!fp)
+		return -1;
+
+	/*
+	 * in order to handle split hierarchy, we need to scan /proc/mounts
+	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
+				__stringify(PATH_MAX)"s %*d %*d\n",
+				mountpoint, type, tokens) == 3) {
+
+		if (!path_v1[0] && !strcmp(type, "cgroup")) {
+
+			token = strtok_r(tokens, ",", &saved_ptr);
+
+			while (token != NULL) {
+				if (subsys && !strcmp(token, subsys)) {
+					strcpy(path_v1, mountpoint);
+					break;
+				}
+				token = strtok_r(NULL, ",", &saved_ptr);
+			}
+		}
+
+		if (!path_v2[0] && !strcmp(type, "cgroup2"))
+			strcpy(path_v2, mountpoint);
+
+		if (path_v1[0] && path_v2[0])
+			break;
+	}
+	fclose(fp);
+
+	if (path_v1[0])
+		path = path_v1;
+	else if (path_v2[0])
+		path = path_v2;
+	else
+		return -1;
+
+	if (strlen(path) < maxlen) {
+		strcpy(buf, path);
+		return 0;
+	}
+	return -1;
+}
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index 92d03b8..936edb9 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -28,6 +28,8 @@ FS(bpf_fs)
 #undef FS
 
 
+int cgroupfs_find_mountpoint(char *buf, size_t maxlen, const char *subsys);
+
 int filename__read_int(const char *filename, int *value);
 int filename__read_ull(const char *filename, unsigned long long *value);
 int filename__read_xll(const char *filename, unsigned long long *value);
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 4881d4a..5bc9d3b 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -3,75 +3,16 @@
 #include "evsel.h"
 #include "cgroup.h"
 #include "evlist.h"
-#include <linux/stringify.h>
 #include <linux/zalloc.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
+#include <api/fs/fs.h>
 
 int nr_cgroups;
 
-static int
-cgroupfs_find_mountpoint(char *buf, size_t maxlen)
-{
-	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;
-
-	fp = fopen("/proc/mounts", "r");
-	if (!fp)
-		return -1;
-
-	/*
-	 * in order to handle split hierarchy, we need to scan /proc/mounts
-	 * 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 %"__stringify(PATH_MAX)"s %"__stringify(PATH_MAX)"s %"
-				__stringify(PATH_MAX)"s %*d %*d\n",
-				mountpoint, type, tokens) == 3) {
-
-		if (!path_v1[0] && !strcmp(type, "cgroup")) {
-
-			token = strtok_r(tokens, ",", &saved_ptr);
-
-			while (token != NULL) {
-				if (!strcmp(token, "perf_event")) {
-					strcpy(path_v1, mountpoint);
-					break;
-				}
-				token = strtok_r(NULL, ",", &saved_ptr);
-			}
-		}
-
-		if (!path_v2[0] && !strcmp(type, "cgroup2"))
-			strcpy(path_v2, mountpoint);
-
-		if (path_v1[0] && path_v2[0])
-			break;
-	}
-	fclose(fp);
-
-	if (path_v1[0])
-		path = path_v1;
-	else if (path_v2[0])
-		path = path_v2;
-	else
-		return -1;
-
-	if (strlen(path) < maxlen) {
-		strcpy(buf, path);
-		return 0;
-	}
-	return -1;
-}
-
 static int open_cgroup(const char *name)
 {
 	char path[PATH_MAX + 1];
@@ -79,7 +20,7 @@ static int open_cgroup(const char *name)
 	int fd;
 
 
-	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1))
+	if (cgroupfs_find_mountpoint(mnt, PATH_MAX + 1, "perf_event"))
 		return -1;
 
 	scnprintf(path, PATH_MAX, "%s/%s", mnt, name);

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

end of thread, other threads:[~2020-03-19 14:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-27 10:00 [PATCH] tools lib api fs: Move cgroupsfs_find_mountpoint() Namhyung Kim
2020-01-27 11:12 ` Jiri Olsa
2020-01-27 14:58   ` Namhyung Kim
2020-01-27 16:22     ` Jiri Olsa
2020-01-28  0:49       ` [PATCH v2] tools lib api fs: Move cgroupsfs__mountpoint() Namhyung Kim
2020-01-28  9:40         ` Jiri Olsa
2020-02-10 19:37           ` Arnaldo Carvalho de Melo
2020-03-19 14:10 ` [tip: perf/core] tools lib api fs: Move cgroupsfs_find_mountpoint() tip-bot2 for Namhyung Kim

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.