[ltt-dev] [PATCH] LTTNG: same-name marker handling

[guijianfeng@cn.fujitsu.com (Gui Jianfeng)]

Hi Mathieu,

This patch enables same-name marker handling in debugfs.
The cached marker pointer is removed, and by looking up for
each time instead.
If there are two markers with the same channel name, marker name
and format pair, for example, channel:fs, name:close, format:"fd %u"
in fs_trace and and marker_example respectively. you can get the 
following message when you read "info" file in debugfs.

[root at localhost close]# cat info
Location: fs_trace
format: "fd %u"
state: 0
event_id: 0
call: 0xc0468167
probe single : 0xc0466ed8

Location: marker_example
format: "fd %u"
state: 0
event_id: 0
call: 0xc0468167
probe single : 0xc0466ed8

Location indicates where this marker locates.

and enable them all by issuing the following command:

[root at localhost close]# echo 1 > enable

Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
---
 kernel/marker.c         |    3 +-
 ltt/ltt-trace-control.c |  118 ++++++++++++++++++++++++++++++-----------------
 2 files changed, 77 insertions(+), 44 deletions(-)

diff --git a/kernel/marker.c b/kernel/marker.c
index 3820622..bb2b718 100644
--- a/kernel/marker.c
+++ b/kernel/marker.c
@@ -1321,6 +1321,7 @@ void exit_user_markers(struct task_struct *p)
 		mutex_unlock(&markers_mutex);
 	}
 }
+#endif
 
 int is_marker_enabled(const char *channel, const char *name)
 {
@@ -1332,7 +1333,7 @@ int is_marker_enabled(const char *channel, const char *name)
 
 	return entry && !!entry->refcount;
 }
-#endif
+EXPORT_SYMBOL_GPL(is_marker_enabled);
 
 int marker_module_notify(struct notifier_block *self,
 			 unsigned long val, void *data)
diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
index 128db4e..0dc5e93 100644
--- a/ltt/ltt-trace-control.c
+++ b/ltt/ltt-trace-control.c
@@ -19,6 +19,7 @@
 #include <linux/debugfs.h>
 #include <linux/ltt-tracer.h>
 #include <linux/notifier.h>
+#include <linux/marker.h>
 
 #define LTT_CONTROL_DIR "control"
 #define MARKERS_CONTROL_DIR "markers"
@@ -60,7 +61,6 @@ static struct dentry *dir_lookup(struct dentry *parent, const char *name)
 	return d;
 }
 
