[ltt-dev] [PATCH] LTTng: Make mark-control work in debugfs

[compudj@krystal.dyndns.org (Mathieu Desnoyers)]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> >> Hi Mathieu,
> >>
> >> This patch makes mark-control work in debugfs
> >> Control files are organized as following.
> >> ltt/
> >> |-- markers
> >> |   |-- fs
> >> |   |   |-- buffer_wait_end
> >> |   |   |   |-- enabled
> >> |   |   |   `-- info
> >> |   |   |-- buffer_wait_start
> >> |   |   |   |-- enabled
> >> |   |   |   `-- info
> >> |   |   |-- close
> >> |   |   |   |-- enabled
> >> |   |   |   `-- info
> >> |   |   |-- exec
> >> |   |   |   |-- enabled
> >> |   |   |   `-- info
> >> ...
> >>
> >> One can enable or disable a marker by using the following command
> >> # echo 1 > ltt/markers/fs/close/enabled
> >> # echo 0 > ltt/markers/fs/close/enabled
> >>
> >> Show the marker information as following 
> >> # cat ltt/markers/fs/close/info
> >> format: "fd %u"
> >> state: 1
> >> event_id: 2
> >> call: 0xc0459118
> >> probe single : 0xc04f7b08
> >>
> > 
> > Thanks a lot Gui, this is a very good patch. I'll make the cosmetic
> > changes requested by scripts/checkpatch.pl myself and merge it. (please
> 
>   I see, thanks for pointing this out. :)
> 
>   BTW, I'll post a patch to modify user tools accordingly as soon as this patch
>   has been merged into the new version of LTTng.
> 

Hrm, I'm playing a bit with the interface, and I think I should wait
before I merge it. There are a few things to fix.

First, when enabling a marker with the old ltt-marker-control.ko, if we
rmmod the module containing this marker and after modprobe the module
again, the marker stays enabled. With this debugfs interface, it seems
to become disabled.

Also, when I connect to a marker, there is some module refcounting
involved to make sure the marked modules stays loaded. For some reason
I've been able to unload a module with armed markers.. this should
probably be looked at.

Thirdly, when I do

cd /mnt/debugfs/ltt/markers
for a in */*/enable; do echo $a; echo 1 > $a; done

I get :
metadata/core_marker_format
-su: echo: write error: File exists
metadata/core_marker_id
-su: echo: write error: File exists

Which is expected, because those marker are activated within the kernel
and should not have their state modified by the user-visible API.

Then I do :
for a in */*/enable; do echo $a; echo 0 > $a; done

