perf tools: Enhance parsing events tracepoint error output
Jiri Olsa
reviewercover perf tools: Enhance parsing events tracepoint error output
hi,
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
...
I changed api/fs tracefs/debugfs related code and I'm not completely sure what were the long term intentions with this code, so please comment ;-)
jirka
---
Jiri Olsa (11):
tools: Add err.h with ERR_PTR PTR_ERR interface
perf tools: Add tracing_path and remove unneeded functions perf tools: Do not change lib/api/fs/debugfs directly perf tools: Move debugfs__strerror_open into util.c object perf tools: Move tracing_path stuff under same namespace perf tools: Move tracing_path interface into trace-event-path.c perf tools: Make tracing_path_strerror_open message generic perf tools: Do not export debugfs_mountpoint and tracefs_mountpoint perf tools: Propagate error info for the tracepoint parsing perf tools: Propagate error info from tp_format
perf tools: Enhance parsing events tracepoint error output
tools/include/linux/err.h | 28 +++++++++++++++++++ tools/lib/api/fs/debugfs.c | 53 +----------------------------------- tools/lib/api/fs/debugfs.h | 5 ---- tools/lib/api/fs/tracefs.c | 2 +- tools/lib/api/fs/tracefs.h | 2 -- tools/perf/builtin-trace.c | 4 +-- tools/perf/perf.c | 11 ++++---- tools/perf/tests/openat-syscall-all-cpus.c | 2 ++ tools/perf/tests/openat-syscall.c | 2 ++ tools/perf/util/Build | 1 + tools/perf/util/evsel.c | 8 ++++-- tools/perf/util/parse-events.c | 58 ++++++++++++++++++++++++++++++---------- tools/perf/util/parse-events.h | 3 ++- tools/perf/util/parse-events.y | 16 ++++++----- tools/perf/util/trace-event-path.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/trace-event.c | 7 +++-- tools/perf/util/trace-event.h | 12 +++++++++ tools/perf/util/util.c | 119 --------------------------------------------------------------------------------- tools/perf/util/util.h | 8 ------ 19 files changed, 250 insertions(+), 220 deletions(-) create mode 100644 tools/include/linux/err.h create mode 100644 tools/perf/util/trace-event-path.c
Jiri Olsa
reviewerPATCH 1/11 tools: Add err.h with ERR_PTR PTR_ERR interface
Adding part of the kernel's <linux/err.h> interface:
inline void * __must_check ERR_PTR(long error);
inline long __must_check PTR_ERR(__force const void *ptr);
inline bool __must_check IS_ERR(__force const void *ptr);
it will be used to propagate error through pointers in following patches.
--- tools/include/linux/err.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tools/include/linux/err.h
tools/include/linux/err.h new file +28 -0
@@ -0,0 +1,28 @@ +#ifndef __TOOLS_LINUX_ERR_H +#define __TOOLS_LINUX_ERR_H + +#include <linux/compiler.h> +#include <linux/types.h> + +#include <asm/errno.h> + +#define MAX_ERRNO 4095 + +#define IS_ERR_VALUE (x) unlikely((x) >= (unsigned long)-MAX_ERRNO) + +static inline void *__must_check ERR_PTR (long error) +{ +return (void *) error; +} + +static inline long __must_check PTR_ERR (__force const void *ptr) +{ +return (long ) ptr; +} + +static inline bool __must_check IS_ERR (__force const void *ptr) +{ +return IS_ERR_VALUE ((unsigned long )ptr); +} + +#endif /* _LINUX_ERR_H */
Signature · 1 line
2.4.3
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 01/11] tools: Add err.h with ERR_PTR PTR_ERR interface
Em Wed, Aug 26, 2015 at 03:46:43PM +0200, Jiri Olsa escreveu:
Quoted 21 lines
Adding part of the kernel's <
linux/err.h> interface:
inline void *__must_checkERR_PTR(long error);
inline long__must_checkPTR_ERR(__force const void *ptr);
inline bool__must_checkIS_ERR(__force const void *ptr);it will be used to propagate error through pointers in following patches.
Signed-off-by Jiri Olsa <[email protected]>--- tools/include/linux/err.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tools/include/linux/err.htools/include/linux/err.h new file +0
-0
tools/include/linux/err.h line 8
Quoted 8 lines
tools/include/linux/err.h new file +8
-0@@ -0,0 +1,28 @@ +#ifndef __TOOLS_LINUX_ERR_H +#define __TOOLS_LINUX_ERR_H + +#include <linux/compiler.h> +#include <linux/types.h> + +#include <asm/errno.h> +
You deleted the comment in the kernel sources at this point:
/*
* Kernel pointers have redundant information, so we can use a
* scheme where we can return either an error code or a normal
* pointer with the same return value.
*
* This should be a per-architecture thing, to allow different
* error and pointer decisions.
*/
tools/include/linux/err.h line 9
Quoted 1 line
tools/include/linux/err.h new file +1
-0+#define MAX_ERRNO 4095
Now we're dealing with user pointers, are we completely sure we can use this trick here?
- Arnaldo
Quoted 20 lines
tools/include/linux/err.h new file +18
-0+#define IS_ERR_VALUE (x) unlikely((x) >= (unsigned long)-MAX_ERRNO) + +static inline void *__must_check ERR_PTR (long error) +{ +return (void *) error; +} + +static inline long __must_check PTR_ERR (__force const void *ptr) +{ +return (long ) ptr; +} + +static inline bool __must_check IS_ERR (__force const void *ptr) +{ +return IS_ERR_VALUE ((unsigned long )ptr); +} + +#endif /* _LINUX_ERR_H */ Signature · 1 line
2.4.3
Re: [PATCH 01/11] tools: Add err.h with ERR_PTR PTR_ERR interface
Quoted 16 lines · Arnaldo Carvalho de Melo
Em Wed, Aug 26, 2015 at 03:46:43PM +0200, Jiri Olsa escreveu:
Quoted 15 lines
Adding part of the kernel's <
linux/err.h> interface:
inline void *__must_checkERR_PTR(long error);
inline long__must_checkPTR_ERR(__force const void *ptr);
inline bool__must_checkIS_ERR(__force const void *ptr);it will be used to propagate error through pointers in following patches.
Signed-off-by Jiri Olsa <[email protected]>--- tools/include/linux/err.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tools/include/linux/err.h
Quoted 16 lines
Quoted 6 lines
tools/include/linux/err.h new file +0
-0tools/include/linux/err.h line 8
Quoted 8 lines
tools/include/linux/err.h new file +8
-0@@ -0,0 +1,28 @@ +#ifndef __TOOLS_LINUX_ERR_H +#define __TOOLS_LINUX_ERR_H + +#include <linux/compiler.h> +#include <linux/types.h> + +#include <asm/errno.h> +You deleted the comment in the kernel sources at this point:
right.. I did not want to bring too much attention :-))
Quoted 10 lines
/*
* Kernel pointers have redundant information, so we can use a
* scheme where we can return either an error code or a normal
* pointer with the same return value.
*
* This should be a per-architecture thing, to allow different
* error and pointer decisions.
*/
Quoted 4 lines
tools/include/linux/err.h line 9
Quoted 1 line
tools/include/linux/err.h new file +1
-0+#define MAX_ERRNO 4095Now we're dealing with user pointers, are we completely sure we can use this trick here?
it's safe for user as well, because 'error' pointers fall down to the unused hole:
Documentation/x86/x86_64/mm.txt:ffffffffffe00000 - ffffffffffffffff (=2 MB) unused hole
haven't checked for other archs, but since it's used within generic code, it should be ok
I'll put the comment back with additional explanation wrt user space in v2
jirka
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 01/11] tools: Add err.h with ERR_PTR PTR_ERR interface
Em Mon, Aug 31, 2015 at 09:37:18AM +0200, Jiri Olsa escreveu:
Quoted 14 lines · Arnaldo Carvalho de Melo
Quoted 12 lines
Em Wed, Aug 26, 2015 at 03:46:43PM +0200, Jiri Olsa escreveu:
tools/include/linux/err.h line 7
Quoted 9 lines
+++
b/tools/include/linux/err.htools/include/linux/err.h new file +7
-0@@ -0,0 +1,28 @@ +#ifndef __TOOLS_LINUX_ERR_H +#define __TOOLS_LINUX_ERR_H + +#include <linux/compiler.h> +#include <linux/types.h> + +#include <asm/errno.h> You deleted the comment in the kernel sources at this point:
Quoted 1 line
right.. I did not want to bring too much attention :-))
:-)
Please get the explanation about why it is safe (the unused hole bits) and merge it with the bits from the kernel comment that make sense, well, I think we can just do a s/Kernel//g and explain why that is so.
Quoted 17 lines
Quoted 9 lines
/*
* Kernel pointers have redundant information, so we can use a
* scheme where we can return either an error code or a normal
* pointer with the same return value.
*
* This should be a per-architecture thing, to allow different
* error and pointer decisions.
*/Quoted 4 lines
tools/include/linux/err.h line 9
Quoted 1 line
tools/include/linux/err.h new file +1
-0+#define MAX_ERRNO 4095Now we're dealing with user pointers, are we completely sure we can use this trick here?
it's safe for user as well, because 'error' pointers fall down to the unused hole:
Quoted 8 lines
Documentation/x86/x86_64/mm.txt:ffffffffffe00000- ffffffffffffffff (=2 MB) unused holehaven't checked for other archs, but since it's used within generic code, it should be ok
I'll put the comment back with additional explanation wrt user space in v2
Thanks!
- Arnaldo
Jiri Olsa
reviewerPATCH 2/11 perf tools: Add tracing_path and remove unneeded functions
In reply to Jiri OlsaThere's no need for find_tracing_dir, because perf already searches for debugfs/tracefs mount on start and populate tracing_events_path.
Adding tracing_path to carry tracing dir string to be used in get_tracing_file instead of calling find_tracing_dir.
--- tools/perf/util/util.c | 56 ++++---------------------------------------------- tools/perf/util/util.h | 2 +- 2 files changed, 5 insertions(+), 53 deletions(-)
tools/perf/util/util.c +4 -52
@@ -34,6 +34,7 @@ bool test_attr__enabled;bool perf_host = true;bool perf_guest = false; +char tracing_path[PATH_MAX +1 ] ="/sys/kernel/debug/tracing" ;char tracing_events_path[PATH_MAX +1 ] ="/sys/kernel/debug/tracing/events" ;void event_attr_init (struct perf_event_attr *attr)
@@ -391,6 +392,8 @@ void set_term_quiet_input(struct termios *old)static void set_tracing_events_path (const char *tracing,const char *mountpoint) { +snprintf (tracing_path,sizeof (tracing_path),"%s/%s" , + mountpoint, tracing);snprintf (tracing_events_path,sizeof (tracing_events_path),"%s/%s%s" , mountpoint, tracing,"events" ); }
@@ -440,62 +443,11 @@ void perf_debugfs_set_path(const char *mntpt)set_tracing_events_path ("tracing/" , mntpt); } -static const char *find_tracefs (void ) -{ -const char *path =__perf_tracefs_mount (NULL ); - -return path; -} - -static const char *find_debugfs (void ) -{ -const char *path =__perf_debugfs_mount (NULL ); - -if (!path) -fprintf (stderr,"Your kernel does not support the debugfs filesystem" ); - -return path; -} - -/* -* Finds the path to the debugfs/tracing -* Allocates the string and stores it. -*/ -const char *find_tracing_dir (void ) -{ -const char *tracing_dir ="" ; -static char *tracing; -static int tracing_found; -const char *debugfs; - -if (tracing_found) -return tracing; - - debugfs =find_tracefs (); -if (!debugfs) { - tracing_dir ="/tracing" ; - debugfs =find_debugfs (); -if (!debugfs) -return NULL ; - } - -if (asprintf (&tracing,"%s%s" , debugfs, tracing_dir) <0 ) -return NULL ; - - tracing_found =1 ; -return tracing; -} -char *get_tracing_file (const char *name) { -const char *tracing;char *file; - tracing =find_tracing_dir (); -if (!tracing) -return NULL ; - -if (asprintf (&file,"%s/%s" , tracing, name) <0 ) +if (asprintf (&file,"%s/%s" , tracing_path, name) <0 )return NULL ;return file;
tools/perf/util/util.h +1 -1
@@ -83,10 +83,10 @@extern const char *graph_line;extern const char *graph_dotted_line;extern char buildid_dir[]; +extern char tracing_path[];extern char tracing_events_path[];extern void perf_debugfs_set_path (const char *mountpoint);const char *perf_debugfs_mount (const char *mountpoint); -const char *find_tracing_dir (void );char *get_tracing_file (const char *name);void put_tracing_file (char *file);
Signature · 1 line
2.4.3
Re: [PATCH 02/11] perf tools: Add tracing_path and remove unneeded functions
Quoted 3 lines · Jiri Olsa
There's no need for
find_tracing_dir, because perf already searches for debugfs/tracefs mount on start and populatetracing_events_path.
I'm getting a bit lost searching through the git history of these functions. Why is it safe to delete these functions? Where did the old user that required find_tracing_dir() to mount debugfs disappear to?
The code to do the mount of debugfs/tracefs when perf starts appears to have been around for years. Is the mounting operation of find_tracing_dir() just dead code?
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 02/11] perf tools: Add tracing_path and remove unneeded functions
Em Thu, Aug 27, 2015 at 11:47:19PM +0100, Matt Fleming escreveu:
Quoted 5 lines · Jiri Olsa
Quoted 3 lines
There's no need for
find_tracing_dir, because perf already searches for debugfs/tracefs mount on start and populatetracing_events_path.
Quoted 7 lines
I'm getting a bit lost searching through the git history of these functions. Why is it safe to delete these functions? Where did the old user that required
find_tracing_dir()to mount debugfs disappear to?The code to do the mount of debugfs/tracefs when perf starts appears to have been around for years. Is the mounting operation of
find_tracing_dir()just dead code?
It looks like, these things should come after we realize that tracepoints were requested in record, so we already looked where things are mounted, etc, without digging deeper I think Jiri's patch is ok.
- Arnaldo
Re: [PATCH 02/11] perf tools: Add tracing_path and remove unneeded functions
Quoted 13 lines · Arnaldo Carvalho de Melo
Em Thu, Aug 27, 2015 at 11:47:19PM +0100, Matt Fleming escreveu:
Quoted 12 lines · Jiri Olsa
Quoted 3 lines
There's no need for
find_tracing_dir, because perf already searches for debugfs/tracefs mount on start and populatetracing_events_path.I'm getting a bit lost searching through the git history of these functions. Why is it safe to delete these functions? Where did the old user that required
find_tracing_dir()to mount debugfs disappear to?The code to do the mount of debugfs/tracefs when perf starts appears to have been around for years. Is the mounting operation of
find_tracing_dir()just dead code?
yep, I'll address that in v2.. found TODO in fs.c ;-)
jirka
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tools: Add tracing_path and remove unneeded functions
In reply to Jiri Olsaperf tools: Add tracing_path and remove unneeded functions
Commit-ID: 9f44f0cc1c32f1542071447a9493652bbc03facb Gitweb: http://git.kernel.org/tip/9f44f0cc1c32f1542071447a9493652bbc03facb Author: Jiri Olsa <[email protected]> AuthorDate: Wed, 26 Aug 2015 15:46:44 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 28 Aug 2015 14:53:51 -0300
perf tools: Add tracing_path and remove unneeded functions
There's no need for find_tracing_dir, because perf already searches for debugfs/tracefs mount on start and populate tracing_events_path.
Adding tracing_path to carry tracing dir string to be used in get_tracing_file instead of calling find_tracing_dir.
--- tools/perf/util/util.c | 56 ++++---------------------------------------------- tools/perf/util/util.h | 2 +- 2 files changed, 5 insertions(+), 53 deletions(-)
tools/perf/util/util.c +4 -52
@@ -34,6 +34,7 @@ bool test_attr__enabled;bool perf_host = true;bool perf_guest = false; +char tracing_path[PATH_MAX +1 ] ="/sys/kernel/debug/tracing" ;char tracing_events_path[PATH_MAX +1 ] ="/sys/kernel/debug/tracing/events" ;void event_attr_init (struct perf_event_attr *attr)
@@ -391,6 +392,8 @@ void set_term_quiet_input(struct termios *old)static void set_tracing_events_path (const char *tracing,const char *mountpoint) { +snprintf (tracing_path,sizeof (tracing_path),"%s/%s" , + mountpoint, tracing);snprintf (tracing_events_path,sizeof (tracing_events_path),"%s/%s%s" , mountpoint, tracing,"events" ); }
@@ -440,62 +443,11 @@ void perf_debugfs_set_path(const char *mntpt)set_tracing_events_path ("tracing/" , mntpt); } -static const char *find_tracefs (void ) -{ -const char *path =__perf_tracefs_mount (NULL ); - -return path; -} - -static const char *find_debugfs (void ) -{ -const char *path =__perf_debugfs_mount (NULL ); - -if (!path) -fprintf (stderr,"Your kernel does not support the debugfs filesystem" ); - -return path; -} - -/* -* Finds the path to the debugfs/tracing -* Allocates the string and stores it. -*/ -const char *find_tracing_dir (void ) -{ -const char *tracing_dir ="" ; -static char *tracing; -static int tracing_found; -const char *debugfs; - -if (tracing_found) -return tracing; - - debugfs =find_tracefs (); -if (!debugfs) { - tracing_dir ="/tracing" ; - debugfs =find_debugfs (); -if (!debugfs) -return NULL ; - } - -if (asprintf (&tracing,"%s%s" , debugfs, tracing_dir) <0 ) -return NULL ; - - tracing_found =1 ; -return tracing; -} -char *get_tracing_file (const char *name) { -const char *tracing;char *file; - tracing =find_tracing_dir (); -if (!tracing) -return NULL ; - -if (asprintf (&file,"%s/%s" , tracing, name) <0 ) +if (asprintf (&file,"%s/%s" , tracing_path, name) <0 )return NULL ;return file;
tools/perf/util/util.h +1 -1
@@ -83,10 +83,10 @@extern const char *graph_line;extern const char *graph_dotted_line;extern char buildid_dir[]; +extern char tracing_path[];extern char tracing_events_path[];extern void perf_debugfs_set_path (const char *mountpoint);const char *perf_debugfs_mount (const char *mountpoint); -const char *find_tracing_dir (void );char *get_tracing_file (const char *name);void put_tracing_file (char *file);
Jiri Olsa
reviewerPATCH 3/11 perf tools: Do not change lib/api/fs/debugfs directly
In reply to Jiri OlsaThe tracing_events_path is the variable we want to change via --debugfs-dir option, not the debugfs_mountpoint.
--- tools/perf/perf.c | 2 +- tools/perf/util/util.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
tools/perf/perf.c +1 -1
@@ -231,7 +231,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argc)--; }else if (!prefixcmp (cmd,CMD_DEBUGFS_DIR )) {perf_debugfs_set_path (cmd +strlen (CMD_DEBUGFS_DIR )); -fprintf (stderr,"dir: %s\n" , debugfs_mountpoint); +fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--list-cmds" )) {
tools/perf/util/util.c +0 -1
@@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)void perf_debugfs_set_path (const char *mntpt) { -snprintf (debugfs_mountpoint,strlen (debugfs_mountpoint),"%s" , mntpt);set_tracing_events_path ("tracing/" , mntpt); }
Signature · 1 line
2.4.3
A message that is not in the archive (sent off-list or not yet received): CAE_Gge2cn8LpuETTTkf2nP8JLj=9RDiuW3M8BXzv7ZTJpY9x7w@mail.gmail.com
Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Quoted 24 lines · Raphaël Beamonte
Quoted 3 lines · Jiri Olsa
The
tracing_events_pathis the variable we want to change via --debugfs-dir option, not thedebugfs_mountpoint.<SNIP>
Quoted 1 line
perf_debugfs_set_path(cmd +
strlen(CMD_DEBUGFS_DIR));Quoted 10 lines
tools/perf/perf.c +1
-1-fprintf (stderr,"dir: %s\n" , debugfs_mountpoint); +fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--list-cmds" )) {tools/perf/util/util.c +0
-0tools/perf/util/util.c const char *perf_debugfs_mount(const char *mountpoint) · line 442
Quoted 4 lines
tools/perf/util/util.c +0
-1@@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)void perf_debugfs_set_path (const char *mntpt) { - snprintf(debugfs_mountpoint, strlen(debugfs_mountpoint), "%s",mntpt);
Quoted 5 lines
tools/perf/util/util.c const char *perf_debugfs_mount(const char *mountpoint) · line 443
Quoted 2 lines
tools/perf/util/util.c +0
-0set_tracing_events_path ("tracing/" , mntpt) ; }Why keep a function name with debugfs in it here if we're not touching debugfs anymore? Shouldn't we call directly
set_tracing_events_pathif
please check patch 05/11
Quoted 2 lines
that's what we want to do? Also, the option name --debugfs-dir is not entirely relevant anymore?
probably, but it's out there and someone could be using it
jirka
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
In reply to a message not in the archiveEm Wed, Aug 26, 2015 at 10:06:45AM -0400, Raphaël Beamonte escreveu:
Quoted 24 lines · Jiri Olsa
Quoted 3 lines
The
tracing_events_pathis the variable we want to change via --debugfs-dir option, not thedebugfs_mountpoint.<SNIP>
Quoted 1 line
perf_debugfs_set_path(cmd +
strlen(CMD_DEBUGFS_DIR));Quoted 10 lines
tools/perf/perf.c +1
-1-fprintf (stderr,"dir: %s\n" , debugfs_mountpoint); +fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--list-cmds" )) {tools/perf/util/util.c +0
-0tools/perf/util/util.c const char *perf_debugfs_mount(const char *mountpoint) · line 442
Quoted 4 lines
tools/perf/util/util.c +0
-1@@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)void perf_debugfs_set_path (const char *mntpt) { - snprintf(debugfs_mountpoint, strlen(debugfs_mountpoint), "%s",mntpt);
Quoted 5 lines
tools/perf/util/util.c const char *perf_debugfs_mount(const char *mountpoint) · line 443
Quoted 2 lines
tools/perf/util/util.c +0
-0set_tracing_events_path ("tracing/" , mntpt) ; }Why keep a function name with debugfs in it here if we're not touching debugfs anymore?
Right
Quoted 3 lines
Shouldn't we call directly
set_tracing_events_pathif that's what we want to do? Also, the option name --debugfs-dir is not entirely relevant anymore?
Yeah, probably that was a really bad name to start with, i.e. it should've been named perhaps --event-definitions-dir or something to that degree.
"debugfs", "tracefs" and the next thing to come in this area are just implementation details :-)
- Arnaldo
Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
Quoted 7 lines · Arnaldo Carvalho de Melo
Yeah, probably that was a really bad name to start with, i.e. it should've been named perhaps --event-definitions-dir or something to that degree.
"debugfs", "tracefs" and the next thing to come in this area are just implementation details :-)
Perhaps there should be an alias for --debugfs-dir that more accurately reflects the "events definition dir" use?
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Re: [PATCH 03/11] perf tools: Do not change lib/api/fs/debugfs directly
In reply to Jiri OlsaQuoted 9 lines · Jiri Olsa
The
tracing_events_pathis the variable we want to change via --debugfs-dir option, not thedebugfs_mountpoint.Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/perf.c | 2 +- tools/perf/util/util.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tools: Do not change lib/api/fs/ debugfs directly
In reply to Jiri Olsaperf tools: Do not change lib/api/fs/debugfs directly
Commit-ID: 9f30fffc78ca35c862f74f34cc597c7fdddc8793 Gitweb: http://git.kernel.org/tip/9f30fffc78ca35c862f74f34cc597c7fdddc8793 Author: Jiri Olsa <[email protected]> AuthorDate: Wed, 26 Aug 2015 15:46:45 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 28 Aug 2015 14:53:53 -0300
perf tools: Do not change lib/api/fs/debugfs directly
The tracing_events_path is the variable we want to change via --debugfs-dir option, not the debugfs_mountpoint.
--- tools/perf/perf.c | 2 +- tools/perf/util/util.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
tools/perf/perf.c +1 -1
@@ -231,7 +231,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argc)--; }else if (!prefixcmp (cmd,CMD_DEBUGFS_DIR )) {perf_debugfs_set_path (cmd +strlen (CMD_DEBUGFS_DIR )); -fprintf (stderr,"dir: %s\n" , debugfs_mountpoint); +fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ; }else if (!strcmp (cmd,"--list-cmds" )) {
tools/perf/util/util.c +0 -1
@@ -439,7 +439,6 @@ const char *perf_debugfs_mount(const char *mountpoint)void perf_debugfs_set_path (const char *mntpt) { -snprintf (debugfs_mountpoint,strlen (debugfs_mountpoint),"%s" , mntpt);set_tracing_events_path ("tracing/" , mntpt); }
Jiri Olsa
reviewerPATCH 4/11 perf tools: Move debugfs__strerror_open into util.c object
In reply to Jiri OlsaMoving debugfs__strerror_open out of api/fs/debugfs.c, because it's not debugfs specific. It'll be changed to consider tracefs mount as well in following patches.
--- tools/lib/api/fs/debugfs.c | 51 --------------------------------------------- tools/lib/api/fs/debugfs.h | 3 --- tools/perf/util/util.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/util.h | 2 ++ 4 files changed, 54 insertions(+), 54 deletions(-)
tools/lib/api/fs/debugfs.c +0 -51
@@ -76,54 +76,3 @@ out:return debugfs_mountpoint; } -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) -{ -char sbuf[128 ]; - -switch (err) { -case ENOENT : -if (debugfs_found) { -snprintf (buf, size, -"Error:\tFile %s/%s not found.\n" -"Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n" , - debugfs_mountpoint, filename); -break ; - } -snprintf (buf, size,"%s" , -"Error:\tUnable to find debugfs\n" -"Hint:\tWas your kernel compiled with debugfs support?\n" -"Hint:\tIs the debugfs filesystem mounted?\n" -"Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'" ); -break ; -case EACCES : { -const char *mountpoint = debugfs_mountpoint; - -if (!access (debugfs_mountpoint,R_OK ) &&strncmp (filename,"tracing/" ,8 ) ==0 ) { -const char *tracefs_mntpoint =tracefs_find_mountpoint (); - -if (tracefs_mntpoint) - mountpoint = tracefs_mntpoint; - } - -snprintf (buf, size, -"Error:\tNo permissions to read %s/%s\n" -"Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n" , - debugfs_mountpoint, filename, mountpoint); - } -break ; -default : -snprintf (buf, size,"%s" ,strerror_r (err, sbuf,sizeof (sbuf))); -break ; - } - -return 0 ; -} - -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) -{ -char path[PATH_MAX ]; - -snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); - -return debugfs__strerror_open (err, buf, size, path); -}
tools/lib/api/fs/debugfs.h +0 -3
@@ -17,7 +17,4 @@ char *debugfs_mount(const char *mountpoint);extern char debugfs_mountpoint[]; -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); -#endif /* __API_DEBUGFS_H__ */
tools/perf/util/util.c +52 -0
58 lines not on this page · raw message
tools/perf/util/util.h +2 -0
@@ -87,6 +87,8 @@ extern char tracing_path[];extern char tracing_events_path[];extern void perf_debugfs_set_path (const char *mountpoint);const char *perf_debugfs_mount (const char *mountpoint); +int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); +int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name);char *get_tracing_file (const char *name);void put_tracing_file (char *file);
Signature · 1 line
2.4.3
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 04/11] perf tools: Move debugfs__strerror_open into util.c object
Em Wed, Aug 26, 2015 at 03:46:46PM +0200, Jiri Olsa escreveu:
Quoted 3 lines
Moving
debugfs__strerror_openout ofapi/fs/debugfs.c, because it's not debugfs specific. It'll be changed to consider tracefs mount as well in following patches.
If it is "debugfs__" it should not deal with tracefs, right?
My feeling is that they should be decoupled more, not the other way around :-\
- Arnaldo
Quoted 158 lines
Signed-off-by Jiri Olsa <[email protected]>--- tools/lib/api/fs/debugfs.c | 51 --------------------------------------------- tools/lib/api/fs/debugfs.h | 3 --- tools/perf/util/util.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/util.h | 2 ++ 4 files changed, 54 insertions(+), 54 deletions(-)tools/lib/api/fs/debugfs.c +0
-51@@ -76,54 +76,3 @@ out:return debugfs_mountpoint; } -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) -{ -char sbuf[128 ]; - -switch (err) { -case ENOENT : -if (debugfs_found) { -snprintf (buf, size, -"Error:\tFile %s/%s not found.\n" -"Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n" , - debugfs_mountpoint, filename); -break ; - } -snprintf (buf, size,"%s" , -"Error:\tUnable to find debugfs\n" -"Hint:\tWas your kernel compiled with debugfs support?\n" -"Hint:\tIs the debugfs filesystem mounted?\n" -"Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'" ); -break ; -case EACCES : { -const char *mountpoint = debugfs_mountpoint; - -if (!access (debugfs_mountpoint,R_OK ) &&strncmp (filename,"tracing/" ,8 ) ==0 ) { -const char *tracefs_mntpoint =tracefs_find_mountpoint (); - -if (tracefs_mntpoint) - mountpoint = tracefs_mntpoint; - } - -snprintf (buf, size, -"Error:\tNo permissions to read %s/%s\n" -"Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n" , - debugfs_mountpoint, filename, mountpoint); - } -break ; -default : -snprintf (buf, size,"%s" ,strerror_r (err, sbuf,sizeof (sbuf))); -break ; - } - -return 0 ; -} - -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) -{ -char path[PATH_MAX ]; - -snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); - -return debugfs__strerror_open (err, buf, size, path); -}tools/lib/api/fs/debugfs.h +0
-3@@ -17,7 +17,4 @@ char *debugfs_mount(const char *mountpoint);extern char debugfs_mountpoint[]; -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); -#endif /* __API_DEBUGFS_H__ */ tools/perf/util/util.c +52
-0@@ -442,6 +442,58 @@ void perf_debugfs_set_path(const char *mntpt)set_tracing_events_path ("tracing/" , mntpt); } +int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) +{ +char sbuf[128 ]; + +switch (err) { +case ENOENT : +if (debugfs_configured ()) { +snprintf (buf, size, +"Error:\tFile %s/%s not found.\n" +"Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n" , + debugfs_mountpoint, filename); +break ; + } +snprintf (buf, size,"%s" , +"Error:\tUnable to find debugfs\n" +"Hint:\tWas your kernel compiled with debugfs support?\n" +"Hint:\tIs the debugfs filesystem mounted?\n" +"Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'" ); +break ; +case EACCES : { +const char *mountpoint = debugfs_mountpoint; + +if (!access (debugfs_mountpoint,R_OK ) &&strncmp (filename,"tracing/" ,8 ) ==0 ) { +const char *tracefs_mntpoint =tracefs_find_mountpoint (); + +if (tracefs_mntpoint) + mountpoint = tracefs_mntpoint; + } + +snprintf (buf, size, +"Error:\tNo permissions to read %s/%s\n" +"Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n" , + debugfs_mountpoint, filename, mountpoint); + } +break ; +default : +snprintf (buf, size,"%s" ,strerror_r (err, sbuf,sizeof (sbuf))); +break ; + } + +return 0 ; +} + +int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) +{ +char path[PATH_MAX ]; + +snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); + +return debugfs__strerror_open (err, buf, size, path); +} +char *get_tracing_file (const char *name) {char *file;tools/perf/util/util.h +2
-0@@ -87,6 +87,8 @@ extern char tracing_path[];extern char tracing_events_path[];extern void perf_debugfs_set_path (const char *mountpoint);const char *perf_debugfs_mount (const char *mountpoint); +int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); +int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name);char *get_tracing_file (const char *name);void put_tracing_file (char *file);Signature · 1 line
2.4.3
Re: [PATCH 04/11] perf tools: Move debugfs__strerror_open into util.c object
Quoted 9 lines · Arnaldo Carvalho de Melo
Em Wed, Aug 26, 2015 at 03:46:46PM +0200, Jiri Olsa escreveu:
Quoted 3 lines
Moving
debugfs__strerror_openout ofapi/fs/debugfs.c, because it's not debugfs specific. It'll be changed to consider tracefs mount as well in following patches.If it is "
debugfs__" it should not deal with tracefs, right?My feeling is that they should be decoupled more, not the other way around :-\
please check 5/11:
perf tools: Move tracing_path stuff under same namespace
jirka
Re: [PATCH 04/11] perf tools: Move debugfs__strerror_open into util.c object
In reply to Jiri OlsaQuoted 4 lines · Jiri Olsa
Moving
debugfs__strerror_openout ofapi/fs/debugfs.c, because it's not debugfs specific. It'll be changed to consider tracefs mount as well in following patches.
Quoted 8 lines
Signed-off-by Jiri Olsa <[email protected]>--- tools/lib/api/fs/debugfs.c | 51 --------------------------------------------- tools/lib/api/fs/debugfs.h | 3 --- tools/perf/util/util.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/util.h | 2 ++ 4 files changed, 54 insertions(+), 54 deletions(-)
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Jiri Olsa
reviewerPATCH 5/11 perf tools: Move tracing_path stuff under same namespace
In reply to Jiri OlsaRenaming all functions touching tracing_path under same namespace. New interface is:
char tracing_path[]; char tracing_events_path[]; - tracing mount
void perf_tracing_path_set(const char *mountpoint);
- setting directly tracing_path(_events), used by --debugfs-dir option
const char *perf_tracing_path_mount(const char *mountpoint);
- initial setup of tracing_path(_events), called from perf.c
int tracing_path_strerror_open(int err, char *buf, size_t size, const char *filename); int tracing_path_strerror_open_tp(int err, char *buf, size_t size, const char *sys, const char *name);
- strerror functions to get error string when failing to open
tracepoint files
char *get_tracing_file(const char *name); void put_tracing_file(char *file); - get/put tracing file path
--- tools/perf/builtin-trace.c | 4 ++-- tools/perf/perf.c | 8 ++++---- tools/perf/util/util.c | 26 +++++++++++++------------- tools/perf/util/util.h | 8 ++++---- 4 files changed, 23 insertions(+), 23 deletions(-)
tools/perf/builtin-trace.c +2 -2
@@ -2667,11 +2667,11 @@ out_delete_evlist:char errbuf[BUFSIZ ]; out_error_sched_stat_runtime: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); goto out_error; out_error_raw_syscalls: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); goto out_error; out_error_mmap:
tools/perf/perf.c +4 -4
@@ -214,7 +214,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)fprintf (stderr,"No directory given for --debugfs-dir.\n" );usage (perf_usage_string); } -perf_debugfs_set_path ((*argv)[1 ]); +perf_tracing_path_set ((*argv)[1 ]);if (envchanged) *envchanged =1 ; (*argv)++;
@@ -230,7 +230,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argv)++; (*argc)--; }else if (!prefixcmp (cmd,CMD_DEBUGFS_DIR )) { -perf_debugfs_set_path (cmd +strlen (CMD_DEBUGFS_DIR )); +perf_tracing_path_set (cmd +strlen (CMD_DEBUGFS_DIR ));fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ;
@@ -517,8 +517,8 @@ int main(int argc, const char **argv) cmd =perf_extract_argv0_path (argv[0 ]);if (!cmd) cmd ="perf-help" ; -/* get debugfs mount point from /proc/mounts */ -perf_debugfs_mount (NULL ); +/* get debugfs/tracefs mount point from /proc/mounts */ +perf_tracing_path_mount (NULL );/* * "perf-xxxx" is the same as "perf xxxx", but we obviously: *
tools/perf/util/util.c +13 -13
@@ -390,7 +390,7 @@ void set_term_quiet_input(struct termios *old)tcsetattr (0 ,TCSANOW , &tc); } -static void set_tracing_events_path (const char *tracing,const char *mountpoint) +static void tracing_path_set (const char *tracing,const char *mountpoint) {snprintf (tracing_path,sizeof (tracing_path),"%s/%s" , mountpoint, tracing);
@@ -398,7 +398,7 @@ static void set_tracing_events_path(const char *tracing, const char *mountpoint) mountpoint, tracing, "events"); } -static const char *__perf_tracefs_mount (const char *mountpoint) +static const char *tracing_path_tracefs_mount (const char *mountpoint) {const char *mnt;
@@ -406,12 +406,12 @@ static const char *__perf_tracefs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("" , mnt); +tracing_path_set ("" , mnt);return mnt; } -static const char *__perf_debugfs_mount (const char *mountpoint) +static const char *tracing_path_debugfs_mount (const char *mountpoint) {const char *mnt;
@@ -419,30 +419,30 @@ static const char *__perf_debugfs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("tracing/" , mnt); +tracing_path_set ("tracing/" , mnt);return mnt; } -const char *perf_debugfs_mount (const char *mountpoint) +const char *perf_tracing_path_mount (const char *mountpoint) {const char *mnt; - mnt =__perf_tracefs_mount (mountpoint); + mnt =tracing_path_tracefs_mount (mountpoint);if (mnt)return mnt; - mnt =__perf_debugfs_mount (mountpoint); + mnt =tracing_path_debugfs_mount (mountpoint);return mnt; } -void perf_debugfs_set_path (const char *mntpt) +void perf_tracing_path_set (const char *mntpt) { -set_tracing_events_path ("tracing/" , mntpt); +tracing_path_set ("tracing/" , mntpt); } -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename) {char sbuf[128 ];
@@ -485,13 +485,13 @@ int debugfs__strerror_open(int err, char *buf, size_t size, const char *filenamereturn 0 ; } -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) {char path[PATH_MAX ];snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); -return debugfs__strerror_open (err, buf, size, path); +return tracing_path_strerror_open (err, buf, size, path); }char *get_tracing_file (const char *name)
tools/perf/util/util.h +4 -4
@@ -85,10 +85,10 @@ extern const char *graph_dotted_line;extern char buildid_dir[];extern char tracing_path[];extern char tracing_events_path[]; -extern void perf_debugfs_set_path (const char *mountpoint); -const char *perf_debugfs_mount (const char *mountpoint); -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); +extern void perf_tracing_path_set (const char *mountpoint); +const char *perf_tracing_path_mount (const char *mountpoint); +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename); +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name);char *get_tracing_file (const char *name);void put_tracing_file (char *file);
Signature · 1 line
2.4.3
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 05/11] perf tools: Move tracing_path stuff under same namespace
Em Wed, Aug 26, 2015 at 03:46:47PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
Renaming all functions touching
tracing_pathunder same namespace. New interface is:
But we were trying to have debugfs stuff in tools/lib/api/fs/, so that it could eventually be used by some other tools, etc, and now we're going the other way around, de-librarifying, not good :-\
- Arnaldo
Quoted 191 lines
char tracing_path[]; char tracing_events_path[]; - tracing mountvoid
perf_tracing_path_set(const char *mountpoint);
- setting directlytracing_path(_events), used by --debugfs-dir optionconst char *perf_tracing_path_mount(const char *mountpoint);
- initial setup oftracing_path(_events), called fromperf.cint
tracing_path_strerror_open(int err, char *buf, size_t size, const char *filename); inttracing_path_strerror_open_tp(int err, char *buf, size_t size, const char *sys, const char *name);
- strerror functions to get error string when failing to open
tracepoint fileschar *get_tracing_file(const char *name); void put_tracing_file(char *file); - get/put tracing file pathSigned-off-by Jiri Olsa <[email protected]>--- tools/perf/builtin-trace.c | 4 ++-- tools/perf/perf.c | 8 ++++---- tools/perf/util/util.c | 26 +++++++++++++------------- tools/perf/util/util.h | 8 ++++---- 4 files changed, 23 insertions(+), 23 deletions(-)tools/perf/builtin-trace.c +2
-2@@ -2667,11 +2667,11 @@ out_delete_evlist:char errbuf[BUFSIZ ]; out_error_sched_stat_runtime: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); goto out_error; out_error_raw_syscalls: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); goto out_error; out_error_mmap:tools/perf/perf.c +4
-4@@ -214,7 +214,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)fprintf (stderr,"No directory given for --debugfs-dir.\n" );usage (perf_usage_string); } -perf_debugfs_set_path ((*argv)[1 ]); +perf_tracing_path_set ((*argv)[1 ]);if (envchanged) *envchanged =1 ; (*argv)++;@@ -230,7 +230,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argv)++; (*argc)--; }else if (!prefixcmp (cmd,CMD_DEBUGFS_DIR )) { -perf_debugfs_set_path (cmd +strlen (CMD_DEBUGFS_DIR )); +perf_tracing_path_set (cmd +strlen (CMD_DEBUGFS_DIR ));fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ;@@ -517,8 +517,8 @@ int main(int argc, const char **argv) cmd =perf_extract_argv0_path (argv[0 ]);if (!cmd) cmd ="perf-help" ; -/* get debugfs mount point from /proc/mounts */ -perf_debugfs_mount (NULL ); +/* get debugfs/tracefs mount point from /proc/mounts */ +perf_tracing_path_mount (NULL );/* * "perf-xxxx" is the same as "perf xxxx", but we obviously: * tools/perf/util/util.c +13
-13@@ -390,7 +390,7 @@ void set_term_quiet_input(struct termios *old)tcsetattr (0 ,TCSANOW , &tc); } -static void set_tracing_events_path (const char *tracing,const char *mountpoint) +static void tracing_path_set (const char *tracing,const char *mountpoint) {snprintf (tracing_path,sizeof (tracing_path),"%s/%s" , mountpoint, tracing);@@ -398,7 +398,7 @@ static void set_tracing_events_path(const char *tracing, const char *mountpoint) mountpoint, tracing, "events"); } -static const char *__perf_tracefs_mount (const char *mountpoint) +static const char *tracing_path_tracefs_mount (const char *mountpoint) {const char *mnt;@@ -406,12 +406,12 @@ static const char *__perf_tracefs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("" , mnt); +tracing_path_set ("" , mnt);return mnt; } -static const char *__perf_debugfs_mount (const char *mountpoint) +static const char *tracing_path_debugfs_mount (const char *mountpoint) {const char *mnt;@@ -419,30 +419,30 @@ static const char *__perf_debugfs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("tracing/" , mnt); +tracing_path_set ("tracing/" , mnt);return mnt; } -const char *perf_debugfs_mount (const char *mountpoint) +const char *perf_tracing_path_mount (const char *mountpoint) {const char *mnt; - mnt =__perf_tracefs_mount (mountpoint); + mnt =tracing_path_tracefs_mount (mountpoint);if (mnt)return mnt; - mnt =__perf_debugfs_mount (mountpoint); + mnt =tracing_path_debugfs_mount (mountpoint);return mnt; } -void perf_debugfs_set_path (const char *mntpt) +void perf_tracing_path_set (const char *mntpt) { -set_tracing_events_path ("tracing/" , mntpt); +tracing_path_set ("tracing/" , mntpt); } -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename) {char sbuf[128 ];@@ -485,13 +485,13 @@ int debugfs__strerror_open(int err, char *buf, size_t size, const char *filenamereturn 0 ; } -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) {char path[PATH_MAX ];snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); -return debugfs__strerror_open (err, buf, size, path); +return tracing_path_strerror_open (err, buf, size, path); }char *get_tracing_file (const char *name)tools/perf/util/util.h +4
-4@@ -85,10 +85,10 @@ extern const char *graph_dotted_line;extern char buildid_dir[];extern char tracing_path[];extern char tracing_events_path[]; -extern void perf_debugfs_set_path (const char *mountpoint); -const char *perf_debugfs_mount (const char *mountpoint); -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); +extern void perf_tracing_path_set (const char *mountpoint); +const char *perf_tracing_path_mount (const char *mountpoint); +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename); +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name);char *get_tracing_file (const char *name);void put_tracing_file (char *file);Signature · 1 line
2.4.3
Re: [PATCH 05/11] perf tools: Move tracing_path stuff under same namespace
Quoted 7 lines · Arnaldo Carvalho de Melo
Em Wed, Aug 26, 2015 at 03:46:47PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
Renaming all functions touching
tracing_pathunder same namespace. New interface is:But we were trying to have debugfs stuff in
tools/lib/api/fs/, so that it could eventually be used by some other tools, etc, and now we're going the other way around, de-librarifying, not good :-\
well this gathers tracefs/debugfs together, the api/fs/{trace|debug}fs are just building blocks
I only moved the debugfs__strerror_open_tp out of the debugfs object because it needs to be one layer up, because it needs to touch tracefs as well
jirka
Quoted 194 lines
- Arnaldo
Quoted 191 lines
char tracing_path[]; char tracing_events_path[]; - tracing mountvoid
perf_tracing_path_set(const char *mountpoint);
- setting directlytracing_path(_events), used by --debugfs-dir optionconst char *perf_tracing_path_mount(const char *mountpoint);
- initial setup oftracing_path(_events), called fromperf.cint
tracing_path_strerror_open(int err, char *buf, size_t size, const char *filename); inttracing_path_strerror_open_tp(int err, char *buf, size_t size, const char *sys, const char *name);
- strerror functions to get error string when failing to open
tracepoint fileschar *get_tracing_file(const char *name); void put_tracing_file(char *file); - get/put tracing file pathSigned-off-by Jiri Olsa <[email protected]>--- tools/perf/builtin-trace.c | 4 ++-- tools/perf/perf.c | 8 ++++---- tools/perf/util/util.c | 26 +++++++++++++------------- tools/perf/util/util.h | 8 ++++---- 4 files changed, 23 insertions(+), 23 deletions(-)tools/perf/builtin-trace.c +2
-2@@ -2667,11 +2667,11 @@ out_delete_evlist:char errbuf[BUFSIZ ]; out_error_sched_stat_runtime: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); goto out_error; out_error_raw_syscalls: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); goto out_error; out_error_mmap:tools/perf/perf.c +4
-4@@ -214,7 +214,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)fprintf (stderr,"No directory given for --debugfs-dir.\n" );usage (perf_usage_string); } -perf_debugfs_set_path ((*argv)[1 ]); +perf_tracing_path_set ((*argv)[1 ]);if (envchanged) *envchanged =1 ; (*argv)++;@@ -230,7 +230,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argv)++; (*argc)--; }else if (!prefixcmp (cmd,CMD_DEBUGFS_DIR )) { -perf_debugfs_set_path (cmd +strlen (CMD_DEBUGFS_DIR )); +perf_tracing_path_set (cmd +strlen (CMD_DEBUGFS_DIR ));fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ;@@ -517,8 +517,8 @@ int main(int argc, const char **argv) cmd =perf_extract_argv0_path (argv[0 ]);if (!cmd) cmd ="perf-help" ; -/* get debugfs mount point from /proc/mounts */ -perf_debugfs_mount (NULL ); +/* get debugfs/tracefs mount point from /proc/mounts */ +perf_tracing_path_mount (NULL );/* * "perf-xxxx" is the same as "perf xxxx", but we obviously: * tools/perf/util/util.c +13
-13@@ -390,7 +390,7 @@ void set_term_quiet_input(struct termios *old)tcsetattr (0 ,TCSANOW , &tc); } -static void set_tracing_events_path (const char *tracing,const char *mountpoint) +static void tracing_path_set (const char *tracing,const char *mountpoint) {snprintf (tracing_path,sizeof (tracing_path),"%s/%s" , mountpoint, tracing);@@ -398,7 +398,7 @@ static void set_tracing_events_path(const char *tracing, const char *mountpoint) mountpoint, tracing, "events"); } -static const char *__perf_tracefs_mount (const char *mountpoint) +static const char *tracing_path_tracefs_mount (const char *mountpoint) {const char *mnt;@@ -406,12 +406,12 @@ static const char *__perf_tracefs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("" , mnt); +tracing_path_set ("" , mnt);return mnt; } -static const char *__perf_debugfs_mount (const char *mountpoint) +static const char *tracing_path_debugfs_mount (const char *mountpoint) {const char *mnt;@@ -419,30 +419,30 @@ static const char *__perf_debugfs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("tracing/" , mnt); +tracing_path_set ("tracing/" , mnt);return mnt; } -const char *perf_debugfs_mount (const char *mountpoint) +const char *perf_tracing_path_mount (const char *mountpoint) {const char *mnt; - mnt =__perf_tracefs_mount (mountpoint); + mnt =tracing_path_tracefs_mount (mountpoint);if (mnt)return mnt; - mnt =__perf_debugfs_mount (mountpoint); + mnt =tracing_path_debugfs_mount (mountpoint);return mnt; } -void perf_debugfs_set_path (const char *mntpt) +void perf_tracing_path_set (const char *mntpt) { -set_tracing_events_path ("tracing/" , mntpt); +tracing_path_set ("tracing/" , mntpt); } -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename) {char sbuf[128 ];@@ -485,13 +485,13 @@ int debugfs__strerror_open(int err, char *buf, size_t size, const char *filenamereturn 0 ; } -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) {char path[PATH_MAX ];snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); -return debugfs__strerror_open (err, buf, size, path); +return tracing_path_strerror_open (err, buf, size, path); }char *get_tracing_file (const char *name)tools/perf/util/util.h +4
-4@@ -85,10 +85,10 @@ extern const char *graph_dotted_line;extern char buildid_dir[];extern char tracing_path[];extern char tracing_events_path[]; -extern void perf_debugfs_set_path (const char *mountpoint); -const char *perf_debugfs_mount (const char *mountpoint); -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); +extern void perf_tracing_path_set (const char *mountpoint); +const char *perf_tracing_path_mount (const char *mountpoint); +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename); +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name);char *get_tracing_file (const char *name);void put_tracing_file (char *file);Signature · 1 line
2.4.3
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 05/11] perf tools: Move tracing_path stuff under same namespace
Em Wed, Aug 26, 2015 at 04:48:36PM +0200, Jiri Olsa escreveu:
Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 7 lines
Em Wed, Aug 26, 2015 at 03:46:47PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
Renaming all functions touching
tracing_pathunder same namespace. New interface is:But we were trying to have debugfs stuff in
tools/lib/api/fs/, so that it could eventually be used by some other tools, etc, and now we're going the other way around, de-librarifying, not good :-\
Quoted 6 lines
well this gathers tracefs/debugfs together, the api/fs/{trace|debug}fs are just building blocks
I only moved the
debugfs__strerror_open_tpout of the debugfs object because it needs to be one layer up, because it needs to touch tracefs as well
Why not leave it there, since, for historical reasons, what is tracefs now was something implemented inside debugfs, so having that special case in debugfs__strerror_open_tp():
if (!access(debugfs_mountpoint, R_OK) && strncmp(filename, "tracing/", 8) == 0) {Is ok, since tools will need to deal with older kernels, etc.
But ok, I see your point, we should rename it to something like tracepoint__strerror_open_definition(), so as to detach this from "debugfs" _and_ "tracefs", pseudo-filesystem based DWARF-like stuff (event definitions), but then why not have that somewhere in tools/lib/api/tracepoint/.
Or was it the difficulty in getting this nicely namespaced what made you get it back to tools/perf/util/?
I can have some empathy for that, if that is the case ;-\
- Arnaldo
Quoted 194 lines
Quoted 194 lines
- Arnaldo
Quoted 191 lines
char tracing_path[]; char tracing_events_path[]; - tracing mountvoid
perf_tracing_path_set(const char *mountpoint);
- setting directlytracing_path(_events), used by --debugfs-dir optionconst char *perf_tracing_path_mount(const char *mountpoint);
- initial setup oftracing_path(_events), called fromperf.cint
tracing_path_strerror_open(int err, char *buf, size_t size, const char *filename); inttracing_path_strerror_open_tp(int err, char *buf, size_t size, const char *sys, const char *name);
- strerror functions to get error string when failing to open
tracepoint fileschar *get_tracing_file(const char *name); void put_tracing_file(char *file); - get/put tracing file pathSigned-off-by Jiri Olsa <[email protected]>--- tools/perf/builtin-trace.c | 4 ++-- tools/perf/perf.c | 8 ++++---- tools/perf/util/util.c | 26 +++++++++++++------------- tools/perf/util/util.h | 8 ++++---- 4 files changed, 23 insertions(+), 23 deletions(-)tools/perf/builtin-trace.c +2
-2@@ -2667,11 +2667,11 @@ out_delete_evlist:char errbuf[BUFSIZ ]; out_error_sched_stat_runtime: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"sched" ,"sched_stat_runtime" ); goto out_error; out_error_raw_syscalls: -debugfs__strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); +tracing_path_strerror_open_tp (errno, errbuf,sizeof (errbuf),"raw_syscalls" ,"sys_(enter|exit)" ); goto out_error; out_error_mmap:tools/perf/perf.c +4
-4@@ -214,7 +214,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)fprintf (stderr,"No directory given for --debugfs-dir.\n" );usage (perf_usage_string); } -perf_debugfs_set_path ((*argv)[1 ]); +perf_tracing_path_set ((*argv)[1 ]);if (envchanged) *envchanged =1 ; (*argv)++;@@ -230,7 +230,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) (*argv)++; (*argc)--; }else if (!prefixcmp (cmd,CMD_DEBUGFS_DIR )) { -perf_debugfs_set_path (cmd +strlen (CMD_DEBUGFS_DIR )); +perf_tracing_path_set (cmd +strlen (CMD_DEBUGFS_DIR ));fprintf (stderr,"dir: %s\n" , tracing_path);if (envchanged) *envchanged =1 ;@@ -517,8 +517,8 @@ int main(int argc, const char **argv) cmd =perf_extract_argv0_path (argv[0 ]);if (!cmd) cmd ="perf-help" ; -/* get debugfs mount point from /proc/mounts */ -perf_debugfs_mount (NULL ); +/* get debugfs/tracefs mount point from /proc/mounts */ +perf_tracing_path_mount (NULL );/* * "perf-xxxx" is the same as "perf xxxx", but we obviously: * tools/perf/util/util.c +13
-13@@ -390,7 +390,7 @@ void set_term_quiet_input(struct termios *old)tcsetattr (0 ,TCSANOW , &tc); } -static void set_tracing_events_path (const char *tracing,const char *mountpoint) +static void tracing_path_set (const char *tracing,const char *mountpoint) {snprintf (tracing_path,sizeof (tracing_path),"%s/%s" , mountpoint, tracing);@@ -398,7 +398,7 @@ static void set_tracing_events_path(const char *tracing, const char *mountpoint) mountpoint, tracing, "events"); } -static const char *__perf_tracefs_mount (const char *mountpoint) +static const char *tracing_path_tracefs_mount (const char *mountpoint) {const char *mnt;@@ -406,12 +406,12 @@ static const char *__perf_tracefs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("" , mnt); +tracing_path_set ("" , mnt);return mnt; } -static const char *__perf_debugfs_mount (const char *mountpoint) +static const char *tracing_path_debugfs_mount (const char *mountpoint) {const char *mnt;@@ -419,30 +419,30 @@ static const char *__perf_debugfs_mount(const char *mountpoint)if (!mnt)return NULL ; -set_tracing_events_path ("tracing/" , mnt); +tracing_path_set ("tracing/" , mnt);return mnt; } -const char *perf_debugfs_mount (const char *mountpoint) +const char *perf_tracing_path_mount (const char *mountpoint) {const char *mnt; - mnt =__perf_tracefs_mount (mountpoint); + mnt =tracing_path_tracefs_mount (mountpoint);if (mnt)return mnt; - mnt =__perf_debugfs_mount (mountpoint); + mnt =tracing_path_debugfs_mount (mountpoint);return mnt; } -void perf_debugfs_set_path (const char *mntpt) +void perf_tracing_path_set (const char *mntpt) { -set_tracing_events_path ("tracing/" , mntpt); +tracing_path_set ("tracing/" , mntpt); } -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename) +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename) {char sbuf[128 ];@@ -485,13 +485,13 @@ int debugfs__strerror_open(int err, char *buf, size_t size, const char *filenamereturn 0 ; } -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name) {char path[PATH_MAX ];snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); -return debugfs__strerror_open (err, buf, size, path); +return tracing_path_strerror_open (err, buf, size, path); }char *get_tracing_file (const char *name)tools/perf/util/util.h +4
-4@@ -85,10 +85,10 @@ extern const char *graph_dotted_line;extern char buildid_dir[];extern char tracing_path[];extern char tracing_events_path[]; -extern void perf_debugfs_set_path (const char *mountpoint); -const char *perf_debugfs_mount (const char *mountpoint); -int debugfs__strerror_open (int err,char *buf,size_t size,const char *filename); -int debugfs__strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); +extern void perf_tracing_path_set (const char *mountpoint); +const char *perf_tracing_path_mount (const char *mountpoint); +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename); +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name);char *get_tracing_file (const char *name);void put_tracing_file (char *file);Signature · 1 line
2.4.3
Re: [PATCH 05/11] perf tools: Move tracing_path stuff under same namespace
Quoted 23 lines · Arnaldo Carvalho de Melo
Em Wed, Aug 26, 2015 at 04:48:36PM +0200, Jiri Olsa escreveu:
Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 7 lines
Em Wed, Aug 26, 2015 at 03:46:47PM +0200, Jiri Olsa escreveu:
Quoted 2 lines
Renaming all functions touching
tracing_pathunder same namespace. New interface is:But we were trying to have debugfs stuff in
tools/lib/api/fs/, so that it could eventually be used by some other tools, etc, and now we're going the other way around, de-librarifying, not good :-\Quoted 6 lines
well this gathers tracefs/debugfs together, the api/fs/{trace|debug}fs are just building blocks
I only moved the
debugfs__strerror_open_tpout of the debugfs object because it needs to be one layer up, because it needs to touch tracefs as wellWhy not leave it there, since, for historical reasons, what is tracefs now was something implemented inside debugfs, so having that special case in
debugfs__strerror_open_tp():if (!access(debugfs_mountpoint, R_OK) && strncmp(filename, "tracing/", 8) == 0) {
Quoted 7 lines
Is ok, since tools will need to deal with older kernels, etc.
But ok, I see your point, we should rename it to something like
tracepoint__strerror_open_definition(), so as to detach this from "debugfs" _and_ "tracefs", pseudo-filesystem based DWARF-like stuff (event definitions), but then why not have that somewhere intools/lib/api/tracepoint/.
no problem with moving it over to the lib
how about having api/fs/tpfs
or eventsfs
or tracingfs
doing the same as tracing_path* stuff ATM
jirka
Re: [PATCH 05/11] perf tools: Move tracing_path stuff under same namespace
Quoted 9 lines · Jiri Olsa
no problem with moving it over to the lib
how about having api/fs/tpfs
or eventsfs
or tracingfsdoing the same as tracing_path* stuff ATM
But we're talking about tracing path components here right and not a file system? It'd make more sense to me to have this logically above the file system layers.
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Re: [PATCH 05/11] perf tools: Move tracing_path stuff under same namespace
Quoted 9 lines · Matt Fleming
Quoted 8 lines · Jiri Olsa
no problem with moving it over to the lib
how about having api/fs/tpfs
or eventsfs
or tracingfs
Quoted 5 lines
Quoted 1 line
doing the same as tracing_path* stuff ATM
But we're talking about tracing path components here right and not a file system? It'd make more sense to me to have this logically above the file system layers.
right, on the other side it's fs related.. just holder for preffered place to read tracepoints from
I dont have strong opinion on where to place it,
but I dont see better place.. fs/tracing_path.c ?
jirka
Jiri Olsa
reviewerPATCH 6/11 perf tools: Move tracing_path interface into trace-event-path.c
In reply to Jiri OlsaMoving tracing_path interface into trace-event-path.c out of util.c.
--- tools/perf/perf.c | 1 + tools/perf/tests/openat-syscall-all-cpus.c | 2 + tools/perf/tests/openat-syscall.c | 2 + tools/perf/util/Build | 1 + tools/perf/util/trace-event-path.c | 129 +++++++++++++++++++++++++++++ tools/perf/util/trace-event.h | 12 +++ tools/perf/util/util.c | 122 --------------------------- tools/perf/util/util.h | 10 --- 8 files changed, 147 insertions(+), 132 deletions(-) create mode 100644 tools/perf/util/trace-event-path.c
tools/perf/perf.c +1 -0
@@ -15,6 +15,7 @@#include "util/parse-events.h" #include "util/parse-options.h" #include "util/debug.h" +#include "util/trace-event.h" #include <api/fs/debugfs.h> #include <pthread.h>
tools/perf/tests/openat-syscall-all-cpus.c +2 -0
@@ -1,3 +1,5 @@ +#include <api/fs/debugfs.h> +#include <api/fs/tracefs.h> #include "evsel.h" #include "tests.h" #include "thread_map.h"
tools/perf/tests/openat-syscall.c +2 -0
@@ -1,3 +1,5 @@ +#include <api/fs/debugfs.h> +#include <api/fs/tracefs.h> #include "thread_map.h" #include "evsel.h" #include "debug.h"
tools/perf/util/Build +1 -0
@@ -56,6 +56,7 @@ libperf-y += pmu-bison.o libperf-y += trace-event-read.o libperf-y += trace-event-info.o libperf-y += trace-event-scripting.o +libperf-y += trace-event-path.o libperf-y += trace-event.o libperf-y += svghelper.o libperf-y += sort.o
tools/perf/util/trace-event-path.c new file +129 -0
129 lines not on this page · raw message
tools/perf/util/trace-event.h +12 -0
@@ -92,4 +92,16 @@ int common_pc(struct scripting_context *context);int common_flags (struct scripting_context *context);int common_lock_depth (struct scripting_context *context); +/* +* The tracing_path interface from trace-event-path.c object. +*/ +extern char tracing_path[]; +extern char tracing_events_path[]; +extern void perf_tracing_path_set (const char *mountpoint); +const char *perf_tracing_path_mount (const char *mountpoint); +int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename); +int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); +char *get_tracing_file (const char *name); +void put_tracing_file (char *file); +#endif /* _PERF_UTIL_TRACE_EVENT_H */
tools/perf/util/util.c +0 -122
134 lines not on this page · raw message
tools/perf/util/util.h +0 -10
@@ -74,8 +74,6 @@#include <linux/magic.h> #include <linux/types.h> #include <sys/ttydefaults.h> -#include <api/fs/debugfs.h> -#include <api/fs/tracefs.h> #include <termios.h> #include <linux/bitops.h> #include <termios.h>
@@ -83,14 +81,6 @@extern const char *graph_line;extern const char *graph_dotted_line;extern char buildid_dir[]; -extern char tracing_path[]; -extern char tracing_events_path[]; -extern void perf_tracing_path_set (const char *mountpoint); -const char *perf_tracing_path_mount (const char *mountpoint); -int tracing_path_strerror_open (int err,char *buf,size_t size,const char *filename); -int tracing_path_strerror_open_tp (int err,char *buf,size_t size,const char *sys,const char *name); -char *get_tracing_file (const char *name); -void put_tracing_file (char *file);/* On most systems <limits.h> would have given us this, but * not on some systems (e.g. GNU/Hurd).
Signature · 1 line
2.4.3
Jiri Olsa
reviewerPATCH 7/11 perf tools: Make tracing_path_strerror_open message generic
In reply to Jiri OlsaMaking tracing_path_strerror_open message generic by using both debugfs/tracefs words in error message plus the tracing_path instead of debugfs_mountpoint.
--- tools/perf/util/trace-event-path.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
tools/perf/util/trace-event-path.c +10 -10
@@ -66,33 +66,33 @@ int tracing_path_strerror_open(int err, char *buf, size_t size, const char *fileswitch (err) {case ENOENT : -if (debugfs_configured ()) { +if (debugfs_configured () ||tracefs_configured ()) {snprintf (buf, size,"Error:\tFile %s/%s not found.\n" "Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n" , - debugfs_mountpoint, filename); + tracing_events_path, filename);break ; }snprintf (buf, size,"%s" , -"Error:\tUnable to find debugfs\n" -"Hint:\tWas your kernel compiled with debugfs support?\n" -"Hint:\tIs the debugfs filesystem mounted?\n" +"Error:\tUnable to find debugfs/tracefs\n" +"Hint:\tWas your kernel compiled with debugfs/tracefs support?\n" +"Hint:\tIs the debugfs/tracefs filesystem mounted?\n" "Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'" );break ;case EACCES : { -const char *mountpoint = debugfs_mountpoint; +const char *mountpoint =debugfs_find_mountpoint (); -if (!access (debugfs_mountpoint,R_OK ) &&strncmp (filename,"tracing/" ,8 ) ==0 ) { +if (!access (mountpoint,R_OK ) &&strncmp (filename,"tracing/" ,8 ) ==0 ) {const char *tracefs_mntpoint =tracefs_find_mountpoint ();if (tracefs_mntpoint) - mountpoint = tracefs_mntpoint; + mountpoint =tracefs_find_mountpoint (); }snprintf (buf, size,"Error:\tNo permissions to read %s/%s\n" "Hint:\tTry 'sudo mount -o remount,mode=755 %s'\n" , - debugfs_mountpoint, filename, mountpoint); + tracing_events_path, filename, mountpoint); }break ;default :
@@ -107,7 +107,7 @@ int tracing_path_strerror_open_tp(int err, char *buf, size_t size, const char *s {char path[PATH_MAX ]; -snprintf (path,PATH_MAX ,"tracing/events/%s/%s" , sys, name ?:"*" ); +snprintf (path,PATH_MAX ,"%s/%s" , sys, name ?:"*" );return tracing_path_strerror_open (err, buf, size, path); }
Signature · 1 line
2.4.3
Jiri Olsa
reviewerPATCH 8/11 perf tools: Do not export debugfs_mountpoint and tracefs_mountpoint
In reply to Jiri OlsaBoth debugfs_mountpoint and tracefs_mountpoint should not be changed externally. Both mounpoints are returned by following interface:
debugfs_find_mountpoint
tracefs_find_mountpoint
--- tools/lib/api/fs/debugfs.c | 2 +- tools/lib/api/fs/debugfs.h | 2 -- tools/lib/api/fs/tracefs.c | 2 +- tools/lib/api/fs/tracefs.h | 2 -- 4 files changed, 2 insertions(+), 6 deletions(-)
tools/lib/api/fs/debugfs.c +1 -1
@@ -18,7 +18,7 @@#define DEBUGFS_DEFAULT_PATH "/sys/kernel/debug"#endif -char debugfs_mountpoint[PATH_MAX +1 ] =DEBUGFS_DEFAULT_PATH ; +static char debugfs_mountpoint[PATH_MAX +1 ] =DEBUGFS_DEFAULT_PATH ;static const char *const debugfs_known_mountpoints[] = {DEBUGFS_DEFAULT_PATH ,
tools/lib/api/fs/debugfs.h +0 -2
@@ -15,6 +15,4 @@ bool debugfs_configured(void);const char *debugfs_find_mountpoint (void );char *debugfs_mount (const char *mountpoint); -extern char debugfs_mountpoint[]; -#endif /* __API_DEBUGFS_H__ */
tools/lib/api/fs/tracefs.c +1 -1
@@ -16,7 +16,7 @@#define TRACEFS_DEFAULT_PATH "/sys/kernel/tracing"#endif -char tracefs_mountpoint[PATH_MAX +1 ] =TRACEFS_DEFAULT_PATH ; +static char tracefs_mountpoint[PATH_MAX +1 ] =TRACEFS_DEFAULT_PATH ;static const char *const tracefs_known_mountpoints[] = {TRACEFS_DEFAULT_PATH ,
tools/lib/api/fs/tracefs.h +0 -2
@@ -16,6 +16,4 @@ const char *tracefs_find_mountpoint(void);int tracefs_valid_mountpoint (const char *debugfs);char *tracefs_mount (const char *mountpoint); -extern char tracefs_mountpoint[]; -#endif /* __API_DEBUGFS_H__ */
Signature · 1 line
2.4.3
Jiri Olsa
reviewerPATCH 9/11 perf tools: Propagate error info for the tracepoint parsing
In reply to Jiri OlsaPass 'struct parse_events_error *error' to the parse-event.c tracepoint adding path. It will be filled with error data in following patches.
--- tools/perf/util/parse-events.c | 27 ++++++++++++++++----------- tools/perf/util/parse-events.h | 3 ++- tools/perf/util/parse-events.y | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-)
tools/perf/util/parse-events.c +16 -11
@@ -387,7 +387,8 @@ int parse_events_add_cache(struct list_head *list, int *idx, }static int add_tracepoint (struct list_head *list,int *idx, -char *sys_name,char *evt_name) +char *sys_name,char *evt_name, +struct parse_events_error *error__maybe_unused ) {struct perf_evsel *evsel;
@@ -401,7 +402,8 @@ static int add_tracepoint(struct list_head *list, int *idx, }static int add_tracepoint_multi_event (struct list_head *list,int *idx, -char *sys_name,char *evt_name) +char *sys_name,char *evt_name, +struct parse_events_error *error) {char evt_path[MAXPATHLEN ];struct dirent *evt_ent;
@@ -425,7 +427,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,if (!strglobmatch (evt_ent->d_name, evt_name))continue ; - ret =add_tracepoint (list, idx, sys_name, evt_ent->d_name); + ret =add_tracepoint (list, idx, sys_name, evt_ent->d_name, error); }closedir (evt_dir);
@@ -433,15 +435,17 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx, }static int add_tracepoint_event (struct list_head *list,int *idx, -char *sys_name,char *evt_name) +char *sys_name,char *evt_name, +struct parse_events_error *error) {return strpbrk (evt_name,"*?" ) ? -add_tracepoint_multi_event (list, idx, sys_name, evt_name) : -add_tracepoint (list, idx, sys_name, evt_name); +add_tracepoint_multi_event (list, idx, sys_name, evt_name, error) : +add_tracepoint (list, idx, sys_name, evt_name, error); }static int add_tracepoint_multi_sys (struct list_head *list,int *idx, -char *sys_name,char *evt_name) +char *sys_name,char *evt_name, +struct parse_events_error *error) {struct dirent *events_ent;DIR *events_dir;
@@ -465,7 +469,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,continue ; ret =add_tracepoint_event (list, idx, events_ent->d_name, - evt_name); + evt_name, error); }closedir (events_dir);
@@ -473,12 +477,13 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx, }int parse_events_add_tracepoint (struct list_head *list,int *idx, -char *sys,char *event) +char *sys,char *event, +struct parse_events_error *error) {if (strpbrk (sys,"*?" )) -return add_tracepoint_multi_sys (list, idx, sys, event); +return add_tracepoint_multi_sys (list, idx, sys, event, error);else -return add_tracepoint_event (list, idx, sys, event); +return add_tracepoint_event (list, idx, sys, event, error); } static int
tools/perf/util/parse-events.h +2 -1
@@ -118,7 +118,8 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add);int parse_events__modifier_group (struct list_head *list,char *event_mod);int parse_events_name (struct list_head *list,char *name);int parse_events_add_tracepoint (struct list_head *list,int *idx, -char *sys,char *event); +char *sys,char *event, +struct parse_events_error *error); int parse_events_add_numeric(struct parse_events_evlist *data, struct list_head *list, u32 type, u64 config,
tools/perf/util/parse-events.y +2 -2
@@ -376,7 +376,7 @@ PE_NAME '-' PE_NAME ':' PE_NAME snprintf(&sys_name, 128, "%s-%s", $1, $3); ALLOC_LIST(list); - ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5)); + ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error)); $$ = list; } |
@@ -386,7 +386,7 @@ PE_NAME ':' PE_NAME struct list_head *list; ALLOC_LIST(list); - if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) { + if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) { struct parse_events_error *error = data->error; if (error) {
Signature · 1 line
2.4.3
Re: [PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing
Quoted 11 lines · Jiri Olsa
Pass 'struct
parse_events_error*error' to theparse-event.ctracepoint adding path. It will be filled with error data in following patches.Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/util/parse-events.c | 27 ++++++++++++++++----------- tools/perf/util/parse-events.h | 3 ++- tools/perf/util/parse-events.y | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-)
Is there a reason you decided to go ahead with passing 'data->error' directly to the parsing functions instead of adding some global state for recording errors, like a globally accessible 'parse_events_error'?
Because I notice that if someone wanted to improve the breakpoint parsing code, they'd basically need to make the same changes you've made in this patch.
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Re: [PATCH 09/11] perf tools: Propagate error info for the tracepoint parsing
Quoted 13 lines · Matt Fleming
Quoted 11 lines · Jiri Olsa
Pass 'struct
parse_events_error*error' to theparse-event.ctracepoint adding path. It will be filled with error data in following patches.Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/util/parse-events.c | 27 ++++++++++++++++----------- tools/perf/util/parse-events.h | 3 ++- tools/perf/util/parse-events.y | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-)
Quoted 7 lines
Is there a reason you decided to go ahead with passing '
data->error' directly to the parsing functions instead of adding some global state for recording errors, like a globally accessible 'parse_events_error'?Because I notice that if someone wanted to improve the breakpoint parsing code, they'd basically need to make the same changes you've made in this patch.
well, I dont see much benefit other than loosing one extra arg and worrying about concurent access in the future.. but I might have missed some
jirka
Jiri Olsa
reviewerPATCH 10/11 perf tools: Propagate error info from tp_format
In reply to Jiri OlsaPropagate error info from tp_format via ERR_PTR to get it all the way down to the parse-event.c tracepoint adding routines.
--- tools/perf/util/evsel.c | 8 ++++++-- tools/perf/util/parse-events.c | 3 ++- tools/perf/util/trace-event.c | 7 +++++-- 3 files changed, 13 insertions(+), 5 deletions(-)
tools/perf/util/evsel.c +6 -2
@@ -13,6 +13,7 @@#include <traceevent/event-parse.h> #include <linux/hw_breakpoint.h> #include <linux/perf_event.h> +#include <linux/err.h> #include <sys/resource.h> #include "asm/bug.h" #include "callchain.h"
@@ -227,6 +228,7 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)struct perf_evsel *perf_evsel__newtp_idx (const char *sys,const char *name,int idx) {struct perf_evsel *evsel =zalloc (perf_evsel__object.size); +int err = -ENOMEM ;if (evsel !=NULL ) { struct perf_event_attr attr = {
@@ -239,8 +241,10 @@ struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, int goto out_free; evsel->tp_format =trace_event__tp_format (sys, name); -if (evsel->tp_format ==NULL ) +if (IS_ERR (evsel->tp_format)) { + err =PTR_ERR (evsel->tp_format); goto out_free; + }event_attr_init (&attr); attr.config = evsel->tp_format->id;
@@ -253,7 +257,7 @@ struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, int out_free:zfree (&evsel->name);free (evsel); -return NULL ; +return ERR_PTR (err); }const char *perf_evsel__hw_names[PERF_COUNT_HW_MAX ] = {
tools/perf/util/parse-events.c +2 -1
@@ -1,4 +1,5 @@#include <linux/hw_breakpoint.h> +#include <linux/err.h> #include "util.h" #include "../perf.h" #include "evlist.h"
@@ -393,7 +394,7 @@ 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 (!evsel) +if (IS_ERR (evsel))return -ENOMEM ;list_add_tail (&evsel->node, list);
tools/perf/util/trace-event.c +5 -2
@@ -7,6 +7,7 @@#include <sys/stat.h> #include <fcntl.h> #include <linux/kernel.h> +#include <linux/err.h> #include <traceevent/event-parse.h> #include "trace-event.h" #include "machine.h"
@@ -73,12 +74,14 @@ tp_format(const char *sys, const char *name)char path[PATH_MAX ];size_t size;char *data; +int err;scnprintf (path,PATH_MAX ,"%s/%s/%s/format" , tracing_events_path, sys, name); -if (filename__read_str (path, &data, &size)) -return NULL ; + err =filename__read_str (path, &data, &size); +if (err) +return ERR_PTR (err);pevent_parse_format (pevent, &event, data, size, sys);
Signature · 1 line
2.4.3
Jiri Olsa
reviewerPATCH 11/11 perf tools: Enhance parsing events tracepoint error output
In reply to Jiri OlsaEnhancing 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
...
--- tools/perf/util/parse-events.c | 30 +++++++++++++++++++++++++++--- tools/perf/util/parse-events.y | 16 +++++++++------- 2 files changed, 36 insertions(+), 10 deletions(-)
tools/perf/util/parse-events.c +27 -3
@@ -387,6 +387,28 @@ 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 ]; + 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 +416,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 -ENOMEM ; + }list_add_tail (&evsel->node, list);
@@ -414,7 +438,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 ; }
@@ -454,7 +478,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 ; }
tools/perf/util/parse-events.y +9 -7
@@ -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;
Signature · 1 line
2.4.3