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

Groups > linux.kernel > #1219993 > unrolled thread

[PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

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 5/5] perf tools: Enhance parsing events tracepoint error output Jiri Olsa <jolsa@kernel.org> - 2015-09-07 10:40 +0200
    Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Namhyung Kim <namhyung@kernel.org> - 2015-09-10 09:10 +0200
      Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Jiri Olsa <jolsa@redhat.com> - 2015-09-10 10:10 +0200
        Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Namhyung Kim <namhyung@kernel.org> - 2015-09-11 18:20 +0200
          Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Jiri Olsa <jolsa@redhat.com> - 2015-09-11 18:20 +0200
            Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-09-11 20:00 +0200
              Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 21:00 +0200
                Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-09-11 22:00 +0200
                  Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> - 2015-09-11 22:30 +0200
                    Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output Raphaël Beamonte <raphael.beamonte@gmail.com> - 2015-09-12 00:10 +0200
        Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error  output Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-14 23:00 +0200
    [tip:perf/core] perf tools:   Enhance parsing events tracepoint error output tip-bot for Jiri Olsa <tipbot@zytor.com> - 2015-09-16 09:40 +0200

#1219993 — [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

Enhancing parsing events tracepoint error output. Adding
more verbose output when the tracepoint is not found or
the tracing event path cannot be access.

  $ sudo perf record -e sched:sched_krava ls
  event syntax error: 'sched:sched_krava'
                       \___ unknown tracepoint

  Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
  Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.

  Run 'perf list' for a list of valid events
  ...

  $ perf record -e sched:sched_krava ls
  event syntax error: 'sched:sched_krava'
                       \___ can't access trace events

  Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
  Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

  Run 'perf list' for a list of valid events
  ...

Link: http://lkml.kernel.org/n/tip-l0lu26995rir1h6v0e7kqyzz@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.c | 35 ++++++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.y | 16 +++++++++-------
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c47831c47220..d3fb90be6216 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -387,6 +387,33 @@ int parse_events_add_cache(struct list_head *list, int *idx,
 	return add_event(list, idx, &attr, name, NULL);
 }
 
+static void tracepoint_error(struct parse_events_error *error, int err,
+			     char *sys, char *name)
+{
+	char help[BUFSIZ];
+
+	/*
+	 * We get error directly from syscall errno ( > 0),
+	 * or from encoded pointer's error ( < 0).
+	 */
+	err = abs(err);
+
+	switch (err) {
+	case EACCES:
+		error->str = strdup("can't access trace events");
+		break;
+	case ENOENT:
+		error->str = strdup("unknown tracepoint");
+		break;
+	default:
+		error->str = strdup("failed to add tracepoint");
+		break;
+	}
+
+	tracing_path__strerror_open_tp(err, help, sizeof(help), sys, name);
+	error->help = strdup(help);
+}
+
 static int add_tracepoint(struct list_head *list, int *idx,
 			  char *sys_name, char *evt_name,
 			  struct parse_events_error *error __maybe_unused)