-
 static ssize_t alloc_write(struct file *file, const char __user *user_buf,
 		size_t count, loff_t *ppos)
 {
@@ -697,23 +697,24 @@ static struct file_operations ltt_destroy_trace_operations = {
 	.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;
+	const char *channel, *marker;
 	int len;
 
-	marker = (struct marker *)filp->private_data;
+	marker = filp->f_dentry->d_parent->d_name.name;
+	channel = filp->f_dentry->d_parent->d_parent->d_name.name;
+
+	len = 0;
 	buf = kmalloc(1024, GFP_KERNEL);
 
-	len = sprintf(buf, "%d\n", _imv_read(marker->state));
+	if (is_marker_enabled(channel, marker))
+		len = sprintf(buf, "%d\n", 1);
+	else
+		len = sprintf(buf, "%d\n", 0);
 
 	len = simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
 	kfree(buf);
@@ -727,9 +728,11 @@ static ssize_t marker_enable_write(struct file *filp, const char __user *ubuf,
 	char buf[NAME_MAX];
 	int buf_size;
 	int err = 0;
-	struct marker *marker;
+	const char *channel, *marker;
+
+	marker = filp->f_dentry->d_parent->d_name.name;
+	channel = filp->f_dentry->d_parent->d_parent->d_name.name;
 
-	marker = (struct marker *)filp->private_data;
 	buf_size = min(cnt, sizeof(buf) - 1);
 	err = copy_from_user(buf, ubuf, buf_size);
 	if (err)
@@ -741,16 +744,14 @@ static ssize_t marker_enable_write(struct file *filp, const char __user *ubuf,
 	case 'Y':
 	case 'y':
 	case '1':
-		err = ltt_marker_connect(marker->channel, marker->name,
-					 "default");
+		err = ltt_marker_connect(channel, marker, "default");
 		if (err)
 			return err;
 		break;
 	case 'N':
 	case 'n':
 	case '0':
-		err = ltt_marker_disconnect(marker->channel, marker->name,
-					    "default");
+		err = ltt_marker_disconnect(channel, marker, "default");
 		if (err)
 			return err;
 		break;
@@ -762,35 +763,46 @@ static ssize_t marker_enable_write(struct file *filp, const char __user *ubuf,
 }
 
 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;
+	const char *channel, *marker;
 	int len;
+	struct marker_iter iter;
+
+	marker = filp->f_dentry->d_parent->d_name.name;
+	channel = filp->f_dentry->d_parent->d_parent->d_name.name;
 
-	marker = (struct marker *)filp->private_data;
+	len = 0;
 	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);
+	marker_iter_reset(&iter);
+	marker_iter_start(&iter);
+	for (; iter.marker != NULL; marker_iter_next(&iter)) {
+		if (!strcmp(iter.marker->channel, channel) &&
+		    !strcmp(iter.marker->name, marker))
+			len += sprintf(buf + len, "Location: %s\n"
+				       "format: \"%s\"\nstate: %d\n"
+				       "event_id: %hu\n"
+				       "call: 0x%p\n"
+				       "probe %s : 0x%p\n\n",
+				       iter.module ? iter.module->name :
+				       "Core Kernel",
+				       iter.marker->format,
+				       _imv_read(iter.marker->state),
+				       iter.marker->event_id,
+				       iter.marker->call,
+				       iter.marker->ptype ?
+				       "multi" : "single", iter.marker->ptype ?
+				       (void *)iter.marker->multi :
+				       (void *)iter.marker->single.func);
+	}
+	marker_iter_stop(&iter);
 
 	len = simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
 	kfree(buf);
@@ -799,7 +811,6 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
 }
 
 static const struct file_operations info_fops = {
-	.open = marker_info_open,
 	.read = marker_info_read,
 };
 
@@ -836,7 +847,7 @@ static int build_marker_file(struct marker *marker)
 	enable_d = dir_lookup(marker_d, "enable");
 	if (!enable_d) {
 		enable_d = debugfs_create_file("enable", 0644, marker_d,
-						marker, &enable_fops);
+						NULL, &enable_fops);
 		if (IS_ERR(enable_d) || !enable_d) {
 			printk(KERN_ERR
 			       "%s: create file of %s failed\n",
@@ -849,7 +860,7 @@ static int build_marker_file(struct marker *marker)
 	info_d = dir_lookup(marker_d, "info");
 	if (!info_d) {
 		info_d = debugfs_create_file("info", 0444, marker_d,
-						marker, &info_fops);
+						NULL, &info_fops);
 		if (IS_ERR(info_d) || !info_d) {
 			printk(KERN_ERR
 			       "%s: create file of %s failed\n",
@@ -888,26 +899,46 @@ err_build_fail:
 	return err;
 }
 
-static int remove_marker_control_dir(struct marker *marker)
+static int remove_marker_control_dir(struct module *mod, struct marker *marker)
 {
 	struct dentry *channel_d, *marker_d;
+	const char *channel, *name;
+	int count;
+	struct marker_iter iter;
+
+	count = 0;
 
 	channel_d = dir_lookup(markers_control_dir, marker->channel);
 	if (!channel_d)
 		return -ENOENT;
+	channel = channel_d->d_name.name;
 
 	marker_d = dir_lookup(channel_d, marker->name);
 	if (!marker_d)
 		return -ENOENT;
+	name = marker_d->d_name.name;
+
+	marker_iter_reset(&iter);
+	marker_iter_start(&iter);
+	for (; iter.marker != NULL; marker_iter_next(&iter)) {
+		if (!strcmp(iter.marker->channel, channel) &&
+		    !strcmp(iter.marker->name, name) && mod != iter.module)
+			count++;
+	}
+
+	if (count > 0)
+		goto end;
 
 	debugfs_remove_recursive(marker_d);
 	if (list_empty(&channel_d->d_subdirs))
 		debugfs_remove(channel_d);
 
+end:
 	return 0;
 }
 
-static void cleanup_control_dir(struct marker *begin, struct marker *end)
+static void cleanup_control_dir(struct module *mod, struct marker *begin,
+				struct marker *end)
 {
 	struct marker *iter;
 
@@ -915,12 +946,13 @@ static void cleanup_control_dir(struct marker *begin, struct marker *end)
 		return;
 
 	for (iter = begin; iter < end; iter++)
-		remove_marker_control_dir(iter);
+		remove_marker_control_dir(mod, iter);
 
 	return;
 }
 
-static void build_control_dir(struct marker *begin, struct marker *end)
+static void build_control_dir(struct module *mod, struct marker *begin,
+			      struct marker *end)
 {
 	struct marker *iter;
 	int err;
@@ -937,7 +969,7 @@ static void build_control_dir(struct marker *begin, struct marker *end)
 
 	return;
 err_build_fail:
-	cleanup_control_dir(begin, end);
+	cleanup_control_dir(mod, begin, end);
 }
 
 static int module_notify(struct notifier_block *self,
@@ -947,11 +979,11 @@ static int module_notify(struct notifier_block *self,
 
 	switch (val) {
 	case MODULE_STATE_COMING:
-		build_control_dir(mod->markers,
+		build_control_dir(mod, mod->markers,
 				  mod->markers + mod->num_markers);
 		break;
 	case MODULE_STATE_GOING:
-		cleanup_control_dir(mod->markers,
+		cleanup_control_dir(mod, mod->markers,
 				    mod->markers + mod->num_markers);
 		break;
 	}
-- 
1.5.4.rc3