Generic Trace Setup and Control (GTSC) kernel API (2/3)
RFC 1/1 Generic Trace Setup and Control (GTSC) kernel API (2/3)
Signature · 5 lines
-- David Wilder IBM Linux Technology Center Beaverton, Oregon, USA [email protected] (503)578-3789
This patch introduces the Generic Trace Setup and Control (GTSC) API. In the kernel, GTSC provides a simple API for starting and managing data channels to user space. GTSC builds on the relay interface. The documentation for the GTSC is provided in a separate patch.
include/linux/gtsc.h | 124 ++++++++++++++++ lib/Kconfig | 9 ++ lib/Makefile | 2 + lib/gtsc.c | 385 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 520 insertions(+), 0 deletions(-)
include/linux/gtsc.h new file +124 -0
@@ -0,0 +1,124 @@ +/* +* gtsc.h - GTSC defines and function prototypes +* +* Copyright (C) 2006 IBM Inc. +* +* Tom Zanussi <[email protected]> +* Martin Hunt <[email protected]> +* David Wilder <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +* +*/ +#ifndef _LINUX_GTSC_H +#define _LINUX_GTSC_H + +#include <linux/relay.h> + +/* +* GTSC channel flags +*/ +#define GTSC_GLOBAL 0x01 +#define GTSC_FLIGHT 0x02 + +enum { + Gtsc_trace_setup =1 , + Gtsc_trace_running, + Gtsc_trace_stopped, +}; + +#define GTSC_TRACE_ROOT_NAME_SIZE 64/* Max root dir identifier */ +#define GTSC_TRACE_NAME_SIZE 64/* Max trace identifier */ + +/* +* Global root user information +*/ +struct gtsc_root { +struct list_head list; +char gtsc_name[GTSC_TRACE_ROOT_NAME_SIZE ]; +struct dentry *gtsc_root; +unsigned int gtsc_users; +}; + +/* +* Client information +*/ +struct gtsc_trace { +int trace_state; +struct dentry *state_file; +struct rchan *rchan; +struct dentry *dir; +struct dentry *dropped_file; +atomic_t dropped; +struct gtsc_root *root; +void *private_data; +unsigned int flags; +unsigned int buf_size; +unsigned int buf_nr; +}; + +static inline int gtsc_trace_running (struct gtsc_trace *gtsc) +{ +return gtsc->trace_state == Gtsc_trace_running; +} + +#if defined(CONFIG_GTSC ) + +/** +* gtsc_trace_setup: create a new gtsc trace handle +* +* @root: The root directory name in the root of the debugfs +* to place trace directories. Created as needed. +* @name: Trace directory name, created in @root +* @buf_size: size of the relay sub-buffers +* @buf_nr: number of relay sub-buffers +* @flags: Option selection (see GTSC channel flags definitions) +* default values when flags=0 are: use per-CPU buffering, +* use non-overwrite mode. See Documentation/gtsc.txt for details. +* +* returns a gtsc_trace handle or NULL, if setup failed. +*/ +extern struct gtsc_trace *gtsc_trace_setup (char *root,char *name,u32 buf_size, +u32 buf_nr,u32 flags); + +/** +* gtsc_trace_startstop: start or stop tracing. +* +* @gtsc: gtsc trace handle to start or stop. +* @start: set to 1 to start tracing set to 0 to stop. +* +* returns 0 if successful. +*/ +extern int gtsc_trace_startstop (struct gtsc_trace *gtsc,int start); + +/** +* gtsc_trace_cleanup: destroys the gtsc channel. +* +* @gtsc: gtsc trace handle to cleanup +*/ +extern void gtsc_trace_cleanup (struct gtsc_trace *gtsc); + +/** +* gtsc_timestamp: returns a time stamp. +*/ +extern unsigned long long gtsc_timestamp (void ); + +#else /* !CONFIG_GTSC */ +#define gtsc_trace_setup (root, name, buf_size, buf_nr, flags) (NULL) +#define gtsc_trace_startstop (gtsc, start) (-EINVAL) +#define gtsc_trace_cleanup (gtsc) do { } while (0) +#define gtsc_timestamp (void) (unsigned long long) (0) +#endif /* CONFIG_GTSC */ + +#endif
lib/Kconfig +9 -0
@@ -124,4 +124,13 @@ config HAS_DMA depends on !NO_DMA default y +config GTSC + bool "Generic Trace Setup and Control" + select RELAY + select DEBUG_FS + help + This option enables support for the GTSC. + + If unsure, say N. + endmenu
lib/Makefile +2 -0
@@ -62,6 +62,8 @@ obj-$(CONFIG_FAULT_INJECTION) += fault-inject.o lib-$(CONFIG_GENERIC_BUG) += bug.o +obj-$(CONFIG_GTSC) += gtsc.o + hostprogs-y := gen_crc32table clean-files := crc32table.h
lib/gtsc.c new file +385 -0
385 lines not on this page · raw message
Quoted 3 lines · David Wilder
This patch introduces the Generic Trace Setup and Control (GTSC) API. In the kernel, GTSC provides a simple API for starting and managing data channels to user space. GTSC builds on the relay interface.
My main grief is names choosing. gtsc_ prefixes are everywhere. In fact doing s/gtsc//g on this patchset would produce better patch. See below.
include/linux/gtsc.h line 33
Quoted 7 lines
include/linux/gtsc.h new file +5
-0+/* +* GTSC channel flags +*/ +#define GTSC_GLOBAL 0x01 +#define GTSC_FLIGHT 0x02
The fact that is channel flags is not deducable.
include/linux/gtsc.h line 36
Quoted 2 lines
include/linux/gtsc.h new file +2
-0+enum { + Gtsc_trace_setup =1 ,
Starting from zero seems unimportant. You check for equality anyway. Or not?
include/linux/gtsc.h line 39
Quoted 3 lines
include/linux/gtsc.h new file +3
-0+ Gtsc_trace_running, + Gtsc_trace_stopped, +};
All uppercase would be better.
Quoted 5 lines
include/linux/gtsc.h new file +5
-0+/* +* Global root user information +*/ +struct gtsc_root { +struct list_head list;
include/linux/gtsc.h line 58
Quoted 10 lines
include/linux/gtsc.h new file +10
-0+char gtsc_name[GTSC_TRACE_ROOT_NAME_SIZE ]; +struct dentry *gtsc_root; +unsigned int gtsc_users; +}; + +/* +* Client information +*/ +struct gtsc_trace { +int trace_state;
named enum, please.
Quoted 6 lines
include/linux/gtsc.h new file +6
-0+struct dentry *state_file; +struct rchan *rchan; +struct dentry *dir; +struct dentry *dropped_file; +atomic_t dropped; +struct gtsc_root *root;
include/linux/gtsc.h line 74
Quoted 10 lines
include/linux/gtsc.h new file +10
-0+void *private_data; +unsigned int flags; +unsigned int buf_size; +unsigned int buf_nr; +}; + +static inline int gtsc_trace_running (struct gtsc_trace *gtsc) +{ +return gtsc->trace_state == Gtsc_trace_running; +}
Like here. Compare to
static inline int trace_running(struct trace *trace)
{
return trace->state == TRACE_RUNNING;
}It's about traces not naming your particular project.
Quoted 1 line
+#if
defined(CONFIG_GTSC)
#ifdef CONFIG_* usually
include/linux/gtsc.h line 103
Quoted 9 lines
include/linux/gtsc.h new file +9
-0+/** +* gtsc_trace_startstop: start or stop tracing. +* +* @gtsc: gtsc trace handle to start or stop. +* @start: set to 1 to start tracing set to 0 to stop. +* +* returns 0 if successful. +*/ +extern int gtsc_trace_startstop (struct gtsc_trace *gtsc,int start);
Two functions in one.
include/linux/gtsc.h line 115
Quoted 4 lines
include/linux/gtsc.h new file +4
-0+/** +* gtsc_timestamp: returns a time stamp. +*/ +extern unsigned long long gtsc_timestamp (void );
Isn't it obvious from name that timestamp is returned?
include/linux/gtsc.h line 121
Quoted 5 lines
include/linux/gtsc.h new file +5
-0+#else /* !CONFIG_GTSC */ +#define gtsc_trace_setup (root, name, buf_size, buf_nr, flags) (NULL) +#define gtsc_trace_startstop (gtsc, start) (-EINVAL) +#define gtsc_trace_cleanup (gtsc) do { } while (0) +#define gtsc_timestamp (void) (unsigned long long) (0)
static inlines, so those "foo is unused" warnings would not appear.
include/linux/gtsc.h
Quoted 6 lines
include/linux/gtsc.h new file +6
-0+config GTSC + bool "Generic Trace Setup and Control" + select RELAY + select DEBUG_FS + help + This option enables support for the GTSC.
too small help text and trailing whitespace.
Help texts are usually indented like
Tabhelp
TabSpaceSpace[actual text]
TabSpaceSpace[more text]
Quoted 7 lines
lib/gtsc.c new file +4
-0+static ssize_t gtsc_state_write (struct file *filp,const char __user *buffer, +size_t count,loff_t *ppos) +{ +struct gtsc_trace *gtsc = filp->private_data;
lib/gtsc.c line 67
Quoted 10 lines
lib/gtsc.c new file +10
-0+char buf[16 ]; +int ret; + +memset (buf,0 ,sizeof (buf)); + +if (count >sizeof (buf)) +return -EINVAL ; + +if (copy_from_user (buf, buffer, count)) +return -EFAULT ;
This is too dangerous to leave without explicit placing of terminator. Think someone will copy it without strncmp() usage.
Quoted 10 lines
lib/gtsc.c new file +10
-0+if (strncmp (buf,"start" ,strlen ("start" )) ==0 ) { + ret =gtsc_trace_startstop (gtsc,1 ); +if (ret) +return ret; + }else if (strncmp (buffer,"stop" ,strlen ("stop" )) ==0 ) { + ret =gtsc_trace_startstop (gtsc,0 ); +if (ret) +return ret; + }else +return -EINVAL ;
lib/gtsc.c line 88
Quoted 10 lines
lib/gtsc.c new file +10
-0+ +return count; +} + + +static struct file_operations gtsc_state_fops = { + .owner =THIS_MODULE , + .open = gtsc_state_open, + .read = gtsc_state_read, + .write = gtsc_state_write,
[Tab]=[Space][method], is nicer ;-)
Quoted 22 lines
lib/gtsc.c new file +21
-0+static struct file_operations gtsc_dropped_fops = { + .owner =THIS_MODULE , + .open = gtsc_dropped_open, + .read = gtsc_dropped_read, +}; +int gtsc_trace_startstop (struct gtsc_trace *gtsc,int start) +{ +int ret = -EINVAL ; +/* +* For starting a trace, we can transition from a setup or stopped +* trace. For stopping a trace, the state must be running +*/ +if (start) { +if (gtsc->trace_state == Gtsc_trace_setup) { + ret =gtsc_trace_setup_channel (gtsc, gtsc->buf_size, + gtsc->buf_nr, + gtsc->flags); +if (ret) +return ret; + } + gtsc->trace_state = Gtsc_trace_running;
lib/gtsc.c line 369
Quoted 10 lines
lib/gtsc.c new file +10
-0+ }else { +if (gtsc->trace_state == Gtsc_trace_running) { + gtsc->trace_state = Gtsc_trace_stopped; +relay_flush (gtsc->rchan); + } + } + +return 0 ; +} +EXPORT_SYMBOL_GPL (gtsc_trace_startstop);
Can we get another user to justify this generalizing?
I also ask to remove extern from prototypes, making .h something like 20 lines long and move kernel-doc comments to *.c file.
Quoted 2 lines · Alexey Dobriyan
Can we get another user to justify this generalizing?
Systemtap will be using this. Most users of the relay interface will need to duplicate much of the functionality of GTSC so I expect others will use it when it is available.
I forgot to CC the list in my response to Alexey.
I plan to address Alexey's concerns in a couple of days (as soon as I get past the OLS push).
Quoted 5 lines · Alexey Dobriyan
Can we get another user to justify this generalizing?
Systemtap has plans to use the GTSC also.
Signature · 4 lines
David Wilder IBM Linux Technology Center Beaverton, Oregon, USA [email protected] (503)578-3789