@@ -394,8 +421,10 @@ static int add_tracepoint(struct list_head *list, int *idx,
 	struct perf_evsel *evsel;
 
 	evsel = perf_evsel__newtp_idx(sys_name, evt_name, (*idx)++);
-	if (IS_ERR(evsel))
+	if (IS_ERR(evsel)) {
+		tracepoint_error(error, PTR_ERR(evsel), sys_name, evt_name);
 		return PTR_ERR(evsel);
+	}
 
 	list_add_tail(&evsel->node, list);
 	return 0;
@@ -413,7 +442,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
 	snprintf(evt_path, MAXPATHLEN, "%s/%s", tracing_events_path, sys_name);
 	evt_dir = opendir(evt_path);
 	if (!evt_dir) {
-		perror("Can't open event dir");
+		tracepoint_error(error, errno, sys_name, evt_name);
 		return -1;
 	}
 
@@ -453,7 +482,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
 
 	events_dir = opendir(tracing_events_path);
 	if (!events_dir) {
-		perror("Can't open event dir");
+		tracepoint_error(error, errno, sys_name, evt_name);
 		return -1;
 	}
 
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 54a3004a8192..8bcc45868457 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -371,28 +371,30 @@ event_legacy_tracepoint:
 PE_NAME '-' PE_NAME ':' PE_NAME
 {
 	struct parse_events_evlist *data = _data;
+	struct parse_events_error *error = data->error;
 	struct list_head *list;
 	char sys_name[128];
 	snprintf(&sys_name, 128, "%s-%s", $1, $3);
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error));
+	if (parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, error)) {
+		if (error)
+			error->idx = @1.first_column;
+		return -1;
+	}
 	$$ = list;
 }
 |
 PE_NAME ':' PE_NAME
 {
 	struct parse_events_evlist *data = _data;
+	struct parse_events_error *error = data->error;
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) {
-		struct parse_events_error *error = data->error;
-
-		if (error) {
+	if (parse_events_add_tracepoint(list, &data->idx, $1, $3, error)) {
+		if (error)
 			error->idx = @1.first_column;
-			error->str = strdup("unknown tracepoint");
-		}
 		return -1;
 	}
 	$$ = list;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1221959 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

On Mon, Sep 07, 2015 at 10:38:07AM +0200, Jiri Olsa wrote:
> Enhancing parsing events tracepoint error output. Adding
> more verbose output when the tracepoint is not found or
> the tracing event path cannot be access.
> 
>   $ sudo perf record -e sched:sched_krava ls
>   event syntax error: 'sched:sched_krava'
>                        \___ unknown tracepoint
> 
>   Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
>   Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
> 
>   Run 'perf list' for a list of valid events
>   ...
> 
>   $ perf record -e sched:sched_krava ls
>   event syntax error: 'sched:sched_krava'
>                        \___ can't access trace events
> 
>   Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
>   Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

What about tracefs?  On my system, tracefs is mounted on
/sys/kernel/debug/tracing thus I cannot access trace events after
remounting debugfs with mode=755.

Also, IIRC tracepoint events adds PERF_SAMPLE_RAW bit automatically,
and it requires perf_event_paranoid being -1 for non-root user, right?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1221991 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

On Thu, Sep 10, 2015 at 04:00:30PM +0900, Namhyung Kim wrote:
> On Mon, Sep 07, 2015 at 10:38:07AM +0200, Jiri Olsa wrote:
> > Enhancing parsing events tracepoint error output. Adding
> > more verbose output when the tracepoint is not found or
> > the tracing event path cannot be access.
> > 
> >   $ sudo perf record -e sched:sched_krava ls
> >   event syntax error: 'sched:sched_krava'
> >                        \___ unknown tracepoint
> > 
> >   Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
> >   Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
> > 
> >   Run 'perf list' for a list of valid events
> >   ...
> > 
> >   $ perf record -e sched:sched_krava ls
> >   event syntax error: 'sched:sched_krava'
> >                        \___ can't access trace events
> > 
> >   Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
> >   Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
> 
> What about tracefs?  On my system, tracefs is mounted on
> /sys/kernel/debug/tracing thus I cannot access trace events after
> remounting debugfs with mode=755.

right, patch below keeps the actual mount and
display proper info.. could you please try?

> 
> Also, IIRC tracepoint events adds PERF_SAMPLE_RAW bit automatically,
> and it requires perf_event_paranoid being -1 for non-root user, right?

there's related error message when you try to open the
tracepoint, the whole session is like:


[jolsa@krava perf]$ ./perf record -e sched:sched_switch ls
event syntax error: 'sched:sched_switch'
                     \___ can't access trace events

Error:  No permissions to read /sys/kernel/debug/tracing/events/sched/sched_switch
Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'

Run 'perf list' for a list of valid events

 usage: perf record [<options>] [<command>]
    or: perf record [<options>] -- <command> [<options>]

    -e, --event <event>   event selector. use 'perf list' to list available events
[jolsa@krava perf]$ sudo mount -o remount,mode=755 /sys/kernel/debug/tracing
[jolsa@krava perf]$ ./perf record -e sched:sched_switch ls
Error:
You may not have permission to collect stats.
Consider tweaking /proc/sys/kernel/perf_event_paranoid:
 -1 - Not paranoid at all
  0 - Disallow raw tracepoint access for unpriv
  1 - Disallow cpu events for unpriv
  2 - Disallow kernel profiling for unpriv



thanks,
jirka


---
diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
index 38aca2dd1946..0406a7d5c891 100644
--- a/tools/lib/api/fs/tracing_path.c
+++ b/tools/lib/api/fs/tracing_path.c
@@ -12,12 +12,14 @@
 #include "tracing_path.h"
 
 
+char tracing_mnt[PATH_MAX + 1]         = "/sys/kernel/debug";
 char tracing_path[PATH_MAX + 1]        = "/sys/kernel/debug/tracing";
 char tracing_events_path[PATH_MAX + 1] = "/sys/kernel/debug/tracing/events";
 
 
 static void __tracing_path_set(const char *tracing, const char *mountpoint)
 {
+	snprintf(tracing_mnt, sizeof(tracing_mnt), "%s", mountpoint);
 	snprintf(tracing_path, sizeof(tracing_path), "%s/%s",
 		 mountpoint, tracing);
 	snprintf(tracing_events_path, sizeof(tracing_events_path), "%s/%s%s",
@@ -109,19 +111,10 @@ static int strerror_open(int err, char *buf, size_t size, const char *filename)
 			 "Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'");
 		break;
 	case EACCES: {
-		const char *mountpoint = debugfs__mountpoint();
-
-		if (!access(mountpoint, R_OK) && strncmp(filename, "tracing/", 8) == 0) {
-			const char *tracefs_mntpoint = tracefs__mountpoint();
-
-			if (tracefs_mntpoint)
-				mountpoint = tracefs__mountpoint();
-		}
-
 		snprintf(buf, size,
 			 "Error:\tNo permissions to read %s/%s\n"
 			 "Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n",
-			 tracing_events_path, filename, mountpoint);
+			 tracing_events_path, filename, tracing_mnt);
 	}
 		break;
 	default:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1222938 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

Hi Jiri,

On Thu, Sep 10, 2015 at 10:05:37AM +0200, Jiri Olsa wrote:
> On Thu, Sep 10, 2015 at 04:00:30PM +0900, Namhyung Kim wrote:
> > On Mon, Sep 07, 2015 at 10:38:07AM +0200, Jiri Olsa wrote:
> > > Enhancing parsing events tracepoint error output. Adding
> > > more verbose output when the tracepoint is not found or
> > > the tracing event path cannot be access.
> > > 
> > >   $ sudo perf record -e sched:sched_krava ls
> > >   event syntax error: 'sched:sched_krava'
> > >                        \___ unknown tracepoint
> > > 
> > >   Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
> > >   Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
> > > 
> > >   Run 'perf list' for a list of valid events
> > >   ...
> > > 
> > >   $ perf record -e sched:sched_krava ls
> > >   event syntax error: 'sched:sched_krava'
> > >                        \___ can't access trace events
> > > 
> > >   Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
> > >   Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
> > 
> > What about tracefs?  On my system, tracefs is mounted on
> > /sys/kernel/debug/tracing thus I cannot access trace events after
> > remounting debugfs with mode=755.
> 
> right, patch below keeps the actual mount and
> display proper info.. could you please try?

Sure, this patch displays the proper tracefs mountpoint.  But it also
has a problem - if tracefs is mounted under debugfs, the access mode
of debugfs also affects, so in this case I had to change it both for
debugfs and tracefs..


> 
> > 
> > Also, IIRC tracepoint events adds PERF_SAMPLE_RAW bit automatically,
> > and it requires perf_event_paranoid being -1 for non-root user, right?
> 
> there's related error message when you try to open the
> tracepoint, the whole session is like:

Ah, great. :)

