| Message ID | 1470072340-17748-1-git-send-email-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 1 Aug 2016 13:25:40 -0400 robert.foss@collabora.com wrote: > From: Scott James Remnant <scott@ubuntu.com> > > This patch uses TRACE_EVENT to add tracepoints for the open(), > exec() and uselib() syscalls so that ureadahead can cheaply trace > the boot sequence to determine what to read to speed up the next. > Good luck. AFAIK, Viro refuses to have tracepoints in the vfs subsystem. -- Steve > Signed-off-by: Scott James Remnant <scott@ubuntu.com> > Tested-by: Robert Foss <robert.foss@collabora.com> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > fs/exec.c | 7 ++++++- > fs/open.c | 4 ++++ > include/trace/events/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 63 insertions(+), 1 deletion(-) > create mode 100644 include/trace/events/fs.h > > diff --git a/fs/exec.c b/fs/exec.c > index 887c1c9..123b257 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -58,6 +58,8 @@ > #include <linux/compat.h> > #include <linux/vmalloc.h> > > +#include <trace/events/fs.h> > + > #include <asm/uaccess.h> > #include <asm/mmu_context.h> > #include <asm/tlb.h> > @@ -797,9 +799,12 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) > if (err) > goto exit; > > - if (name->name[0] != '\0') > + if (name->name[0] != '\0') { > fsnotify_open(file); > > + trace_open_exec(name->name); > + } > + > out: > return file; > > diff --git a/fs/open.c b/fs/open.c > index 93ae3cd..2ec0680 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -34,6 +34,9 @@ > > #include "internal.h" > > +#define CREATE_TRACE_POINTS > +#include <trace/events/fs.h> > + > int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs, > struct file *filp) > { > @@ -1020,6 +1023,7 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode) > } else { > fsnotify_open(f); > fd_install(fd, f); > + trace_do_sys_open(tmp->name, flags, mode); > } > } > putname(tmp); > diff --git a/include/trace/events/fs.h b/include/trace/events/fs.h > new file mode 100644 > index 0000000..fb634b7 > --- /dev/null > +++ b/include/trace/events/fs.h > @@ -0,0 +1,53 @@ > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM fs > + > +#if !defined(_TRACE_FS_H) || defined(TRACE_HEADER_MULTI_READ) > +#define _TRACE_FS_H > + > +#include <linux/fs.h> > +#include <linux/tracepoint.h> > + > +TRACE_EVENT(do_sys_open, > + > + TP_PROTO(const char *filename, int flags, int mode), > + > + TP_ARGS(filename, flags, mode), > + > + TP_STRUCT__entry( > + __string( filename, filename ) > + __field( int, flags ) > + __field( int, mode ) > + ), > + > + TP_fast_assign( > + __assign_str(filename, filename); > + __entry->flags = flags; > + __entry->mode = mode; > + ), > + > + TP_printk("\"%s\" %x %o", > + __get_str(filename), __entry->flags, __entry->mode) > +); > + > +TRACE_EVENT(open_exec, > + > + TP_PROTO(const char *filename), > + > + TP_ARGS(filename), > + > + TP_STRUCT__entry( > + __string( filename, filename ) > + ), > + > + TP_fast_assign( > + __assign_str(filename, filename); > + ), > + > + TP_printk("\"%s\"", > + __get_str(filename)) > +); > + > +#endif /* _TRACE_FS_H */ > + > +/* This part must be outside protection */ > +#include <trace/define_trace.h> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 01, 2016 at 01:25:40PM -0400, robert.foss@collabora.com wrote: > From: Scott James Remnant <scott@ubuntu.com> > > This patch uses TRACE_EVENT to add tracepoints for the open(), > exec() and uselib() syscalls so that ureadahead can cheaply trace > the boot sequence to determine what to read to speed up the next. NAK. No Tracepoints In VFS. Not going to happen - any tracepoint can all too easily become a cast-in-stone userland ABI. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 01, 2016 at 01:44:53PM -0400, Steven Rostedt wrote: > On Mon, 1 Aug 2016 13:25:40 -0400 > robert.foss@collabora.com wrote: > > > From: Scott James Remnant <scott@ubuntu.com> > > > > This patch uses TRACE_EVENT to add tracepoints for the open(), > > exec() and uselib() syscalls so that ureadahead can cheaply trace > > the boot sequence to determine what to read to speed up the next. > > > > Good luck. AFAIK, Viro refuses to have tracepoints in the vfs subsystem. Damn right. Somebody wants to use those as private debugging patches - sure, that's what the mechanism is good for. Just keep them in your test builds, where *you* will be responsible for keeping them working, etc. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016-08-01 02:10 PM, Al Viro wrote: > On Mon, Aug 01, 2016 at 01:25:40PM -0400, robert.foss@collabora.com wrote: >> From: Scott James Remnant <scott@ubuntu.com> >> >> This patch uses TRACE_EVENT to add tracepoints for the open(), >> exec() and uselib() syscalls so that ureadahead can cheaply trace >> the boot sequence to determine what to read to speed up the next. > > NAK. No Tracepoints In VFS. Not going to happen - any tracepoint can all too > easily become a cast-in-stone userland ABI. > Hey Al, I'm slightly unfamiliar with this territory, so please forgive my lack of knowledge of this topic. What is the negative side of having tracepoint be a permanent fixture in the VFS ABI? And how is VFS different from other subsystems in that regard? Rob. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 1 Aug 2016 15:51:54 -0400 Robert Foss <robert.foss@collabora.com> wrote: > On 2016-08-01 02:10 PM, Al Viro wrote: > > On Mon, Aug 01, 2016 at 01:25:40PM -0400, robert.foss@collabora.com wrote: > >> From: Scott James Remnant <scott@ubuntu.com> > >> > >> This patch uses TRACE_EVENT to add tracepoints for the open(), > >> exec() and uselib() syscalls so that ureadahead can cheaply trace > >> the boot sequence to determine what to read to speed up the next. > > > > NAK. No Tracepoints In VFS. Not going to happen - any tracepoint can all too > > easily become a cast-in-stone userland ABI. > > > > Hey Al, > > I'm slightly unfamiliar with this territory, so please forgive my lack > of knowledge of this topic. > > What is the negative side of having tracepoint be a permanent fixture in > the VFS ABI? Well, tracepoints are not vetted like syscalls are, but since they are an interface for userspace tools, they can become just as permanent of a fixture without the forethought of it being something that must be maintained forever. This has already bitten us a couple of times. > And how is VFS different from other subsystems in that regard? The main difference is that Al is the maintainer of VFS and he's the one that has to deal with maintaining a permanent tracepoint. He could give less about other subsystems because it ain't his problem ;-) -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Al Viro <viro@ZenIV.linux.org.uk> wrote: > On Mon, Aug 01, 2016 at 01:25:40PM -0400, robert.foss@collabora.com wrote: > > From: Scott James Remnant <scott@ubuntu.com> > > > > This patch uses TRACE_EVENT to add tracepoints for the open(), > > exec() and uselib() syscalls so that ureadahead can cheaply trace > > the boot sequence to determine what to read to speed up the next. > > NAK. No Tracepoints In VFS. Not going to happen - any tracepoint can all too > easily become a cast-in-stone userland ABI. And what's the problem with that? ABIs increase the utility of the kernel. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/exec.c b/fs/exec.c index 887c1c9..123b257 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -58,6 +58,8 @@ #include <linux/compat.h> #include <linux/vmalloc.h> +#include <trace/events/fs.h> + #include <asm/uaccess.h> #include <asm/mmu_context.h> #include <asm/tlb.h> @@ -797,9 +799,12 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) if (err) goto exit; - if (name->name[0] != '\0') + if (name->name[0] != '\0') { fsnotify_open(file); + trace_open_exec(name->name); + } + out: return file; diff --git a/fs/open.c b/fs/open.c index 93ae3cd..2ec0680 100644 --- a/fs/open.c +++ b/fs/open.c @@ -34,6 +34,9 @@ #include "internal.h" +#define CREATE_TRACE_POINTS +#include <trace/events/fs.h> + int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs, struct file *filp) { @@ -1020,6 +1023,7 @@ long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode) } else { fsnotify_open(f); fd_install(fd, f); + trace_do_sys_open(tmp->name, flags, mode); } } putname(tmp); diff --git a/include/trace/events/fs.h b/include/trace/events/fs.h new file mode 100644 index 0000000..fb634b7 --- /dev/null +++ b/include/trace/events/fs.h @@ -0,0 +1,53 @@ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM fs + +#if !defined(_TRACE_FS_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_FS_H + +#include <linux/fs.h> +#include <linux/tracepoint.h> + +TRACE_EVENT(do_sys_open, + + TP_PROTO(const char *filename, int flags, int mode), + + TP_ARGS(filename, flags, mode), + + TP_STRUCT__entry( + __string( filename, filename ) + __field( int, flags ) + __field( int, mode ) + ), + + TP_fast_assign( + __assign_str(filename, filename); + __entry->flags = flags; + __entry->mode = mode; + ), + + TP_printk("\"%s\" %x %o", + __get_str(filename), __entry->flags, __entry->mode) +); + +TRACE_EVENT(open_exec, + + TP_PROTO(const char *filename), + + TP_ARGS(filename), + + TP_STRUCT__entry( + __string( filename, filename ) + ), + + TP_fast_assign( + __assign_str(filename, filename); + ), + + TP_printk("\"%s\"", + __get_str(filename)) +); + +#endif /* _TRACE_FS_H */ + +/* This part must be outside protection */ +#include <trace/define_trace.h>