* [PATCH] tools lib api fs: Store tracing mountpoint for better error message
@ 2015-09-19 14:47 Jiri Olsa
2015-09-19 14:50 ` David Ahern
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jiri Olsa @ 2015-09-19 14:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, David Ahern, Ingo Molnar, Namhyung Kim, Peter Zijlstra,
Matt Fleming, Raphaël Beamonte
Storing the actual tracing path mountpoint to display correct
error message hint ('Hint:' line). The error hint rediscovers
mountpoints, but it could be different from what we actually
used in tracing path.
Before we'd display debugfs mount even though tracefs was used:
$ 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/events/sched/sched_krava
Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
...
After this change, correct mountpoint is displayed:
$ 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/events/sched/sched_krava
Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'
...
Link: http://lkml.kernel.org/n/tip-xw7mf64ie0svh6m449vbyh0m@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/api/fs/tracing_path.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
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:
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tools lib api fs: Store tracing mountpoint for better error message
2015-09-19 14:47 [PATCH] tools lib api fs: Store tracing mountpoint for better error message Jiri Olsa
@ 2015-09-19 14:50 ` David Ahern
2015-09-19 15:00 ` Jiri Olsa
2015-09-19 15:26 ` Raphaël Beamonte
2015-09-29 8:38 ` [tip:perf/core] " tip-bot for Jiri Olsa
2 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2015-09-19 14:50 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Peter Zijlstra, Matt Fleming,
Raphaël Beamonte
On 9/19/15 8:47 AM, Jiri Olsa wrote:
> 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";
why all the +1s? null terminator has to fit within PATH_MAX as well.
>
>
> 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:
>
LGTM.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools lib api fs: Store tracing mountpoint for better error message
2015-09-19 14:50 ` David Ahern
@ 2015-09-19 15:00 ` Jiri Olsa
2015-09-19 15:23 ` Raphaël Beamonte
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2015-09-19 15:00 UTC (permalink / raw)
To: David Ahern
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar,
Namhyung Kim, Peter Zijlstra, Matt Fleming, Raphaël Beamonte
On Sat, Sep 19, 2015 at 08:50:19AM -0600, David Ahern wrote:
> On 9/19/15 8:47 AM, Jiri Olsa wrote:
>
> >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";
>
> why all the +1s? null terminator has to fit within PATH_MAX as well.
good question.. can't see a reason ATM
jirka
>
> >
> >
> > 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:
> >
>
> LGTM.
>
> David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools lib api fs: Store tracing mountpoint for better error message
2015-09-19 15:00 ` Jiri Olsa
@ 2015-09-19 15:23 ` Raphaël Beamonte
0 siblings, 0 replies; 6+ messages in thread
From: Raphaël Beamonte @ 2015-09-19 15:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: David Ahern, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Ingo Molnar, Namhyung Kim, Peter Zijlstra, Matt Fleming
2015-09-19 11:00 GMT-04:00 Jiri Olsa <jolsa@redhat.com>:
> On Sat, Sep 19, 2015 at 08:50:19AM -0600, David Ahern wrote:
>> On 9/19/15 8:47 AM, Jiri Olsa wrote:
>>
>> >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";
>>
>> why all the +1s? null terminator has to fit within PATH_MAX as well.
>
> good question.. can't see a reason ATM
It is quite weird actually, it seems there is multiple occurrences of
that, without any comment to explain why that +1, even if the #define
of PATH_MAX in limits.h specify that that size also counts nul.
Most of these occurrences are linked to perf though, but there's also
fs/ocfs2/aops.c, scripts/docproc.c and usr/gen_init_cpio.c.
Perhaps a mistake that started somewhere and that was propagated
looking at the original mistake?
> jirka
>
>>
>> >
>> >
>> > 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:
>> >
>>
>> LGTM.
>>
>> David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools lib api fs: Store tracing mountpoint for better error message
2015-09-19 14:47 [PATCH] tools lib api fs: Store tracing mountpoint for better error message Jiri Olsa
2015-09-19 14:50 ` David Ahern
@ 2015-09-19 15:26 ` Raphaël Beamonte
2015-09-29 8:38 ` [tip:perf/core] " tip-bot for Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Raphaël Beamonte @ 2015-09-19 15:26 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, David Ahern, Ingo Molnar,
Namhyung Kim, Peter Zijlstra, Matt Fleming
2015-09-19 10:47 GMT-04:00 Jiri Olsa <jolsa@kernel.org>:
> Storing the actual tracing path mountpoint to display correct
> error message hint ('Hint:' line). The error hint rediscovers
> mountpoints, but it could be different from what we actually
> used in tracing path.
>
> Before we'd display debugfs mount even though tracefs was used:
> $ 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/events/sched/sched_krava
> Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
> ...
>
> After this change, correct mountpoint is displayed:
> $ 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/events/sched/sched_krava
> Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'
> ...
>
> Link: http://lkml.kernel.org/n/tip-xw7mf64ie0svh6m449vbyh0m@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/lib/api/fs/tracing_path.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> 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:
> --
> 2.4.3
>
Reviewed-by: Raphaël Beamonte <raphael.beamonte@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/core] tools lib api fs: Store tracing mountpoint for better error message
2015-09-19 14:47 [PATCH] tools lib api fs: Store tracing mountpoint for better error message Jiri Olsa
2015-09-19 14:50 ` David Ahern
2015-09-19 15:26 ` Raphaël Beamonte
@ 2015-09-29 8:38 ` tip-bot for Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Jiri Olsa @ 2015-09-29 8:38 UTC (permalink / raw)
To: linux-tip-commits
Cc: dsahern, a.p.zijlstra, linux-kernel, tglx, jolsa, matt, acme, hpa,
raphael.beamonte, mingo, namhyung
Commit-ID: dc240c5dc2a02e335c5bcb50ad3a1274818c8609
Gitweb: http://git.kernel.org/tip/dc240c5dc2a02e335c5bcb50ad3a1274818c8609
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Sat, 19 Sep 2015 16:47:07 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 28 Sep 2015 15:50:54 -0300
tools lib api fs: Store tracing mountpoint for better error message
Storing the actual tracing path mountpoint to display correct
error message hint ('Hint:' line). The error hint rediscovers
mountpoints, but it could be different from what we actually
used in tracing path.
Before we'd display debugfs mount even though tracefs was used:
$ 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/events/sched/sched_krava
Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'
...
After this change, correct mountpoint is displayed:
$ 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/events/sched/sched_krava
Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'
...
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
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>
Cc: Raphael Beamonte <raphael.beamonte@gmail.com>
Link: http://lkml.kernel.org/r/1442674027-19427-1-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/lib/api/fs/tracing_path.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
index 38aca2d..0406a7d 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:
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-29 8:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-19 14:47 [PATCH] tools lib api fs: Store tracing mountpoint for better error message Jiri Olsa
2015-09-19 14:50 ` David Ahern
2015-09-19 15:00 ` Jiri Olsa
2015-09-19 15:23 ` Raphaël Beamonte
2015-09-19 15:26 ` Raphaël Beamonte
2015-09-29 8:38 ` [tip:perf/core] " tip-bot for Jiri Olsa
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.