Thanks,
Namhyung


> [jolsa@krava perf]$ ./perf record -e sched:sched_switch ls
> event syntax error: 'sched:sched_switch'
>                      \___ can't access trace events
> 
> Error:  No permissions to read /sys/kernel/debug/tracing/events/sched/sched_switch
> Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'
> 
> Run 'perf list' for a list of valid events
> 
>  usage: perf record [<options>] [<command>]
>     or: perf record [<options>] -- <command> [<options>]
> 
>     -e, --event <event>   event selector. use 'perf list' to list available events
> [jolsa@krava perf]$ sudo mount -o remount,mode=755 /sys/kernel/debug/tracing
> [jolsa@krava perf]$ ./perf record -e sched:sched_switch ls
> Error:
> You may not have permission to collect stats.
> Consider tweaking /proc/sys/kernel/perf_event_paranoid:
>  -1 - Not paranoid at all
>   0 - Disallow raw tracepoint access for unpriv
>   1 - Disallow cpu events for unpriv
>   2 - Disallow kernel profiling for unpriv
> 
> 
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
> index 38aca2dd1946..0406a7d5c891 100644
> --- a/tools/lib/api/fs/tracing_path.c
> +++ b/tools/lib/api/fs/tracing_path.c
> @@ -12,12 +12,14 @@
>  #include "tracing_path.h"
>  
>  
> +char tracing_mnt[PATH_MAX + 1]         = "/sys/kernel/debug";
>  char tracing_path[PATH_MAX + 1]        = "/sys/kernel/debug/tracing";
>  char tracing_events_path[PATH_MAX + 1] = "/sys/kernel/debug/tracing/events";
>  
>  
>  static void __tracing_path_set(const char *tracing, const char *mountpoint)
>  {
> +	snprintf(tracing_mnt, sizeof(tracing_mnt), "%s", mountpoint);
>  	snprintf(tracing_path, sizeof(tracing_path), "%s/%s",
>  		 mountpoint, tracing);
>  	snprintf(tracing_events_path, sizeof(tracing_events_path), "%s/%s%s",
> @@ -109,19 +111,10 @@ static int strerror_open(int err, char *buf, size_t size, const char *filename)
>  			 "Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'");
>  		break;
>  	case EACCES: {
> -		const char *mountpoint = debugfs__mountpoint();
> -
> -		if (!access(mountpoint, R_OK) && strncmp(filename, "tracing/", 8) == 0) {
> -			const char *tracefs_mntpoint = tracefs__mountpoint();
> -
> -			if (tracefs_mntpoint)
> -				mountpoint = tracefs__mountpoint();
> -		}
> -
>  		snprintf(buf, size,
>  			 "Error:\tNo permissions to read %s/%s\n"
>  			 "Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n",
> -			 tracing_events_path, filename, mountpoint);
> +			 tracing_events_path, filename, tracing_mnt);
>  	}
>  		break;
>  	default:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1222941 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

