Messages in this thread | | | Date | Fri, 25 Jan 2013 11:14:55 -0500 | From | Mathieu Desnoyers <> | Subject | Re: [tracepoint] cargo-culting considered harmful... |
| |
* Steven Rostedt (rostedt@goodmis.org) wrote: > On Fri, 2013-01-25 at 09:38 -0500, Mathieu Desnoyers wrote: > > > Yep, I'd be OK with removing this example, since now all users are > > expected to user TRACE_EVENT(), which is built on top of tracepoints. > > Can I get your Acked-by for the following patch?
Sure,
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Thanks!
Mathieu
> > -- Steve > > commit 867a31fab0a064e54147371425f9fdef933e3d1d > Author: Steven Rostedt <srostedt@redhat.com> > Date: Fri Jan 25 09:46:36 2013 -0500 > > tracing: Remove tracepoint sample code > > The tracepoint sample code was used to teach developers how to > create their own tracepoints. But now the trace_events have been > added as a higher level that is used directly by developers today. > > Only the trace_event code should use the tracepoint interface > directly and no new tracepoints should be added. > > Besides, the example had a race condition with the use of the > ->d_name.name dentry field, as pointed out by Al Viro. > > Best just to remove the code so it wont be used by other developers. > > Cc: Al Viro <viro@ZenIV.linux.org.uk> > Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > diff --git a/samples/Kconfig b/samples/Kconfig > index 7b6792a..6181c2c 100644 > --- a/samples/Kconfig > +++ b/samples/Kconfig > @@ -5,12 +5,6 @@ menuconfig SAMPLES > > if SAMPLES > > -config SAMPLE_TRACEPOINTS > - tristate "Build tracepoints examples -- loadable modules only" > - depends on TRACEPOINTS && m > - help > - This build tracepoints example modules. > - > config SAMPLE_TRACE_EVENTS > tristate "Build trace_events examples -- loadable modules only" > depends on EVENT_TRACING && m > diff --git a/samples/Makefile b/samples/Makefile > index 5ef08bb..1a60c62 100644 > --- a/samples/Makefile > +++ b/samples/Makefile > @@ -1,4 +1,4 @@ > # Makefile for Linux samples code > > -obj-$(CONFIG_SAMPLES) += kobject/ kprobes/ tracepoints/ trace_events/ \ > +obj-$(CONFIG_SAMPLES) += kobject/ kprobes/ trace_events/ \ > hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ > diff --git a/samples/tracepoints/Makefile b/samples/tracepoints/Makefile > deleted file mode 100644 > index 36479ad..0000000 > --- a/samples/tracepoints/Makefile > +++ /dev/null > @@ -1,6 +0,0 @@ > -# builds the tracepoint example kernel modules; > -# then to use one (as root): insmod <module_name.ko> > - > -obj-$(CONFIG_SAMPLE_TRACEPOINTS) += tracepoint-sample.o > -obj-$(CONFIG_SAMPLE_TRACEPOINTS) += tracepoint-probe-sample.o > -obj-$(CONFIG_SAMPLE_TRACEPOINTS) += tracepoint-probe-sample2.o > diff --git a/samples/tracepoints/tp-samples-trace.h b/samples/tracepoints/tp-samples-trace.h > deleted file mode 100644 > index 4d46be9..0000000 > --- a/samples/tracepoints/tp-samples-trace.h > +++ /dev/null > @@ -1,11 +0,0 @@ > -#ifndef _TP_SAMPLES_TRACE_H > -#define _TP_SAMPLES_TRACE_H > - > -#include <linux/proc_fs.h> /* for struct inode and struct file */ > -#include <linux/tracepoint.h> > - > -DECLARE_TRACE(subsys_event, > - TP_PROTO(struct inode *inode, struct file *file), > - TP_ARGS(inode, file)); > -DECLARE_TRACE_NOARGS(subsys_eventb); > -#endif > diff --git a/samples/tracepoints/tracepoint-probe-sample.c b/samples/tracepoints/tracepoint-probe-sample.c > deleted file mode 100644 > index 744c0b9..0000000 > --- a/samples/tracepoints/tracepoint-probe-sample.c > +++ /dev/null > @@ -1,57 +0,0 @@ > -/* > - * tracepoint-probe-sample.c > - * > - * sample tracepoint probes. > - */ > - > -#include <linux/module.h> > -#include <linux/file.h> > -#include <linux/dcache.h> > -#include "tp-samples-trace.h" > - > -/* > - * Here the caller only guarantees locking for struct file and struct inode. > - * Locking must therefore be done in the probe to use the dentry. > - */ > -static void probe_subsys_event(void *ignore, > - struct inode *inode, struct file *file) > -{ > - path_get(&file->f_path); > - dget(file->f_path.dentry); > - printk(KERN_INFO "Event is encountered with filename %s\n", > - file->f_path.dentry->d_name.name); > - dput(file->f_path.dentry); > - path_put(&file->f_path); > -} > - > -static void probe_subsys_eventb(void *ignore) > -{ > - printk(KERN_INFO "Event B is encountered\n"); > -} > - > -static int __init tp_sample_trace_init(void) > -{ > - int ret; > - > - ret = register_trace_subsys_event(probe_subsys_event, NULL); > - WARN_ON(ret); > - ret = register_trace_subsys_eventb(probe_subsys_eventb, NULL); > - WARN_ON(ret); > - > - return 0; > -} > - > -module_init(tp_sample_trace_init); > - > -static void __exit tp_sample_trace_exit(void) > -{ > - unregister_trace_subsys_eventb(probe_subsys_eventb, NULL); > - unregister_trace_subsys_event(probe_subsys_event, NULL); > - tracepoint_synchronize_unregister(); > -} > - > -module_exit(tp_sample_trace_exit); > - > -MODULE_LICENSE("GPL"); > -MODULE_AUTHOR("Mathieu Desnoyers"); > -MODULE_DESCRIPTION("Tracepoint Probes Samples"); > diff --git a/samples/tracepoints/tracepoint-probe-sample2.c b/samples/tracepoints/tracepoint-probe-sample2.c > deleted file mode 100644 > index 9fcf990..0000000 > --- a/samples/tracepoints/tracepoint-probe-sample2.c > +++ /dev/null > @@ -1,44 +0,0 @@ > -/* > - * tracepoint-probe-sample2.c > - * > - * 2nd sample tracepoint probes. > - */ > - > -#include <linux/module.h> > -#include <linux/fs.h> > -#include "tp-samples-trace.h" > - > -/* > - * Here the caller only guarantees locking for struct file and struct inode. > - * Locking must therefore be done in the probe to use the dentry. > - */ > -static void probe_subsys_event(void *ignore, > - struct inode *inode, struct file *file) > -{ > - printk(KERN_INFO "Event is encountered with inode number %lu\n", > - inode->i_ino); > -} > - > -static int __init tp_sample_trace_init(void) > -{ > - int ret; > - > - ret = register_trace_subsys_event(probe_subsys_event, NULL); > - WARN_ON(ret); > - > - return 0; > -} > - > -module_init(tp_sample_trace_init); > - > -static void __exit tp_sample_trace_exit(void) > -{ > - unregister_trace_subsys_event(probe_subsys_event, NULL); > - tracepoint_synchronize_unregister(); > -} > - > -module_exit(tp_sample_trace_exit); > - > -MODULE_LICENSE("GPL"); > -MODULE_AUTHOR("Mathieu Desnoyers"); > -MODULE_DESCRIPTION("Tracepoint Probes Samples"); > diff --git a/samples/tracepoints/tracepoint-sample.c b/samples/tracepoints/tracepoint-sample.c > deleted file mode 100644 > index f4d89e0..0000000 > --- a/samples/tracepoints/tracepoint-sample.c > +++ /dev/null > @@ -1,57 +0,0 @@ > -/* tracepoint-sample.c > - * > - * Executes a tracepoint when /proc/tracepoint-sample is opened. > - * > - * (C) Copyright 2007 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> > - * > - * This file is released under the GPLv2. > - * See the file COPYING for more details. > - */ > - > -#include <linux/module.h> > -#include <linux/sched.h> > -#include <linux/proc_fs.h> > -#include "tp-samples-trace.h" > - > -DEFINE_TRACE(subsys_event); > -DEFINE_TRACE(subsys_eventb); > - > -struct proc_dir_entry *pentry_sample; > - > -static int my_open(struct inode *inode, struct file *file) > -{ > - int i; > - > - trace_subsys_event(inode, file); > - for (i = 0; i < 10; i++) > - trace_subsys_eventb(); > - return -EPERM; > -} > - > -static const struct file_operations mark_ops = { > - .open = my_open, > - .llseek = noop_llseek, > -}; > - > -static int __init sample_init(void) > -{ > - printk(KERN_ALERT "sample init\n"); > - pentry_sample = proc_create("tracepoint-sample", 0444, NULL, > - &mark_ops); > - if (!pentry_sample) > - return -EPERM; > - return 0; > -} > - > -static void __exit sample_exit(void) > -{ > - printk(KERN_ALERT "sample exit\n"); > - remove_proc_entry("tracepoint-sample", NULL); > -} > - > -module_init(sample_init) > -module_exit(sample_exit) > - > -MODULE_LICENSE("GPL"); > -MODULE_AUTHOR("Mathieu Desnoyers"); > -MODULE_DESCRIPTION("Tracepoint sample"); > > >
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |