perf tools: Fix tracing_path_mount proper path

PATCH In review Jiri Olsa 3 participants 4 messages Oct 16, 2018 – Oct 18, 2018

lore

Share & tools

lore

Jiri Olsa

author

2018-10-16 11:48 UTC #0 lore raw

PATCH 1/1 perf tools: Fix tracing_path_mount proper path

If there's no tracefs (RHEL7) support the tracing_path_mount returns debugfs path which results in following fail:

  # perf probe sys_write
  kprobe_events file does not exist - please rebuild kernel with CONFIG_KPROBE_EVENTS.
  Error: Failed to add events.

In tracing_path_debugfs_mount function we need to return the 'tracing' path instead of just the mount to make it work:

  # perf probe sys_write
  Added new event:
    probe:sys_write      (on sys_write)

You can now use it in all perf tools, such as:

perf record -e probe:sys_write -aR sleep 1

Adding the 'return tracing_path;' also to tracing_path_tracefs_mount function just for consistency with tracing_path_debugfs_mount.

Upstream keeps working, because it has the tracefs support.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/lib/api/fs/tracing_path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/lib/api/fs/tracing_path.c +2 -2

index 120037496f77..5afb11b30fca 100644
@@ -36,7 +36,7 @@ static const char *tracing_path_tracefs_mount(void)
 
 	__tracing_path_set("", mnt);
 
-	return mnt;
+	return tracing_path;
 }
 
 static const char *tracing_path_debugfs_mount(void)

@@ -49,7 +49,7 @@ static const char *tracing_path_debugfs_mount(void)
 
 	__tracing_path_set("tracing/", mnt);
 
-	return mnt;
+	return tracing_path;
 }
 
 const char *tracing_path_mount(void)

Signature · 1 line

2.17.2

Arnaldo Carvalho de Melo

2018-10-16 15:18 UTC #1 lore raw

Em Tue, Oct 16, 2018 at 01:48:18PM +0200, Jiri Olsa escreveu:

Quoted 43 lines

If there's no tracefs (RHEL7) support the tracing_path_mount returns debugfs path which results in following fail:
  # perf probe sys_write
  kprobe_events file does not exist - please rebuild kernel with CONFIG_KPROBE_EVENTS.
  Error: Failed to add events.
In tracing_path_debugfs_mount function we need to return the 'tracing' path instead of just the mount to make it work:
  # perf probe sys_write
  Added new event:
    probe:sys_write      (on sys_write)
You can now use it in all perf tools, such as:
perf record -e probe:sys_write -aR sleep 1
Adding the 'return tracing_path;' also to tracing_path_tracefs_mount function just for consistency with tracing_path_debugfs_mount.
Upstream keeps working, because it has the tracefs support.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/lib/api/fs/tracing_path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/lib/api/fs/tracing_path.c +1 -1
index 120037496f77..5afb11b30fca 100644
@@ -36,7 +36,7 @@ static const char *tracing_path_tracefs_mount(void)
 
 	__tracing_path_set("", mnt);
 
-	return mnt;
+	return tracing_path;
 }
 
 static const char *tracing_path_debugfs_mount(void)

tools/lib/api/fs/tracing_path.c static const char *tracing_path_debugfs_mount(void) · line 52

Quoted 5 lines

tools/lib/api/fs/tracing_path.c +1 -1

@@ -49,7 +49,7 @@ static const char *tracing_path_debugfs_mount(void)
 
 	__tracing_path_set("tracing/", mnt);
 
-	return mnt;
+	return tracing_path;

Humm, colour me confused, by the name of the function I expected tracing_path_debugfs_mount() to return the debugfs mount.

tracing_path_tracefs_mount() would return the tracefs mount point, so, there, returning tracing_path would be ok.

Is there some other way to figure out that tracefs isn't available and thus we should use debugfs_mount + "/tracing/" for the "tracefs" mount?

This may well be just the naming confusion... lemme read this a bit more...

- Arnaldo

Quoted 5 lines

tools/lib/api/fs/tracing_path.c +0 -0
 }
 
 const char *tracing_path_mount(void)
Signature · 1 line
2.17.2

Arnaldo Carvalho de Melo

2018-10-16 15:23 UTC #2 lore raw

Em Tue, Oct 16, 2018 at 12:18:13PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 53 lines

Em Tue, Oct 16, 2018 at 01:48:18PM +0200, Jiri Olsa escreveu:
Quoted 43 lines
If there's no tracefs (RHEL7) support the tracing_path_mount returns debugfs path which results in following fail:
  # perf probe sys_write
  kprobe_events file does not exist - please rebuild kernel with CONFIG_KPROBE_EVENTS.
  Error: Failed to add events.