On Sat, Sep 12, 2015 at 01:09:31AM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Thu, Sep 10, 2015 at 10:05:37AM +0200, Jiri Olsa wrote:
> > On Thu, Sep 10, 2015 at 04:00:30PM +0900, Namhyung Kim wrote:
> > > On Mon, Sep 07, 2015 at 10:38:07AM +0200, Jiri Olsa wrote:
> > > > Enhancing parsing events tracepoint error output. Adding
> > > > more verbose output when the tracepoint is not found or
> > > > the tracing event path cannot be access.
> > > > 
> > > >   $ sudo perf record -e sched:sched_krava ls
> > > >   event syntax error: 'sched:sched_krava'
> > > >                        \___ unknown tracepoint
> > > > 
> > > >   Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
> > > >   Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
> > > > 
> > > >   Run 'perf list' for a list of valid events
> > > >   ...
> > > > 
> > > >   $ perf record -e sched:sched_krava ls
> > > >   event syntax error: 'sched:sched_krava'
> > > >                        \___ can't access trace events
> > > > 
> > > >   Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
> > > >   Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
> > > 
> > > What about tracefs?  On my system, tracefs is mounted on
> > > /sys/kernel/debug/tracing thus I cannot access trace events after
> > > remounting debugfs with mode=755.
> > 
> > right, patch below keeps the actual mount and
> > display proper info.. could you please try?
> 
> Sure, this patch displays the proper tracefs mountpoint.  But it also

thanks

> has a problem - if tracefs is mounted under debugfs, the access mode
> of debugfs also affects, so in this case I had to change it both for
> debugfs and tracefs..


hum, I wonder the error message needs to be that smart..

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1223006