And do the first command again :
for a in */*/enable; do echo $a; echo 1 > $a; done

This shows that userspace is able to alter the state of builtin "core"
markers, which I disallow in ltt-marker-control.ko. The same would be
good in the debugfs interface.

I'm providing the patch modified to fit with checkpatch below.

Thanks,

Mathieu


lttng-make-mark-control-work-in-debugfs

This patch makes mark-control work in debugfs
Control files are organized as following.
ltt/
|-- markers
|   |-- fs
|   |   |-- buffer_wait_end
|   |   |   |-- enable
|   |   |   `-- info
|   |   |-- buffer_wait_start
|   |   |   |-- enable
|   |   |   `-- info
|   |   |-- close
|   |   |   |-- enable
|   |   |   `-- info
|   |   |-- exec
|   |   |   |-- enable
|   |   |   `-- info
...

One can enable or disable a marker by using the following command
# echo 1 > ltt/markers/fs/close/enable
# echo 0 > ltt/markers/fs/close/enable

Show the marker information as following 
# cat ltt/markers/fs/close/info
format: "fd %u"
state: 1
event_id: 2
call: 0xc0459118
probe single : 0xc04f7b08

Mathieu :

enabled -> enable to follow the semantic of the rest of the debugfs API.
checkpatch.pl cleanup.

Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at polymtl.ca>
---
 ltt/ltt-trace-control.c |  298 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 294 insertions(+), 4 deletions(-)

Index: linux-2.6-lttng/ltt/ltt-trace-control.c
===================================================================
--- linux-2.6-lttng.orig/ltt/ltt-trace-control.c	2009-02-04 00:56:05.000000000 -0500
+++ linux-2.6-lttng/ltt/ltt-trace-control.c	2009-02-04 01:01:46.000000000 -0500
@@ -1,8 +1,10 @@
 /*
  * LTT trace control module over debugfs.
  *
- * Copyright 2008 -
- *	Zhaolei <zhaolei at cn.fujitsu.com>
+ * Copyright 2008 - Zhaolei <zhaolei at cn.fujitsu.com>
+ *
+ * Copyright 2009 - Gui Jianfeng <guijianfeng at cn.fujitsu.com>
+ *                  Make mark-control work in debugfs
  */
 
 /*
@@ -16,14 +18,17 @@
 #include <linux/uaccess.h>
 #include <linux/debugfs.h>
 #include <linux/ltt-tracer.h>
+#include <linux/notifier.h>
 
 #define LTT_CONTROL_DIR "control"
+#define MARKERS_CONTROL_DIR "markers"
 #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;
+struct dentry *ltt_control_dir, *ltt_setup_trace_file, *ltt_destroy_trace_file,
+	*markers_control_dir;
 
 /*
  * the traces_lock nests inside control_lock.
@@ -692,6 +697,270 @@ static struct file_operations ltt_destro
 	.write = destroy_trace_write,
 };
 
+static int marker_enable_open(struct inode *inode, struct file *filp)
+{
+	filp->private_data = inode->i_private;
+	return 0;
+}
+
+static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
+			    size_t cnt, loff_t *ppos)
+{
+	struct marker *marker;
+	char *buf;
+	int len;
+
+	marker = (struct marker *)filp->private_data;
+	buf = kmalloc(1024, GFP_KERNEL);
+
+	len = sprintf(buf, "%d\n", _imv_read(marker->state));
+
+	len = simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
+	kfree(buf);
+
+	return len;
+}
+
+static ssize_t marker_enable_write(struct file *filp, const char __user *ubuf,
+				size_t cnt, loff_t *ppos)
+{
+	char buf[NAME_MAX];
+	int buf_size;
+	int err = 0;
+	struct marker *marker;
+
+	marker = (struct marker *)filp->private_data;
+	buf_size = min(cnt, sizeof(buf) - 1);
+	err = copy_from_user(buf, ubuf, buf_size);
+	if (err)
+		return err;
+
+	buf[buf_size] = 0;
+
+	switch (buf[0]) {
+	case 'Y':
+	case 'y':
+	case '1':
+		err = ltt_marker_connect(marker->channel, marker->name,
+					 "default");
+		if (err)
+			return err;
+		break;
+	case 'N':
+	case 'n':
+	case '0':
+		err = ltt_marker_disconnect(marker->channel, marker->name,
+					    "default");
+		if (err)
+			return err;
+		break;
+	default:
+		return -EPERM;
+	}
+
+	return cnt;
+}
+
+static const struct file_operations enable_fops = {
+	.open = marker_enable_open,
+	.read = marker_enable_read,
+	.write = marker_enable_write,
+};
+
+static int marker_info_open(struct inode *inode, struct file *filp)
+{
+	filp->private_data = inode->i_private;
+	return 0;
+}
+
+static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
+			    size_t cnt, loff_t *ppos)
+{
+	struct marker *marker;
+	char *buf;
+	int len;
+
+	marker = (struct marker *)filp->private_data;
+	buf = kmalloc(1024, GFP_KERNEL);
+
+	len = sprintf(buf, "format: \"%s\"\nstate: %d\n"
+		      "event_id: %hu\n"
+		      "call: 0x%p\n"
+		      "probe %s : 0x%p\n",
+		      marker->format, _imv_read(marker->state),
+		      marker->event_id, marker->call, marker->ptype ?
+		      "multi" : "single", marker->ptype ?
+		      (void *)marker->multi : (void *)marker->single.func);
+
+	len = simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
+	kfree(buf);
+
+	return len;
+}
+
+static const struct file_operations info_fops = {
+	.open = marker_info_open,
+	.read = marker_info_read,
+};
+
+static int build_marker_file(struct marker *marker)
+{
+	struct dentry *channel_d, *marker_d, *enable_d, *info_d;
+	int err;
+
+	channel_d = dir_lookup(markers_control_dir, marker->channel);
+	if (!channel_d) {
+		channel_d = debugfs_create_dir(marker->channel,
+					       markers_control_dir);
+		if (IS_ERR(channel_d) || !channel_d) {
+			printk(KERN_ERR
+			       "%s: build channel dir of %s failed\n",
+			       __func__, marker->channel);
+			err = -ENOMEM;
+			goto err_build_fail;
+		}
+	}
+
+	marker_d  = dir_lookup(channel_d, marker->name);
+	if (!marker_d) {
+		marker_d = debugfs_create_dir(marker->name, channel_d);
+		if (IS_ERR(marker_d) || !marker_d) {
+			printk(KERN_ERR
+			       "%s: marker dir of %s failed\n",
+			       __func__, marker->name);
+			err = -ENOMEM;
+			goto err_build_fail;
+		}
+	}
+
+	enable_d = dir_lookup(marker_d, "enable");
+	if (!enable_d) {
+		enable_d = debugfs_create_file("enable", 0644, marker_d,
+						marker, &enable_fops);
+		if (IS_ERR(enable_d) || !enable_d) {
+			printk(KERN_ERR
+			       "%s: create file of %s failed\n",
+			       __func__, "enable");
+			err = -ENOMEM;
+			goto err_build_fail;
+		}
+	}
+
+	info_d = dir_lookup(marker_d, "info");
+	if (!info_d) {
+		info_d = debugfs_create_file("info", 0444, marker_d,
+						marker, &info_fops);
+		if (IS_ERR(info_d) || !info_d) {
+			printk(KERN_ERR
+			       "%s: create file of %s failed\n",
+			       __func__, "enable");
+			err = -ENOMEM;
+			goto err_build_fail;
+		}
+	}
+
+	return 0;
+
+err_build_fail:
+	return err;
+}
+
+static int build_marker_control_files(void)
+{
+	struct marker_iter iter;
+	int err;
+
+	err = 0;
+	if (!markers_control_dir)
+		return -EEXIST;
+
+	marker_iter_reset(&iter);
+	marker_iter_start(&iter);
+	for (; iter.marker != NULL; marker_iter_next(&iter)) {
+		err = build_marker_file(iter.marker);
+		if (err)
+			goto err_build_fail;
+	}
+	marker_iter_stop(&iter);
+	return 0;
+
+err_build_fail:
+	return err;
+}
+
+static int remove_marker_control_dir(struct marker *marker)
+{
+	struct dentry *channel_d, *marker_d;
+
+	channel_d = dir_lookup(markers_control_dir, marker->channel);
+	if (!channel_d)
+		return -ENOENT;
+
+	marker_d = dir_lookup(channel_d, marker->name);
+	if (!marker_d)
+		return -ENOENT;
+
+	debugfs_remove_recursive(marker_d);
+	if (list_empty(&channel_d->d_subdirs))
+		debugfs_remove(channel_d);
+
+	return 0;
+}
+
+static void cleanup_control_dir(struct marker *begin, struct marker *end)
+{
+	struct marker *iter;
+
+	if (!markers_control_dir)
+		return;
+
+	for (iter = begin; iter < end; iter++)
+		remove_marker_control_dir(iter);
+
+	return;
+}
+
+static void build_control_dir(struct marker *begin, struct marker *end)
+{
+	struct marker *iter;
+	int err;
+
+	err = 0;
+	if (!markers_control_dir)
+		return;
+
+	for (iter = begin; iter < end; iter++) {
+		err = build_marker_file(iter);
+		if (err)
+			goto err_build_fail;
+	}
+
+	return;
+err_build_fail:
+	cleanup_control_dir(begin, end);
+}
+
+static int module_notify(struct notifier_block *self,
+		  unsigned long val, void *data)
+{
+	struct module *mod = data;
+
+	switch (val) {
+	case MODULE_STATE_COMING:
+		build_control_dir(mod->markers,
+				  mod->markers + mod->num_markers);
+		break;
+	case MODULE_STATE_GOING:
+		cleanup_control_dir(mod->markers,
+				    mod->markers + mod->num_markers);
+		break;
+	}
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block module_nb = {
+	.notifier_call = module_notify,
+};
 
 static int __init ltt_trace_control_init(void)
 {
@@ -733,8 +1002,27 @@ static int __init ltt_trace_control_init
 		goto err_create_destroy_trace_file;
 	}
 
-	return 0;
+	markers_control_dir = debugfs_create_dir(MARKERS_CONTROL_DIR,
+						 ltt_root_dentry);
+	if (IS_ERR(markers_control_dir) || !markers_control_dir) {
+		printk(KERN_ERR
+			"ltt_channel_control_init: create dir of %s failed\n",
+			MARKERS_CONTROL_DIR);
+		err = -ENOMEM;
+		goto err_create_marker_control_dir;
+	}
 
+	if (build_marker_control_files())
+		goto err_build_fail;
+
+	if (!register_module_notifier(&module_nb))
+		return 0;
+
+err_build_fail:
+	debugfs_remove_recursive(markers_control_dir);
+	markers_control_dir = NULL;
+err_create_marker_control_dir:
+	debugfs_remove(ltt_destroy_trace_file);
 err_create_destroy_trace_file:
 	debugfs_remove(ltt_setup_trace_file);
 err_create_setup_trace_file:
@@ -759,6 +1047,8 @@ static void __exit ltt_trace_control_exi
 	debugfs_remove(ltt_setup_trace_file);
 	debugfs_remove(ltt_destroy_trace_file);
 	debugfs_remove_recursive(ltt_control_dir);
+	debugfs_remove_recursive(markers_control_dir);
+	unregister_module_notifier(&module_nb);
 }
 
 module_init(ltt_trace_control_init);


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