In tracing_path_debugfs_mount function we need to return the 'tracing' path instead of just the mount to make it work:
  # perf probe sys_write
  Added new event:
    probe:sys_write      (on sys_write)
You can now use it in all perf tools, such as:
perf record -e probe:sys_write -aR sleep 1
Adding the 'return tracing_path;' also to tracing_path_tracefs_mount function just for consistency with tracing_path_debugfs_mount.
Upstream keeps working, because it has the tracefs support.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/lib/api/fs/tracing_path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/lib/api/fs/tracing_path.c +1 -1
index 120037496f77..5afb11b30fca 100644
@@ -36,7 +36,7 @@ static const char *tracing_path_tracefs_mount(void)
 
 	__tracing_path_set("", mnt);
 
-	return mnt;
+	return tracing_path;
 }
 
 static const char *tracing_path_debugfs_mount(void)
tools/lib/api/fs/tracing_path.c static const char *tracing_path_debugfs_mount(void) · line 52
Quoted 5 lines
tools/lib/api/fs/tracing_path.c +1 -1
@@ -49,7 +49,7 @@ static const char *tracing_path_debugfs_mount(void)
 
 	__tracing_path_set("tracing/", mnt);
 
-	return mnt;
+	return tracing_path;
Humm, colour me confused, by the name of the function I expected tracing_path_debugfs_mount() to return the debugfs mount.

Quoted 8 lines

tracing_path_tracefs_mount() would return the tracefs mount point, so, there, returning tracing_path would be ok.
Is there some other way to figure out that tracefs isn't available and thus we should use debugfs_mount + "/tracing/" for the "tracefs" mount?
This may well be just the naming confusion... lemme read this a bit more...

Indeed, naming confusion, these are static things, used only by:

const char *tracing_path_mount(void)

{
        const char *mnt;

        mnt = tracing_path_tracefs_mount();
        if (mnt)
                return mnt;

        mnt = tracing_path_debugfs_mount();

        return mnt;
}

This one could as well just return tracing_path and the return be used just to figure if it managed to read the mount point for tracefs or debugfs...

Something like:

static int tracing_path_tracefs_mount(void)

{
	const char *mnt = tracefs__mount();
	if (!mnt)
		return -1;

	__tracing_path_set("", mnt);
	return 0;
}

static const char *tracing_path_debugfs_mount(void)

{
	const char *mnt = debugfs__mount();
	if (!mnt)
		return -1;

	__tracing_path_set("tracing/", mnt);
	return 0;
}

const char *tracing_path_mount(void)

{
	if (tracing_path_tracefs_mount() < 0 &&
	    tracing_path_debugfs_mount() < 0)
		return NULL;

	return tracing_path;
}

But anyway, your patch fixes things, applying...

- Arnaldo

tip-bot for Jiri Olsa

merge bot

2018-10-18 06:19 UTC #3 lore raw

In reply to Jiri Olsa

perf tools: Fix tracing_path_mount proper path

Commit-ID:  c458a6206d2a8600934617ccf88ba7d3a030faba
Gitweb:     https://git.kernel.org/tip/c458a6206d2a8600934617ccf88ba7d3a030faba
Author:     Jiri Olsa <[email protected]>
AuthorDate: Tue, 16 Oct 2018 13:48:18 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 16 Oct 2018 12:27:46 -0300

perf tools: Fix tracing_path_mount proper path

If there's no tracefs (RHEL7) support the tracing_path_mount returns debugfs path which results in following fail:

  # perf probe sys_write
  kprobe_events file does not exist - please rebuild kernel with CONFIG_KPROBE_EVENTS.
  Error: Failed to add events.

In tracing_path_debugfs_mount function we need to return the 'tracing' path instead of just the mount to make it work:

  # perf probe sys_write
  Added new event:
    probe:sys_write      (on sys_write)

You can now use it in all perf tools, such as:

perf record -e probe:sys_write -aR sleep 1

Adding the 'return tracing_path;' also to tracing_path_tracefs_mount function just for consistency with tracing_path_debugfs_mount.

Upstream keeps working, because it has the tracefs support.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Michael Petlan <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Fixes 23773ca18b39 ("perf tools: Make perf aware of tracefs")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/lib/api/fs/tracing_path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/lib/api/fs/tracing_path.c +2 -2

index 120037496f77..5afb11b30fca 100644
@@ -36,7 +36,7 @@ static const char *tracing_path_tracefs_mount(void)
 
 	__tracing_path_set("", mnt);
 
-	return mnt;
+	return tracing_path;
 }
 
 static const char *tracing_path_debugfs_mount(void)

@@ -49,7 +49,7 @@ static const char *tracing_path_debugfs_mount(void)
 
 	__tracing_path_set("tracing/", mnt);
 
-	return mnt;
+	return tracing_path;
 }
 
 const char *tracing_path_mount(void)