2015-09-11 12:16 GMT-04:00 Jiri Olsa <jolsa@redhat.com>:
> On Sat, Sep 12, 2015 at 01:09:31AM +0900, Namhyung Kim wrote:
<SNIP>
>> has a problem - if tracefs is mounted under debugfs, the access mode
>> of debugfs also affects, so in this case I had to change it both for
>> debugfs and tracefs..
>
>
> hum, I wonder the error message needs to be that smart..
>
> jirka

Hmm... If tracefs is mounted under debugfs, wouldn't remounting
debugfs do the trick, as it was done before?
If so, why couldn't we just check the paths with a basic strcmp to
verify if tracefs starts by debugfs, and in that case offer to remount
debugfs, else offer to remount tracefs?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1223040 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

Em Fri, Sep 11, 2015 at 01:50:02PM -0400, Raphaël Beamonte escreveu:
> 2015-09-11 12:16 GMT-04:00 Jiri Olsa <jolsa@redhat.com>:
> > On Sat, Sep 12, 2015 at 01:09:31AM +0900, Namhyung Kim wrote:
> <SNIP>
> >> has a problem - if tracefs is mounted under debugfs, the access mode
> >> of debugfs also affects, so in this case I had to change it both for
> >> debugfs and tracefs..

> > hum, I wonder the error message needs to be that smart..
> 
> Hmm... If tracefs is mounted under debugfs, wouldn't remounting
> debugfs do the trick, as it was done before?

Not necessarily, we may be able to access /a/ but not /a/b/, so, before
we get to /a/b/ we need to solve access to /a/ to then realize that
/a/b/ also need permission change so that we can access it.

> If so, why couldn't we just check the paths with a basic strcmp to
> verify if tracefs starts by debugfs, and in that case offer to remount
> debugfs, else offer to remount tracefs?

say it is how it was before tracefs:

 /sys/kernel/debug/tracing/

If we can't access "/sys/kernel/debug/tracing/" because we can't access
"/sys/kernel/debug/" we need first to change (remount, chmod/grp/own,
whatever is best in each hypotetical use case) /sys/kernel/debug/ to
then do the same for /sys/kernel/debug/tracing/, no?

We could of course say something like "Something is wrong with tracefs
and/or debugfs, figure it out and try again", but we can do better,
right? 8-P

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1223073

2015-09-11 14:55 GMT-04:00 Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>:
> Em Fri, Sep 11, 2015 at 01:50:02PM -0400, Raphaël Beamonte escreveu:
>> 2015-09-11 12:16 GMT-04:00 Jiri Olsa <jolsa@redhat.com>:
>> > On Sat, Sep 12, 2015 at 01:09:31AM +0900, Namhyung Kim wrote:
>> <SNIP>
>> >> has a problem - if tracefs is mounted under debugfs, the access mode
>> >> of debugfs also affects, so in this case I had to change it both for
>> >> debugfs and tracefs..
>
>> > hum, I wonder the error message needs to be that smart..
>>
>> Hmm... If tracefs is mounted under debugfs, wouldn't remounting
>> debugfs do the trick, as it was done before?
>
> Not necessarily, we may be able to access /a/ but not /a/b/, so, before
> we get to /a/b/ we need to solve access to /a/ to then realize that
> /a/b/ also need permission change so that we can access it.

Well, I kind of had in mind that if we can access /a/, it's that /a/
is not the problem, so why would we remount it? Remounting /a/b/
should do the trick, and there's no reason we couldn't do it if we can
access /a/. Or perhaps I'm missing something?

>> If so, why couldn't we just check the paths with a basic strcmp to
>> verify if tracefs starts by debugfs, and in that case offer to remount
>> debugfs, else offer to remount tracefs?
>
> say it is how it was before tracefs:
>
>  /sys/kernel/debug/tracing/
>
> If we can't access "/sys/kernel/debug/tracing/" because we can't access
> "/sys/kernel/debug/" we need first to change (remount, chmod/grp/own,
> whatever is best in each hypotetical use case) /sys/kernel/debug/ to
> then do the same for /sys/kernel/debug/tracing/, no?

In that case, I'm following: if we can't access /sys/kernel/debug, we
have to remount it anyway, and there's no guarantee that
/sys/kernel/debug/tracing will have the right permissions. In that
case, ok, we perhaps need to remount both. Perhaps because: can
we be sure that if /sys/kernel/debug is inaccessible, it will be the
same for /sys/kernel/debug/tracing ?

