[PATCH] perf tools: Fix tracing_path_mount proper path

[PATCH] perf tools: Fix tracing_path_mount proper path

[Jiri Olsa]

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/tip-yiwkzexq9fk1ey1xg3gnjlw4@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/api/fs/tracing_path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
index 120037496f77..5afb11b30fca 100644
--- a/tools/lib/api/fs/tracing_path.c
+++ b/tools/lib/api/fs/tracing_path.c
@@ -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)
-- 
2.17.2

Re: [PATCH] perf tools: Fix tracing_path_mount proper path

[Arnaldo Carvalho de Melo]

Em Tue, Oct 16, 2018 at 01:48:18PM +0200, Jiri Olsa escreveu:
> 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/tip-yiwkzexq9fk1ey1xg3gnjlw4@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/api/fs/tracing_path.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
> index 120037496f77..5afb11b30fca 100644
> --- a/tools/lib/api/fs/tracing_path.c
> +++ b/tools/lib/api/fs/tracing_path.c
> @@ -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;

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

>  }
>  
>  const char *tracing_path_mount(void)
> -- 
> 2.17.2

Re: [PATCH] perf tools: Fix tracing_path_mount proper path

[Arnaldo Carvalho de Melo]

Em Tue, Oct 16, 2018 at 12:18:13PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 16, 2018 at 01:48:18PM +0200, Jiri Olsa escreveu:
> > 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/tip-yiwkzexq9fk1ey1xg3gnjlw4@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/api/fs/tracing_path.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
> > index 120037496f77..5afb11b30fca 100644
> > --- a/tools/lib/api/fs/tracing_path.c
> > +++ b/tools/lib/api/fs/tracing_path.c
> > @@ -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;
> 
> 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...

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:perf/urgent] perf tools: Fix tracing_path_mount proper path

[tip-bot for Jiri Olsa]

Commit-ID:  c458a6206d2a8600934617ccf88ba7d3a030faba
Gitweb:     https://git.kernel.org/tip/c458a6206d2a8600934617ccf88ba7d3a030faba
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 16 Oct 2018 13:48:18 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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/tip-yiwkzexq9fk1ey1xg3gnjlw4@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Fixes: 23773ca18b39 ("perf tools: Make perf aware of tracefs")
Link: http://lkml.kernel.org/r/20181016114818.3595-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/tracing_path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
index 120037496f77..5afb11b30fca 100644
--- a/tools/lib/api/fs/tracing_path.c
+++ b/tools/lib/api/fs/tracing_path.c
@@ -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)