[ltt-dev] [PATCH v3 4/4] Add ltt-trace-control module

Mathieu Desnoyers compudj at krystal.dyndns.org
Wed Nov 12 12:53:43 EST 2008


* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> ltt-trace-control is used to control ltt's trace.
> It will replace netlink-based ltt-control.
> 
> Signed-off-by: Zhao Lei <zhaolei at cn.fujitsu.com>
> ---
>  ltt/Kconfig             |    8 +
>  ltt/Makefile            |    1 +
>  ltt/ltt-trace-control.c |  784 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 793 insertions(+), 0 deletions(-)
>  create mode 100644 ltt/ltt-trace-control.c
> 
> diff --git a/ltt/Kconfig b/ltt/Kconfig
> index c63734d..341f178 100644
> --- a/ltt/Kconfig
> +++ b/ltt/Kconfig
> @@ -150,6 +150,14 @@ config LTT_NETLINK_CONTROL
>  	  If you enable this option, the Linux Trace Toolkit Netlink Controller
>  	  will be either built in the kernel or as module.
>  
> +config LTT_TRACE_CONTROL
> +	tristate "Linux Trace Toolkit Trace Controller"
> +	depends on LTT_TRACER
> +	default m
> +	help
> +	  If you enable this option, the debugfs-based Linux Trace Toolkit Trace
> +	  Controller will be either built in the kernel or as module.
> +
>  config LTT_STATEDUMP
>  	tristate "Linux Trace Toolkit State Dump"
>  	depends on LTT_TRACER
> diff --git a/ltt/Makefile b/ltt/Makefile
> index afcee2e..fcd1a12 100644
> --- a/ltt/Makefile
> +++ b/ltt/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_LTT_RELAY)			+= ltt-relay.o
>  obj-$(CONFIG_LTT_RELAY_LOCKED)		+= ltt-relay-locked.o
>  obj-$(CONFIG_LTT_RELAY_ALLOC)		+= ltt-relay-alloc.o
>  obj-$(CONFIG_LTT_NETLINK_CONTROL)	+= ltt-control.o
> +obj-$(CONFIG_LTT_TRACE_CONTROL)		+= ltt-trace-control.o
>  obj-$(CONFIG_LTT_SERIALIZE)		+= ltt-serialize.o
>  obj-$(CONFIG_LTT_STATEDUMP)		+= ltt-statedump.o
>  obj-$(CONFIG_LTT_MARKER_CONTROL)	+= ltt-marker-control.o
> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
> new file mode 100644
> index 0000000..70c3bd3
> --- /dev/null
> +++ b/ltt/ltt-trace-control.c
> @@ -0,0 +1,784 @@
> +/*
> + * LTT trace control module over debugfs.
> + *
> + * Copyright 2008 -
> + *	Zhaolei <zhaolei at cn.fujitsu.com>
> + */
> +
> +/*
> + * Todo:
> + *   Impl read operations for control file to read attributes
> + *   Create a README file in ltt control dir, for display help info
> + */
> +
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/debugfs.h>
> +#include <linux/ltt-tracer.h>
> +
> +#define LTT_CONTROL_DIR "control"
> +#define LTT_SETUP_TRACE_FILE "setup_trace"
> +#define LTT_DESTROY_TRACE_FILE "destroy_trace"
> +
> +#define LTT_WRITE_MAXLEN	(128)
> +
> +struct dentry *ltt_control_dir, *ltt_setup_trace_file, *ltt_destroy_trace_file;
> +
> +static DEFINE_MUTEX(control_lock);
> +
> +/*
> + * lookup a file/dir in parent dir.
> + * only designed to work well for debugfs.
> + * (although it maybe ok for other fs)
> + *
> + * return:
> + *	file/dir's dentry on success
> + *	NULL on failure
> + */
> +static inline struct dentry *dir_lookup(struct dentry *parent,

static should be enough here, without "inline". The compiler will inline
it anyway. It's mostly a matter of habits : generally, inline is only
used for headers. Given this is a C file, let's let the compiler decide
if it wants to inline it or not.

> +		const char *name) {

Style inconsistent with the kernel. The bracket should be one line
below.

> +	struct qstr q;
> +	struct dentry *d;
> +
> +	q.name = name;
> +	q.len = strlen(name);
> +	q.hash = full_name_hash(q.name, q.len);
> +
> +	d = d_lookup(parent, &q);
> +	if (d)
> +		dput(d);
> +
> +	return d;
> +}
> +
> +
> +static ssize_t alloc_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	char cmd[NAME_MAX];
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%s", cmd) != 1) {
> +		err = -EPERM;
> +		goto err_get_cmd;
> +	}
> +
> +	if ((cmd[0] != 'Y' && cmd[0] != 'y' && cmd[0] != '1') || cmd[1]) {
> +		err = -EPERM;
> +		goto err_bad_cmd;
> +	}
> +
> +	err = ltt_trace_alloc(file->f_dentry->d_parent->d_name.name);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR "alloc_write: ltt_trace_alloc failed: %d\n",
> +			err);
> +		goto err_alloc_trace;
> +	}
> +
> +	return count;
> +
> +err_alloc_trace:
> +err_bad_cmd:
> +err_get_cmd:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_alloc_operations = {
> +	.write = alloc_write,
> +};
> +
> +
> +static ssize_t enabled_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	char cmd[NAME_MAX];
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%s", cmd) != 1) {
> +		err = -EPERM;
> +		goto err_get_cmd;
> +	}
> +
> +	if (cmd[1]) {
> +		err = -EPERM;
> +		goto err_bad_cmd;
> +	}
> +
> +	switch (cmd[0]) {
> +	case 'Y':
> +	case 'y':
> +	case '1':
> +		err = ltt_trace_start(
> +			file->f_dentry->d_parent->d_name.name);

does it need to be on 2 lines ? (80 cols ?)

> +		/*
> +		 * ltt_trace_start return +ERRNO,
> +		 * so we can't use IS_ERR_VALUE(err)
> +		 */
> +		if (err) {
> +			printk(KERN_ERR
> +				"enabled_write: ltt_trace_start failed: %d\n",
> +				err);
> +			err = -EPERM;
> +			goto err_start_trace;
> +		}
> +		break;
> +	case 'N':
> +	case 'n':
> +	case '0':
> +		err = ltt_trace_stop(
> +			file->f_dentry->d_parent->d_name.name);

does it need to be on 2 lines ?

> +		/*
> +		 * ltt_trace_stop return +ERRNO,
> +		 * so we can't use IS_ERR_VALUE(err)
> +		 */
> +		if (err) {
> +			printk(KERN_ERR
> +				"enabled_write: ltt_trace_stop failed: %d\n",
> +				err);
> +			err = -EPERM;
> +			goto err_stop_trace;
> +		}
> +		break;
> +	default:
> +		err = -EPERM;
> +		goto err_bad_cmd;
> +	}
> +
> +	return count;
> +
> +err_stop_trace:
> +err_start_trace:
> +err_bad_cmd:
> +err_get_cmd:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_enabled_operations = {
> +	.write = enabled_write,
> +};
> +
> +
> +static ssize_t trans_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	char trans_name[NAME_MAX];
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%s", trans_name) != 1) {
> +		err = -EPERM;
> +		goto err_get_transname;
> +	}
> +
> +	err = ltt_trace_set_type(file->f_dentry->d_parent->d_name.name,
> +		trans_name);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR "trans_write: ltt_trace_set_type failed: %d\n",
> +			err);
> +		goto err_set_trans;
> +	}
> +
> +	return count;
> +
> +err_set_trans:
> +err_get_transname:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_trans_operations = {
> +	.write = trans_write,
> +};
> +
> +
> +static ssize_t subbuf_num_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	unsigned int num;
> +	const char *channel_name;
> +	const char *trace_name;
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%u", &num) != 1) {
> +		err = -EPERM;
> +		goto err_get_number;
> +	}
> +
> +	channel_name = file->f_dentry->d_parent->d_name.name;
> +	trace_name = file->f_dentry->d_parent->d_parent->d_parent->d_name.name;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, channel_name, num);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR "subbuf_num_write: "
> +			"ltt_trace_set_channel_subbufcount failed: %d\n", err);
> +		goto err_set_subbufcount;
> +	}
> +
> +	return count;
> +
> +err_set_subbufcount:
> +err_get_number:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_subbuf_num_operations = {
> +	.write = subbuf_num_write,
> +};
> +
> +
> +static ssize_t subbuf_size_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	unsigned int num;
> +	const char *channel_name;
> +	const char *trace_name;
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%u", &num) != 1) {
> +		err = -EPERM;
> +		goto err_get_number;
> +	}
> +
> +	channel_name = file->f_dentry->d_parent->d_name.name;
> +	trace_name = file->f_dentry->d_parent->d_parent->d_parent->d_name.name;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, channel_name, num);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR "subbuf_size_write: "
> +		"ltt_trace_set_channel_subbufsize failed: %d\n", err);
> +		goto err_set_subbufsize;
> +	}
> +
> +	return count;
> +
> +err_set_subbufsize:
> +err_get_number:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_subbuf_size_operations = {
> +	.write = subbuf_size_write,
> +};
> +
> +
> +static ssize_t overwrite_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	char cmd[NAME_MAX];
> +	const char *channel_name;
> +	const char *trace_name;
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%s", cmd) != 1) {
> +		err = -EPERM;
> +		goto err_get_cmd;
> +	}
> +
> +	if (cmd[1]) {
> +		err = -EPERM;
> +		goto err_bad_cmd;
> +	}
> +
> +	channel_name = file->f_dentry->d_parent->d_name.name;
> +	trace_name = file->f_dentry->d_parent->d_parent->d_parent->d_name.name;
> +
> +	switch (cmd[0]) {
> +	case 'Y':
> +	case 'y':
> +	case '1':
> +		err = ltt_trace_set_channel_overwrite(trace_name, channel_name,
> +			1);
> +		if (IS_ERR_VALUE(err)) {
> +			printk(KERN_ERR "subbuf_size_write: "
> +			"ltt_trace_set_channel_subbufsize failed: %d\n", err);
> +			goto err_set_subbufsize;
> +		}
> +		break;
> +	case 'N':
> +	case 'n':
> +	case '0':
> +		err = ltt_trace_set_channel_overwrite(trace_name, channel_name,
> +			0);
> +		if (IS_ERR_VALUE(err)) {
> +			printk(KERN_ERR "subbuf_size_write: "
> +			"ltt_trace_set_channel_subbufsize failed: %d\n", err);
> +			goto err_set_subbufsize;
> +		}
> +		break;
> +	default:
> +		err = -EPERM;
> +		goto err_bad_cmd;
> +	}
> +
> +	return count;
> +
> +err_set_subbufsize:
> +err_bad_cmd:
> +err_get_cmd:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_overwrite_operations = {
> +	.write = overwrite_write,
> +};
> +
> +
> +static ssize_t mode_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	unsigned int mode_i;
> +	enum trace_mode mode;
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%u", &mode_i) != 1) {
> +		err = -EPERM;
> +		goto err_get_mode;
> +	}
> +
> +	switch (mode_i) {
> +	case 0:
> +		mode = LTT_TRACE_NORMAL;
> +		break;
> +	case 1:
> +		mode = LTT_TRACE_FLIGHT;
> +		break;
> +	case 2:
> +		mode = LTT_TRACE_HYBRID;
> +		break;
> +	default:
> +		err = -EPERM;
> +		goto err_get_mode;
> +	};
> +
> +	err = ltt_trace_set_mode(
> +		file->f_dentry->d_parent->d_parent->d_parent->d_name.name,
> +		mode);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR "mode_write: ltt_trace_set_mode failed: %d\n",
> +			err);
> +		goto err_set_mode;
> +	}
> +
> +	return count;
> +
> +err_set_mode:
> +err_get_mode:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_mode_operations = {
> +	.write = mode_write,
> +};
> +
> +
> +static const char *channels[] = {
> +	"cpu",
> +	"processes",
> +	"interrupts",
> +	"network",
> +	"modules",
> +	"metadata"
> +};
> +static int _create_trace_control_dir(const char *trace_name)
> +{
> +	int err;
> +	struct dentry *trace_root, *channel_root;
> +	struct dentry *tmp_den;
> +	int i;
> +
> +	/*
> +	 * for create channels' dir.
> +	 * Todo:
> +	 *   get channel_names from exported lttng variable.
> +	 */
> +
> +
> +	/* debugfs/control/trace_name */
> +	trace_root = debugfs_create_dir(trace_name, ltt_control_dir);
> +	if (IS_ERR(trace_root) || !trace_root) {
> +		printk(KERN_ERR "_create_trace_control_dir: "
> +			"create control root dir of %s failed\n", trace_name);
> +		err = -ENOMEM;
> +		goto err_create_trace_root;
> +	}
> +
> +	/* debugfs/control/trace_name/alloc */
> +	tmp_den = debugfs_create_file("alloc", S_IWUSR, trace_root, NULL,
> +		&ltt_alloc_operations);
> +	if (IS_ERR(tmp_den) || !tmp_den) {
> +		printk(KERN_ERR "_create_trace_control_dir: "
> +			"create file of alloc failed\n");
> +		err = -ENOMEM;
> +		goto err_create_subdir;
> +	}
> +
> +	/* debugfs/control/trace_name/trans */
> +	tmp_den = debugfs_create_file("trans", S_IWUSR, trace_root, NULL,
> +		&ltt_trans_operations);
> +	if (IS_ERR(tmp_den) || !tmp_den) {
> +		printk(KERN_ERR "_create_trace_control_dir: "
> +			"create file of trans failed\n");
> +		err = -ENOMEM;
> +		goto err_create_subdir;
> +	}
> +
> +	/* debugfs/control/trace_name/enabled */
> +	tmp_den = debugfs_create_file("enabled", S_IWUSR, trace_root, NULL,
> +		&ltt_enabled_operations);
> +	if (IS_ERR(tmp_den) || !tmp_den) {
> +		printk(KERN_ERR "_create_trace_control_dir: "
> +			"create file of enabled failed\n");
> +		err = -ENOMEM;
> +		goto err_create_subdir;
> +	}
> +
> +	/* debugfs/control/trace_name/channel/ */
> +	channel_root = debugfs_create_dir("channel", trace_root);
> +	if (IS_ERR(channel_root) || !channel_root) {
> +		printk(KERN_ERR "_create_trace_control_dir: "
> +			"create dir of channel failed\n");
> +		err = -ENOMEM;
> +		goto err_create_subdir;
> +	}
> +
> +	/*
> +	 * Create dir and files in debugfs/ltt/control/trace_name/channel/
> +	 * Following things(without <>) will be created:
> +	 * `-- <control>
> +	 *     `-- <trace_name>
> +	 *         `-- <channel>
> +	 *             |-- interrupts
> +	 *             |   |-- subbuf_num
> +	 *             |   `-- subbuf_size
> +	 *             |-- metadata
> +	 *             |   |-- subbuf_num
> +	 *             |   `-- subbuf_size
> +	 *             |-- modules
> +	 *             |   |-- subbuf_num
> +	 *             |   `-- subbuf_size
> +	 *             |-- network
> +	 *             |   |-- subbuf_num
> +	 *             |   `-- subbuf_size
> +	 *             |-- processes
> +	 *             |   |-- subbuf_num
> +	 *             |   `-- subbuf_size
> +	 *             `-- cpu
> +	 *                 |-- mode
> +	 *                 |-- subbuf_num
> +	 *                 `-- subbuf_size
> +	 */
> +
> +	for (i = 0; i < sizeof(channels)/sizeof(channels[0]); i++) {
> +		struct dentry *channel_den;
> +
> +		channel_den = debugfs_create_dir(channels[i], channel_root);
> +		if (IS_ERR(channel_den) || !channel_den) {
> +			printk(KERN_ERR "_create_trace_control_dir: "
> +				"create channel dir of %s failed\n",
> +				channels[i]);
> +			err = -ENOMEM;
> +			goto err_create_subdir;
> +		}
> +
> +		tmp_den = debugfs_create_file("subbuf_num", S_IWUSR,
> +			channel_den, NULL, &ltt_subbuf_num_operations);
> +		if (IS_ERR(tmp_den) || !tmp_den) {
> +			printk(KERN_ERR "_create_trace_control_dir: "
> +				"create subbuf_num in %s failed\n",
> +				channels[i]);
> +			err = -ENOMEM;
> +			goto err_create_subdir;
> +		}
> +
> +		tmp_den = debugfs_create_file("subbuf_size", S_IWUSR,
> +			channel_den, NULL, &ltt_subbuf_size_operations);
> +		if (IS_ERR(tmp_den) || !tmp_den) {
> +			printk(KERN_ERR "_create_trace_control_dir: "
> +				"create subbuf_size in %s failed\n",
> +				channels[i]);
> +			err = -ENOMEM;
> +			goto err_create_subdir;
> +		}
> +
> +		tmp_den = debugfs_create_file("overwrite", S_IWUSR, channel_den,
> +			NULL, &ltt_overwrite_operations);
> +		if (IS_ERR(tmp_den) || !tmp_den) {
> +			printk(KERN_ERR "_create_trace_control_dir: "
> +				"create overwrite in %s failed\n", channels[i]);
> +			err = -ENOMEM;
> +			goto err_create_subdir;
> +		}
> +
> +		if (!i) {

Hrm ? What is special about i == 0 ?

> +			/* debugfs/control/trace_name/channel/cpu/mode */
> +			tmp_den = debugfs_create_file("mode", S_IWUSR,
> +				channel_den, NULL, &ltt_mode_operations);
> +			if (IS_ERR(tmp_den) || !tmp_den) {
> +				printk(KERN_ERR "_create_trace_control_dir: "
> +					"create mode in %s failed\n",
> +					channels[i]);
> +				err = -ENOMEM;
> +				goto err_create_subdir;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +
> +err_create_subdir:
> +	debugfs_remove_recursive(trace_root);
> +err_create_trace_root:
> +	return err;
> +}
> +
> +static ssize_t setup_trace_write(struct file *file, const char __user *user_buf,
> +		size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	char trace_name[NAME_MAX];
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%s", trace_name) != 1) {
> +		err = -EPERM;
> +		goto err_get_tracename;
> +	}
> +
> +	mutex_lock(&control_lock);
> +
> +	err = ltt_trace_setup(trace_name);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR
> +			"setup_trace_write: ltt_trace_setup failed: %d\n", err);
> +		goto err_setup_trace;
> +	}
> +
> +	err = _create_trace_control_dir(trace_name);
> +	if (IS_ERR_VALUE(err)) {
> +		printk(KERN_ERR "setup_trace_write: "
> +			"_create_trace_control_dir failed: %d\n", err);
> +		goto err_create_trace_control_dir;
> +	}
> +
> +	mutex_unlock(&control_lock);
> +
> +	return count;
> +
> +err_create_trace_control_dir:
> +	ltt_trace_destroy(trace_name);
> +err_setup_trace:
> +	mutex_unlock(&control_lock);
> +err_get_tracename:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_setup_trace_operations = {
> +	.write = setup_trace_write,
> +};
> +
> +
> +static ssize_t destroy_trace_write(struct file *file,
> +		const char __user *user_buf, size_t count, loff_t *ppos)
> +{
> +	int err = 0;
> +	char buf[NAME_MAX];
> +	int buf_size;
> +	char trace_name[NAME_MAX];
> +	struct dentry *trace_den;
> +
> +
> +	buf_size = min(count, sizeof(buf) - 1);
> +	err = copy_from_user(buf, user_buf, buf_size);
> +	if (err)
> +		goto err_copy_from_user;
> +	buf[buf_size] = 0;
> +
> +	if (sscanf(buf, "%s", trace_name) != 1) {
> +		err = -EPERM;
> +		goto err_get_tracename;
> +	}
> +
> +	mutex_lock(&control_lock);
> +
> +	err = ltt_trace_destroy(trace_name);
> +	/*
> +	 * ltt_trace_destroy return +ERRNO,
> +	 * so we can't use IS_ERR_VALUE(err)

ltt_trace_destroy returns a int, not a pointer to some variable which
can have to take negative value to be an error. In this case, just
checking for non-zero return value is OK. Do we really need to leave
this comment here ?

> +	 */
> +	if (err) {
> +		printk(KERN_ERR
> +			"destroy_trace_write: ltt_trace_destroy failed: %d\n",
> +			err);
> +		err = -EPERM;
> +		goto err_destroy_trace;
> +	}
> +
> +	trace_den = dir_lookup(ltt_control_dir, trace_name);
> +	if (!trace_den) {
> +		printk(KERN_ERR
> +			"destroy_trace_write: lookup for %s's dentry failed\n",
> +			trace_name);
> +		err = -ENOENT;
> +		goto err_get_dentry;
> +	}
> +
> +	debugfs_remove_recursive(trace_den);
> +
> +	mutex_unlock(&control_lock);
> +
> +	return count;
> +
> +err_get_dentry:
> +err_destroy_trace:
> +	mutex_unlock(&control_lock);
> +err_get_tracename:
> +err_copy_from_user:
> +	return err;
> +}
> +
> +static struct file_operations ltt_destroy_trace_operations = {
> +	.write = destroy_trace_write,
> +};
> +
> +
> +static int __init ltt_trace_control_init(void)
> +{
> +	int err = 0;
> +	struct dentry *ltt_root_dentry;
> +
> +	ltt_root_dentry = get_ltt_root();
> +	if (!ltt_root_dentry) {
> +		err = -ENOENT;
> +		goto err_no_root;
> +	}
> +
> +	ltt_control_dir = debugfs_create_dir(LTT_CONTROL_DIR, ltt_root_dentry);
> +	if (IS_ERR(ltt_control_dir) || !ltt_control_dir) {
> +		printk(KERN_ERR
> +			"ltt_channel_control_init: create dir of %s failed\n",
> +			LTT_CONTROL_DIR);
> +		err = -ENOMEM;
> +		goto err_create_control_dir;
> +	}
> +
> +	ltt_setup_trace_file = debugfs_create_file(LTT_SETUP_TRACE_FILE,
> +		S_IWUSR, ltt_root_dentry, NULL, &ltt_setup_trace_operations);
> +	if (IS_ERR(ltt_setup_trace_file) || !ltt_setup_trace_file) {
> +		printk(KERN_ERR
> +			"ltt_channel_control_init: create file of %s failed\n",
> +			LTT_SETUP_TRACE_FILE);
> +		err = -ENOMEM;
> +		goto err_create_setup_trace_file;
> +	}
> +
> +	ltt_destroy_trace_file = debugfs_create_file(LTT_DESTROY_TRACE_FILE,
> +		S_IWUSR, ltt_root_dentry, NULL, &ltt_destroy_trace_operations);
> +	if (IS_ERR(ltt_destroy_trace_file) || !ltt_destroy_trace_file) {
> +		printk(KERN_ERR
> +			"ltt_channel_control_init: create file of %s failed\n",
> +			LTT_DESTROY_TRACE_FILE);
> +		err = -ENOMEM;
> +		goto err_create_destroy_trace_file;
> +	}
> +
> +	return 0;
> +
> +err_create_destroy_trace_file:
> +	debugfs_remove(ltt_setup_trace_file);
> +err_create_setup_trace_file:
> +	debugfs_remove(ltt_control_dir);
> +err_create_control_dir:
> +err_no_root:
> +	return err;
> +}
> +
> +static void __exit ltt_trace_control_exit(void)
> +{
> +	struct dentry *trace_dir;
> +
> +	/* destory all traces */
> +	list_for_each_entry(trace_dir, &ltt_control_dir->d_subdirs,
> +		d_u.d_child) {
> +		ltt_trace_stop(trace_dir->d_name.name);
> +		ltt_trace_destroy(trace_dir->d_name.name);
> +	}
> +
> +	/* clean dirs in debugfs */
> +	debugfs_remove(ltt_setup_trace_file);
> +	debugfs_remove(ltt_destroy_trace_file);
> +	debugfs_remove_recursive(ltt_control_dir);
> +}
> +
> +module_init(ltt_trace_control_init);
> +module_exit(ltt_trace_control_exit);
> +
> +MODULE_LICENSE("GPL");

You could add :

MODULE_AUTHOR(...);
MODULE_DESCRIPTION(...);

Thanks,

Mathieu

> -- 
> 1.5.5.3
> 
> 
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68




More information about the lttng-dev mailing list