> We could of course say something like "Something is wrong with tracefs
> and/or debugfs, figure it out and try again", but we can do better,
> right? 8-P
>
> - Arnaldo

I agree! We were talking in a previous conversation about making perf
the user-friendliest possible. But I don't know if providing two
different remount to the user at the same time is the best thing to do
(even in that second case).
Why not checking first access to debugfs: if it doesn't work, give the
remount line for debugfs then exit. The user will do it then run perf
again. If it works, check tracefs: if it doesn't work, give the
remount line for tracefs. That gives at most two fail-runs of perf
before using it without any problem. It also allows not to give the
user two different remount lines directly when we can't be sure the
second one is useful (perhaps tracefs will be accessible directly?)
Another possibility would be to have another perf command, sort of a
"perf remountfs", to run with sudo and that would make itself both
checks and remount accordingly the two fs.

Thoughts?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1223087 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

Em Fri, Sep 11, 2015 at 03:56:44PM -0400, Raphaël Beamonte escreveu:
> Another possibility would be to have another perf command, sort of a
> "perf remountfs", to run with sudo and that would make itself both
> checks and remount accordingly the two fs.

This part maybe more interesting, so I'll focus on it, yeah, having a:
'perf fixperms' command (have a better name? :) ) may make sense, would
need context tho, i.e. something like:

  perf fixperms trace

Or:

  perf fixperms top

But then, perhaps in that case:

  $ trace ls
  Error:	No permissions to read
  /sys/kernel/debug/tracing/events/raw_syscalls/sys_(enter|exit)
  Hint:	Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

We could just add some more text saying that please check as well that
the other parts of the path we're trying to access are available once
the suggestion is followed.

- Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1223128

2015-09-11 16:22 GMT-04:00 Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>:
> Em Fri, Sep 11, 2015 at 03:56:44PM -0400, Raphaël Beamonte escreveu:
>> Another possibility would be to have another perf command, sort of a
>> "perf remountfs", to run with sudo and that would make itself both
>> checks and remount accordingly the two fs.
>
> This part maybe more interesting, so I'll focus on it, yeah, having a:
> 'perf fixperms' command (have a better name? :) ) may make sense, would
> need context tho, i.e. something like:
>
>   perf fixperms trace
>
> Or:
>
>   perf fixperms top
>
> But then, perhaps in that case:
>
>   $ trace ls
>   Error:        No permissions to read
>   /sys/kernel/debug/tracing/events/raw_syscalls/sys_(enter|exit)
>   Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
>
> We could just add some more text saying that please check as well that
> the other parts of the path we're trying to access are available once
> the suggestion is followed.

Wouldn't it be almost the same as your "Something is wrong with
tracefs and/or debugfs, figure it out and try again" ? ;o)
I think that if the 'fixperms' need a context, we could as well give
the 'perf fixperms' command to type directly in the Hint, such as:

$ trace ls
Error:        No permissions to read
/sys/kernel/debug/tracing/events/raw_syscalls/sys_(enter|exit)
Hint: Try 'sudo perf fixperms trace'

It would be the same as the current copy/paste, but one command that
would fix the situation directly, instead of having to check the
permissions of each level in the path. A little bit user-friendlier!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1224448 — Re: [PATCH 5/5] perf tools: Enhance parsing events tracepoint error output

Em Thu, Sep 10, 2015 at 10:05:37AM +0200, Jiri Olsa escreveu:
> On Thu, Sep 10, 2015 at 04:00:30PM +0900, Namhyung Kim wrote:
> > On Mon, Sep 07, 2015 at 10:38:07AM +0200, Jiri Olsa wrote:
> > > Enhancing parsing events tracepoint error output. Adding
> > > more verbose output when the tracepoint is not found or
> > > the tracing event path cannot be access.
> > > 
> > >   $ sudo perf record -e sched:sched_krava ls
> > >   event syntax error: 'sched:sched_krava'
> > >                        \___ unknown tracepoint
> > > 
> > >   Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
> > >   Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
> > > 
> > >   Run 'perf list' for a list of valid events
> > >   ...
> > > 
> > >   $ perf record -e sched:sched_krava ls
> > >   event syntax error: 'sched:sched_krava'
> > >                        \___ can't access trace events
> > > 
> > >   Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
> > >   Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
> > 
> > What about tracefs?  On my system, tracefs is mounted on
> > /sys/kernel/debug/tracing thus I cannot access trace events after
> > remounting debugfs with mode=755.
> 
> right, patch below keeps the actual mount and
> display proper info.. could you please try?

I'll apply up to 5/5, will wait for this one to be resubmitted with
S-o-B, ok?

- Arnaldo
 
> > 
> > Also, IIRC tracepoint events adds PERF_SAMPLE_RAW bit automatically,
> > and it requires perf_event_paranoid being -1 for non-root user, right?
> 
> there's related error message when you try to open the
> tracepoint, the whole session is like:
> 
> 
> [jolsa@krava perf]$ ./perf record -e sched:sched_switch ls
> event syntax error: 'sched:sched_switch'
>                      \___ can't access trace events
> 
> Error:  No permissions to read /sys/kernel/debug/tracing/events/sched/sched_switch
> Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'
> 
> Run 'perf list' for a list of valid events
> 
>  usage: perf record [<options>] [<command>]
>     or: perf record [<options>] -- <command> [<options>]
> 
>     -e, --event <event>   event selector. use 'perf list' to list available events
> [jolsa@krava perf]$ sudo mount -o remount,mode=755 /sys/kernel/debug/tracing
> [jolsa@krava perf]$ ./perf record -e sched:sched_switch ls
> Error:
> You may not have permission to collect stats.
> Consider tweaking /proc/sys/kernel/perf_event_paranoid:
>  -1 - Not paranoid at all
>   0 - Disallow raw tracepoint access for unpriv
>   1 - Disallow cpu events for unpriv
>   2 - Disallow kernel profiling for unpriv
> 
> 
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
> index 38aca2dd1946..0406a7d5c891 100644
> --- a/tools/lib/api/fs/tracing_path.c
> +++ b/tools/lib/api/fs/tracing_path.c
> @@ -12,12 +12,14 @@
>  #include "tracing_path.h"
>  
>  
> +char tracing_mnt[PATH_MAX + 1]         = "/sys/kernel/debug";
>  char tracing_path[PATH_MAX + 1]        = "/sys/kernel/debug/tracing";
>  char tracing_events_path[PATH_MAX + 1] = "/sys/kernel/debug/tracing/events";
>  
>  
>  static void __tracing_path_set(const char *tracing, const char *mountpoint)
>  {
> +	snprintf(tracing_mnt, sizeof(tracing_mnt), "%s", mountpoint);
>  	snprintf(tracing_path, sizeof(tracing_path), "%s/%s",
>  		 mountpoint, tracing);
>  	snprintf(tracing_events_path, sizeof(tracing_events_path), "%s/%s%s",
> @@ -109,19 +111,10 @@ static int strerror_open(int err, char *buf, size_t size, const char *filename)
>  			 "Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'");
>  		break;
>  	case EACCES: {
> -		const char *mountpoint = debugfs__mountpoint();
> -
> -		if (!access(mountpoint, R_OK) && strncmp(filename, "tracing/", 8) == 0) {
> -			const char *tracefs_mntpoint = tracefs__mountpoint();
> -
> -			if (tracefs_mntpoint)
> -				mountpoint = tracefs__mountpoint();
> -		}
> -
>  		snprintf(buf, size,
>  			 "Error:\tNo permissions to read %s/%s\n"
>  			 "Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n",
> -			 tracing_events_path, filename, mountpoint);
> +			 tracing_events_path, filename, tracing_mnt);
>  	}
>  		break;
>  	default:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1225780 — [tip:perf/core] perf tools: Enhance parsing events tracepoint error output

Commit-ID:  196581717d85f59365dc9303685cd5b1cdf106a3
Gitweb:     http://git.kernel.org/tip/196581717d85f59365dc9303685cd5b1cdf106a3
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 7 Sep 2015 10:38:07 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 15 Sep 2015 09:48:33 -0300

perf tools: Enhance parsing events tracepoint error output

Enhancing parsing events tracepoint error output. Adding
more verbose output when the tracepoint is not found or
the tracing event path cannot be access.

  $ sudo perf record -e sched:sched_krava ls
  event syntax error: 'sched:sched_krava'
                       \___ unknown tracepoint

  Error:  File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found.
  Hint:   Perhaps this kernel misses some CONFIG_ setting to enable this feature?.

  Run 'perf list' for a list of valid events
  ...

  $ perf record -e sched:sched_krava ls
  event syntax error: 'sched:sched_krava'
                       \___ can't access trace events

  Error:  No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava
  Hint:   Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

  Run 'perf list' for a list of valid events
  ...

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Raphael Beamonte <raphael.beamonte@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1441615087-13886-6-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 35 ++++++++++++++++++++++++++++++++---
 tools/perf/util/parse-events.y | 16 +++++++++-------
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c47831c..d3fb90b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -387,6 +387,33 @@ int parse_events_add_cache(struct list_head *list, int *idx,
 	return add_event(list, idx, &attr, name, NULL);
 }
 
+static void tracepoint_error(struct parse_events_error *error, int err,
+			     char *sys, char *name)
+{
+	char help[BUFSIZ];
+
+	/*
+	 * We get error directly from syscall errno ( > 0),
+	 * or from encoded pointer's error ( < 0).
+	 */
+	err = abs(err);
+
+	switch (err) {
+	case EACCES:
+		error->str = strdup("can't access trace events");
+		break;
+	case ENOENT:
+		error->str = strdup("unknown tracepoint");
+		break;
+	default:
+		error->str = strdup("failed to add tracepoint");
+		break;
+	}
+
+	tracing_path__strerror_open_tp(err, help, sizeof(help), sys, name);
+	error->help = strdup(help);
+}
+
 static int add_tracepoint(struct list_head *list, int *idx,
 			  char *sys_name, char *evt_name,
 			  struct parse_events_error *error __maybe_unused)
@@ -394,8 +421,10 @@ static int add_tracepoint(struct list_head *list, int *idx,
 	struct perf_evsel *evsel;
 
 	evsel = perf_evsel__newtp_idx(sys_name, evt_name, (*idx)++);
-	if (IS_ERR(evsel))
+	if (IS_ERR(evsel)) {
+		tracepoint_error(error, PTR_ERR(evsel), sys_name, evt_name);
 		return PTR_ERR(evsel);
+	}
 
 	list_add_tail(&evsel->node, list);
 	return 0;
@@ -413,7 +442,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
 	snprintf(evt_path, MAXPATHLEN, "%s/%s", tracing_events_path, sys_name);
 	evt_dir = opendir(evt_path);
 	if (!evt_dir) {
-		perror("Can't open event dir");
+		tracepoint_error(error, errno, sys_name, evt_name);
 		return -1;
 	}
 
@@ -453,7 +482,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
 
 	events_dir = opendir(tracing_events_path);
 	if (!events_dir) {
-		perror("Can't open event dir");
+		tracepoint_error(error, errno, sys_name, evt_name);
 		return -1;
 	}
 
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 54a3004..8bcc458 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -371,28 +371,30 @@ event_legacy_tracepoint:
 PE_NAME '-' PE_NAME ':' PE_NAME
 {
 	struct parse_events_evlist *data = _data;
+	struct parse_events_error *error = data->error;
 	struct list_head *list;
 	char sys_name[128];
 	snprintf(&sys_name, 128, "%s-%s", $1, $3);
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error));
+	if (parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, error)) {
+		if (error)
+			error->idx = @1.first_column;
+		return -1;
+	}
 	$$ = list;
 }
 |
 PE_NAME ':' PE_NAME
 {
 	struct parse_events_evlist *data = _data;
+	struct parse_events_error *error = data->error;
 	struct list_head *list;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) {
-		struct parse_events_error *error = data->error;
-
-		if (error) {
+	if (parse_events_add_tracepoint(list, &data->idx, $1, $3, error)) {
+		if (error)
 			error->idx = @1.first_column;
-			error->str = strdup("unknown tracepoint");
-		}
 		return -1;
 	}
 	$$ = list;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web