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

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

[Gui Jianfeng]

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

Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
---
 ltt/ltt-trace-control.c |  302 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 297 insertions(+), 5 deletions(-)

diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
index 16502a2..e4676bf 100644
--- a/ltt/ltt-trace-control.c
+++ b/ltt/ltt-trace-control.c
@@ -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,272 @@ static struct file_operations ltt_destroy_trace_operations = {
 	.write = destroy_trace_write,
 };
 
+static int marker_enabled_open(struct inode *inode, struct file *filp)
+{
+	filp->private_data = inode->i_private;
+	return 0;
+}
+
+static ssize_t marker_enabled_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_enabled_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 struct file_operations enabled_fops = {
+	.open = marker_enabled_open,
+	.read = marker_enabled_read,
+	.write = marker_enabled_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 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, *enabled_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;
+		}
+	}
+	
+	enabled_d = dir_lookup(marker_d, "enabled");
+	if (!enabled_d) {
+		enabled_d = debugfs_create_file("enabled", 0644, marker_d,
+						marker, &enabled_fops);
+		if (IS_ERR(enabled_d) || !enabled_d) {
+			printk(KERN_ERR
+			       "%s: create file of %s failed\n",
+			       __func__, "enabled");
+			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__, "enabled");
+			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);
+	return;
+}
+
+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 +1004,27 @@ static int __init ltt_trace_control_init(void)
 		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 +1049,8 @@ static void __exit ltt_trace_control_exit(void)
 	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);
-- 
1.5.4.rc3

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

[Mathieu Desnoyers]

* 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
get used to run it before submission, it's very good at spotting
whitespaces at the end of lines and other coding style errors)

Best regards,

Mathieu

> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
> ---
>  ltt/ltt-trace-control.c |  302 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 297 insertions(+), 5 deletions(-)
> 
> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
> index 16502a2..e4676bf 100644
> --- a/ltt/ltt-trace-control.c
> +++ b/ltt/ltt-trace-control.c
> @@ -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,272 @@ static struct file_operations ltt_destroy_trace_operations = {
>  	.write = destroy_trace_write,
>  };
>  
> +static int marker_enabled_open(struct inode *inode, struct file *filp)
> +{
> +	filp->private_data = inode->i_private;
> +	return 0;
> +}
> +
> +static ssize_t marker_enabled_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_enabled_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 struct file_operations enabled_fops = {
> +	.open = marker_enabled_open,
> +	.read = marker_enabled_read,
> +	.write = marker_enabled_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 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, *enabled_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;
> +		}
> +	}
> +	
> +	enabled_d = dir_lookup(marker_d, "enabled");
> +	if (!enabled_d) {
> +		enabled_d = debugfs_create_file("enabled", 0644, marker_d,
> +						marker, &enabled_fops);
> +		if (IS_ERR(enabled_d) || !enabled_d) {
> +			printk(KERN_ERR
> +			       "%s: create file of %s failed\n",
> +			       __func__, "enabled");
> +			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__, "enabled");
> +			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);
> +	return;
> +}
> +
> +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 +1004,27 @@ static int __init ltt_trace_control_init(void)
>  		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 +1049,8 @@ static void __exit ltt_trace_control_exit(void)
>  	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);
> -- 
> 1.5.4.rc3
> 
> 
> _______________________________________________
> 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

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

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj at krystal.dyndns.org) 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

I'll also change enabled -> enable

to follow the same semantic as debugfs/ltt/ per-channel control files.

Mathieu

> > |   |   |   `-- 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
> get used to run it before submission, it's very good at spotting
> whitespaces at the end of lines and other coding style errors)
> 
> Best regards,
> 
> Mathieu
> 
> > Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
> > ---
> >  ltt/ltt-trace-control.c |  302 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 297 insertions(+), 5 deletions(-)
> > 
> > diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
> > index 16502a2..e4676bf 100644
> > --- a/ltt/ltt-trace-control.c
> > +++ b/ltt/ltt-trace-control.c
> > @@ -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,272 @@ static struct file_operations ltt_destroy_trace_operations = {
> >  	.write = destroy_trace_write,
> >  };
> >  
> > +static int marker_enabled_open(struct inode *inode, struct file *filp)
> > +{
> > +	filp->private_data = inode->i_private;
> > +	return 0;
> > +}
> > +
> > +static ssize_t marker_enabled_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_enabled_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 struct file_operations enabled_fops = {
> > +	.open = marker_enabled_open,
> > +	.read = marker_enabled_read,
> > +	.write = marker_enabled_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 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, *enabled_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;
> > +		}
> > +	}
> > +	
> > +	enabled_d = dir_lookup(marker_d, "enabled");
> > +	if (!enabled_d) {
> > +		enabled_d = debugfs_create_file("enabled", 0644, marker_d,
> > +						marker, &enabled_fops);
> > +		if (IS_ERR(enabled_d) || !enabled_d) {
> > +			printk(KERN_ERR
> > +			       "%s: create file of %s failed\n",
> > +			       __func__, "enabled");
> > +			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__, "enabled");
> > +			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);
> > +	return;
> > +}
> > +
> > +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 +1004,27 @@ static int __init ltt_trace_control_init(void)
> >  		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 +1049,8 @@ static void __exit ltt_trace_control_exit(void)
> >  	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);
> > -- 
> > 1.5.4.rc3
> > 
> > 
> > _______________________________________________
> > 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
> 
> _______________________________________________
> 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

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

[Gui Jianfeng]

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.

> get used to run it before submission, it's very good at spotting
> whitespaces at the end of lines and other coding style errors)
> 
> Best regards,
> 
> Mathieu
> 
>> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
>> ---
>>  ltt/ltt-trace-control.c |  302 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 297 insertions(+), 5 deletions(-)
>>
>> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
>> index 16502a2..e4676bf 100644
>> --- a/ltt/ltt-trace-control.c
>> +++ b/ltt/ltt-trace-control.c
>> @@ -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,272 @@ static struct file_operations ltt_destroy_trace_operations = {
>>  	.write = destroy_trace_write,
>>  };
>>  
>> +static int marker_enabled_open(struct inode *inode, struct file *filp)
>> +{
>> +	filp->private_data = inode->i_private;
>> +	return 0;
>> +}
>> +
>> +static ssize_t marker_enabled_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_enabled_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 struct file_operations enabled_fops = {
>> +	.open = marker_enabled_open,
>> +	.read = marker_enabled_read,
>> +	.write = marker_enabled_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 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, *enabled_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;
>> +		}
>> +	}
>> +	
>> +	enabled_d = dir_lookup(marker_d, "enabled");
>> +	if (!enabled_d) {
>> +		enabled_d = debugfs_create_file("enabled", 0644, marker_d,
>> +						marker, &enabled_fops);
>> +		if (IS_ERR(enabled_d) || !enabled_d) {
>> +			printk(KERN_ERR
>> +			       "%s: create file of %s failed\n",
>> +			       __func__, "enabled");
>> +			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__, "enabled");
>> +			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);
>> +	return;
>> +}
>> +
>> +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 +1004,27 @@ static int __init ltt_trace_control_init(void)
>>  		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 +1049,8 @@ static void __exit ltt_trace_control_exit(void)
>>  	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);
>> -- 
>> 1.5.4.rc3
>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
> 

-- 
Regards
Gui Jianfeng

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

[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

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

[Gui Jianfeng]

Mathieu Desnoyers wrote:
> 
> 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.

  Hi Mathieu,

  When enabling a marker with either way(old proc or debugfs), if this marker
  belongs a module, I'm afraid this module can't be unloaded any more.
  For example:
  #modprobe fs_trace
  #echo "connect fs close default" > /proc/ltt
  #rmmod fs_trace
  We'll get:
  ERROR: Module fs_trace is in use

  I tested under lttng-0.91, do i miss something?

> 
> 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.

  Do you mean even if the marker is enabled(by using debugfs or proc interface),
  the module which includes this marker are still able to be unloaded?

> 
> 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 tried on lttng-0.91
  #echo "disconnect metadata core_marker_format" > /proc/ltt
  #cat /proc/ltt | grep core_marker_format
  i got the following:
    channel: metadata marker: core_marker_format format: "channel %s name %s format %s" state: 0 event_id: 1 call: 0xc0460b7c probe single : 0xc045f904
  
  It seems core_marker_format has been disabled by old interface. Do i miss something again?

  ltt-armall do the restriction, consider the follwing piece of code 
  MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2 " " $4}'|sort -u|grep -v ^metadata
|          grep -v ^locking|grep -v ^lockdep`
  Actually, this block of code do the restriction from userspace.

> 
> 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);
> 
> 

-- 
Regards
Gui Jianfeng

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

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > 
> > 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.
> 
>   Hi Mathieu,
> 
>   When enabling a marker with either way(old proc or debugfs), if this marker
>   belongs a module, I'm afraid this module can't be unloaded any more.
>   For example:
>   #modprobe fs_trace
>   #echo "connect fs close default" > /proc/ltt
>   #rmmod fs_trace
>   We'll get:
>   ERROR: Module fs_trace is in use
> 
>   I tested under lttng-0.91, do i miss something?
> 

That's expected when a marker is a trace_mark_tp. That's the simplest
way I found to deal with references. As I recall, a trace_mark() (not
_tp) should not have this side-effect. So basically, it's mostly the
lttng probes which have this refcounting.

> > 
> > 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.
> 
>   Do you mean even if the marker is enabled(by using debugfs or proc interface),
>   the module which includes this marker are still able to be unloaded?
> 

See the above paragraph about explanation around trace_mark_tp(). So
basically, I expect that lttng module containing trace_mark_tp() should
not be unloadable when armed.

> > 
> > 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 tried on lttng-0.91
>   #echo "disconnect metadata core_marker_format" > /proc/ltt
>   #cat /proc/ltt | grep core_marker_format
>   i got the following:
>     channel: metadata marker: core_marker_format format: "channel %s name %s format %s" state: 0 event_id: 1 call: 0xc0460b7c probe single : 0xc045f904
>   
>   It seems core_marker_format has been disabled by old interface. Do i miss something again?
> 
>   ltt-armall do the restriction, consider the follwing piece of code 
>   MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2 " " $4}'|sort -u|grep -v ^metadata
> |          grep -v ^locking|grep -v ^lockdep`
>   Actually, this block of code do the restriction from userspace.
> 

Ah, this is possible. Then we have to make sure the core event IDs are
never allocated dynamically.

I am pulling your new interface for 2.6.29-rc4, we'll start from there.
As you say, it provides the same functionnality as the old interface. We
might want to test the cases I spotted. They might well be problems that
were there in the previous interface as well.

Thanks,

Mathieu

> > 
> > 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);
> > 
> > 
> 
> -- 
> Regards
> Gui Jianfeng
> 
> 
> 
> _______________________________________________
> 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

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

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> > Mathieu Desnoyers wrote:
> > > 
> > > 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.
> > 
> >   Hi Mathieu,
> > 
> >   When enabling a marker with either way(old proc or debugfs), if this marker
> >   belongs a module, I'm afraid this module can't be unloaded any more.
> >   For example:
> >   #modprobe fs_trace
> >   #echo "connect fs close default" > /proc/ltt
> >   #rmmod fs_trace
> >   We'll get:
> >   ERROR: Module fs_trace is in use
> > 
> >   I tested under lttng-0.91, do i miss something?
> > 
> 
> That's expected when a marker is a trace_mark_tp. That's the simplest
> way I found to deal with references. As I recall, a trace_mark() (not
> _tp) should not have this side-effect. So basically, it's mostly the
> lttng probes which have this refcounting.
> 
> > > 
> > > 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.
> > 
> >   Do you mean even if the marker is enabled(by using debugfs or proc interface),
> >   the module which includes this marker are still able to be unloaded?
> > 
> 
> See the above paragraph about explanation around trace_mark_tp(). So
> basically, I expect that lttng module containing trace_mark_tp() should
> not be unloadable when armed.
> 
> > > 
> > > 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 tried on lttng-0.91
> >   #echo "disconnect metadata core_marker_format" > /proc/ltt
> >   #cat /proc/ltt | grep core_marker_format
> >   i got the following:
> >     channel: metadata marker: core_marker_format format: "channel %s name %s format %s" state: 0 event_id: 1 call: 0xc0460b7c probe single : 0xc045f904
> >   
> >   It seems core_marker_format has been disabled by old interface. Do i miss something again?
> > 
> >   ltt-armall do the restriction, consider the follwing piece of code 
> >   MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2 " " $4}'|sort -u|grep -v ^metadata
> > |          grep -v ^locking|grep -v ^lockdep`
> >   Actually, this block of code do the restriction from userspace.
> > 
> 
> Ah, this is possible. Then we have to make sure the core event IDs are
> never allocated dynamically.
> 
> I am pulling your new interface for 2.6.29-rc4, we'll start from there.
> As you say, it provides the same functionnality as the old interface. We
> might want to test the cases I spotted. They might well be problems that
> were there in the previous interface as well.
> 

I think I have an idea we could implement to support arming markers
which are not loaded in the kernel yet. The use-case is :

We have a marker in a given module, which is not loaded. We are
interested in tracing events in this module happening soon after module
load. Therefore, we cannot use the /mnt/debugfs/ltt/markers/**/* files,
because the marker is not present yet.

We could implement mkdir operations to support this. This is out of
ltt-armall scope, and people would have to use it manually and *know*
what channel and marker names they want to enable, but I think it's ok.

So basically, we would have :

cd /mnt/debugfs/ltt
mkdir channel_name
cd channel_name
mkdir marker_name

The marker_name subdirectory would be populated with standard
information and enable files.

Do you think it could be added easily to ltt-trace-control ?

Mathieu


> Thanks,
> 
> Mathieu
> 
> > > 
> > > 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);
> > > 
> > > 
> > 
> > -- 
> > Regards
> > Gui Jianfeng
> > 
> > 
> > 
> > _______________________________________________
> > 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

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

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

[Gui Jianfeng]

Mathieu Desnoyers wrote:
> I think I have an idea we could implement to support arming markers
> which are not loaded in the kernel yet. The use-case is :
> 
> We have a marker in a given module, which is not loaded. We are
> interested in tracing events in this module happening soon after module
> load. Therefore, we cannot use the /mnt/debugfs/ltt/markers/**/* files,
> because the marker is not present yet.
> 
> We could implement mkdir operations to support this. This is out of
> ltt-armall scope, and people would have to use it manually and *know*
> what channel and marker names they want to enable, but I think it's ok.
> 
> So basically, we would have :
> 
> cd /mnt/debugfs/ltt
> mkdir channel_name
> cd channel_name
> mkdir marker_name
> 
> The marker_name subdirectory would be populated with standard
> information and enable files.

  Do you mean if we do "mkdir channel_name" and "mkdir marker_name"
  kernel will automatically insert the probe module, but in debugfs, 
  it doesn't show all of the markers this module have, but just the 
  requested one?

> 
> Do you think it could be added easily to ltt-trace-control ?
> 
> Mathieu
> 
> 
>> Thanks,
>>
>> Mathieu
>>
>>>> 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);
>>>>
>>>>
>>> -- 
>>> Regards
>>> Gui Jianfeng
>>>
>>>
>>>
>>> _______________________________________________
>>> 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
> 

-- 
Regards
Gui Jianfeng

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

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > I think I have an idea we could implement to support arming markers
> > which are not loaded in the kernel yet. The use-case is :
> > 
> > We have a marker in a given module, which is not loaded. We are
> > interested in tracing events in this module happening soon after module
> > load. Therefore, we cannot use the /mnt/debugfs/ltt/markers/**/* files,
> > because the marker is not present yet.
> > 
> > We could implement mkdir operations to support this. This is out of
> > ltt-armall scope, and people would have to use it manually and *know*
> > what channel and marker names they want to enable, but I think it's ok.
> > 
> > So basically, we would have :
> > 
> > cd /mnt/debugfs/ltt
> > mkdir channel_name
> > cd channel_name
> > mkdir marker_name
> > 
> > The marker_name subdirectory would be populated with standard
> > information and enable files.
> 
>   Do you mean if we do "mkdir channel_name" and "mkdir marker_name"
>   kernel will automatically insert the probe module, but in debugfs, 
>   it doesn't show all of the markers this module have, but just the 
>   requested one?
> 

No. I mean that doing :
mkdir channel_name
mkdir channel_name/marker_name
echo 1 > channel_name/marker_name

Should be the same as the 
echo "connect channel_name marker_name" > /proc/ltt

This worked even if the channel/marker pair were not listed in
/proc/ltt.

This would create the directories locally even if they do not exist in
the marker table, and call ltt_marker_connect() when echo 1 >
channel_name/marker_name is done.

Mathieu

> > 
> > Do you think it could be added easily to ltt-trace-control ?
> > 
> > Mathieu
> > 
> > 
> >> Thanks,
> >>
> >> Mathieu
> >>
> >>>> 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);
> >>>>
> >>>>
> >>> -- 
> >>> Regards
> >>> Gui Jianfeng
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> 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
> > 
> 
> -- 
> Regards
> Gui Jianfeng
> 
> _______________________________________________
> 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

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

[Gui Jianfeng]

Mathieu Desnoyers wrote:
> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>> Mathieu Desnoyers wrote:
>>> I think I have an idea we could implement to support arming markers
>>> which are not loaded in the kernel yet. The use-case is :
>>>
>>> We have a marker in a given module, which is not loaded. We are
>>> interested in tracing events in this module happening soon after module
>>> load. Therefore, we cannot use the /mnt/debugfs/ltt/markers/**/* files,
>>> because the marker is not present yet.
>>>
>>> We could implement mkdir operations to support this. This is out of
>>> ltt-armall scope, and people would have to use it manually and *know*
>>> what channel and marker names they want to enable, but I think it's ok.
>>>
>>> So basically, we would have :
>>>
>>> cd /mnt/debugfs/ltt
>>> mkdir channel_name
>>> cd channel_name
>>> mkdir marker_name
>>>
>>> The marker_name subdirectory would be populated with standard
>>> information and enable files.
>>   Do you mean if we do "mkdir channel_name" and "mkdir marker_name"
>>   kernel will automatically insert the probe module, but in debugfs, 
>>   it doesn't show all of the markers this module have, but just the 
>>   requested one?
>>
> 
> No. I mean that doing :
> mkdir channel_name
> mkdir channel_name/marker_name
> echo 1 > channel_name/marker_name
> 
> Should be the same as the 
> echo "connect channel_name marker_name" > /proc/ltt
> 
> This worked even if the channel/marker pair were not listed in
> /proc/ltt.
> 
> This would create the directories locally even if they do not exist in
> the marker table, and call ltt_marker_connect() when echo 1 >
> channel_name/marker_name is done.

  Well, I got your point now.
  /proc/ltt shows the markers which are located in a specific ELF section "__markers".
  echo "connect channel_name marker_name" > /proc/ltt just creates a marker_entry. Since
  a marker with "channel_name marker_name" doesn't exist in any where. IOW, it can never
  be triggered. Hence, does this feature really make sense?

> 
> Mathieu
> 
>>> Do you think it could be added easily to ltt-trace-control ?
>>>
>>> Mathieu
>>>
>>>
>>>> Thanks,
>>>>
>>>> Mathieu
>>>>
>>>>>> 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);
>>>>>>
>>>>>>
>>>>> -- 
>>>>> Regards
>>>>> Gui Jianfeng
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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
>> -- 
>> Regards
>> Gui Jianfeng
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
> 

-- 
Regards
Gui Jianfeng

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

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> >> Mathieu Desnoyers wrote:
> >>> I think I have an idea we could implement to support arming markers
> >>> which are not loaded in the kernel yet. The use-case is :
> >>>
> >>> We have a marker in a given module, which is not loaded. We are
> >>> interested in tracing events in this module happening soon after module
> >>> load. Therefore, we cannot use the /mnt/debugfs/ltt/markers/**/* files,
> >>> because the marker is not present yet.
> >>>
> >>> We could implement mkdir operations to support this. This is out of
> >>> ltt-armall scope, and people would have to use it manually and *know*
> >>> what channel and marker names they want to enable, but I think it's ok.
> >>>
> >>> So basically, we would have :
> >>>
> >>> cd /mnt/debugfs/ltt
> >>> mkdir channel_name
> >>> cd channel_name
> >>> mkdir marker_name
> >>>
> >>> The marker_name subdirectory would be populated with standard
> >>> information and enable files.
> >>   Do you mean if we do "mkdir channel_name" and "mkdir marker_name"
> >>   kernel will automatically insert the probe module, but in debugfs, 
> >>   it doesn't show all of the markers this module have, but just the 
> >>   requested one?
> >>
> > 
> > No. I mean that doing :
> > mkdir channel_name
> > mkdir channel_name/marker_name
> > echo 1 > channel_name/marker_name
> > 
> > Should be the same as the 
> > echo "connect channel_name marker_name" > /proc/ltt
> > 
> > This worked even if the channel/marker pair were not listed in
> > /proc/ltt.
> > 
> > This would create the directories locally even if they do not exist in
> > the marker table, and call ltt_marker_connect() when echo 1 >
> > channel_name/marker_name is done.
> 
>   Well, I got your point now.
>   /proc/ltt shows the markers which are located in a specific ELF section "__markers".
>   echo "connect channel_name marker_name" > /proc/ltt just creates a marker_entry. Since
>   a marker with "channel_name marker_name" doesn't exist in any where. IOW, it can never
>   be triggered. Hence, does this feature really make sense?
> 

Yes, because it could eventually exist when a module is loaded. We have
this situation :

ltt-armall list markers (marker X unavailable)
modprobe module Z (contains marker X)
would like to probe marker X...

So we have to allow enabling a unexisting marker.

Mathieu

> > 
> > Mathieu
> > 
> >>> Do you think it could be added easily to ltt-trace-control ?
> >>>
> >>> Mathieu
> >>>
> >>>
> >>>> Thanks,
> >>>>
> >>>> Mathieu
> >>>>
> >>>>>> 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);
> >>>>>>
> >>>>>>
> >>>>> -- 
> >>>>> Regards
> >>>>> Gui Jianfeng
> >>>>>
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> 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
> >> -- 
> >> Regards
> >> Gui Jianfeng
> >>
> >> _______________________________________________
> >> ltt-dev mailing list
> >> ltt-dev at lists.casi.polymtl.ca
> >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> >>
> > 
> 
> -- 
> Regards
> Gui Jianfeng
> 
> _______________________________________________
> 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

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

[Gui Jianfeng]

Mathieu Desnoyers wrote:
> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>> Mathieu Desnoyers wrote:
>>> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>>>> Mathieu Desnoyers wrote:
>>>>> I think I have an idea we could implement to support arming markers
>>>>> which are not loaded in the kernel yet. The use-case is :
>>>>>
>>>>> We have a marker in a given module, which is not loaded. We are
>>>>> interested in tracing events in this module happening soon after module
>>>>> load. Therefore, we cannot use the /mnt/debugfs/ltt/markers/**/* files,
>>>>> because the marker is not present yet.
>>>>>
>>>>> We could implement mkdir operations to support this. This is out of
>>>>> ltt-armall scope, and people would have to use it manually and *know*
>>>>> what channel and marker names they want to enable, but I think it's ok.
>>>>>
>>>>> So basically, we would have :
>>>>>
>>>>> cd /mnt/debugfs/ltt
>>>>> mkdir channel_name
>>>>> cd channel_name
>>>>> mkdir marker_name
>>>>>
>>>>> The marker_name subdirectory would be populated with standard
>>>>> information and enable files.
>>>>   Do you mean if we do "mkdir channel_name" and "mkdir marker_name"
>>>>   kernel will automatically insert the probe module, but in debugfs, 
>>>>   it doesn't show all of the markers this module have, but just the 
>>>>   requested one?
>>>>
>>> No. I mean that doing :
>>> mkdir channel_name
>>> mkdir channel_name/marker_name
>>> echo 1 > channel_name/marker_name
>>>
>>> Should be the same as the 
>>> echo "connect channel_name marker_name" > /proc/ltt
>>>
>>> This worked even if the channel/marker pair were not listed in
>>> /proc/ltt.
>>>
>>> This would create the directories locally even if they do not exist in
>>> the marker table, and call ltt_marker_connect() when echo 1 >
>>> channel_name/marker_name is done.
>>   Well, I got your point now.
>>   /proc/ltt shows the markers which are located in a specific ELF section "__markers".
>>   echo "connect channel_name marker_name" > /proc/ltt just creates a marker_entry. Since
>>   a marker with "channel_name marker_name" doesn't exist in any where. IOW, it can never
>>   be triggered. Hence, does this feature really make sense?
>>
> 
> Yes, because it could eventually exist when a module is loaded. We have
> this situation :
> 
> ltt-armall list markers (marker X unavailable)
> modprobe module Z (contains marker X)
> would like to probe marker X...
> 
> So we have to allow enabling a unexisting marker.

  Hi Mathieu,

  I guess it's not easy to implement the *mkdir* for marker control.
  But I have an alternative solution that we can add a control file
  *new_channel* at the same level of channel directories, and add a 
  control file *new_marker* in each channel directory.
  Making use of the following, we can create a marker even if it 
  doesn't exist by now.

  cd /mnt/ltt/markers
  echo "channel_name" > new_channel
  echo "marker_name" > channel_name/new_marker
 
 
> 
> Mathieu
> 
>>> Mathieu
>>>
>>>>> Do you think it could be added easily to ltt-trace-control ?
>>>>>
>>>>> Mathieu
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Mathieu
>>>>>>
>>>>>>>> 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);
>>>>>>>>
>>>>>>>>
>>>>>>> -- 
>>>>>>> Regards
>>>>>>> Gui Jianfeng
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> 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
>>>> -- 
>>>> Regards
>>>> Gui Jianfeng
>>>>
>>>> _______________________________________________
>>>> ltt-dev mailing list
>>>> ltt-dev at lists.casi.polymtl.ca
>>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>>>
>> -- 
>> Regards
>> Gui Jianfeng
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
> 

-- 
Regards
Gui Jianfeng

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

[KOSAKI Motohiro]

>   Hi Mathieu,
> 
>   I guess it's not easy to implement the *mkdir* for marker control.
>   But I have an alternative solution that we can add a control file
>   *new_channel* at the same level of channel directories, and add a 
>   control file *new_marker* in each channel directory.
>   Making use of the following, we can create a marker even if it 
>   doesn't exist by now.
> 
>   cd /mnt/ltt/markers
>   echo "channel_name" > new_channel
>   echo "marker_name" > channel_name/new_marker

No.
good user interface is important than easy implementation.

this UI is really ugly.

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

[Gui Jianfeng]

KOSAKI Motohiro wrote:
>>   Hi Mathieu,
>>
>>   I guess it's not easy to implement the *mkdir* for marker control.
>>   But I have an alternative solution that we can add a control file
>>   *new_channel* at the same level of channel directories, and add a 
>>   control file *new_marker* in each channel directory.
>>   Making use of the following, we can create a marker even if it 
>>   doesn't exist by now.
>>
>>   cd /mnt/ltt/markers
>>   echo "channel_name" > new_channel
>>   echo "marker_name" > channel_name/new_marker
> 
> No.
> good user interface is important than easy implementation.
> 
> this UI is really ugly.

  Hi Kosaki-san,

  Thank you for your kind comments.
  I say *not easy* becasue debugfs itself doesn't support *mkdir* completely.
  If we are going to implement mkdir in marker control, we have to modify debugfs
  itself. IMHO, it's not reasonable and i guess most of the debugfs subsystems don't 
  like to see this change.
  On the other hand, it's very natual to use a control file to do something in debugfs.
  So, I choose this solution. I hope that what I say will clarify the situation.:)
  Mathieu, What do you say?


> 
> 
> 
> 
> 
> 

-- 
Regards
Gui Jianfeng

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

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> KOSAKI Motohiro wrote:
> >>   Hi Mathieu,
> >>
> >>   I guess it's not easy to implement the *mkdir* for marker control.
> >>   But I have an alternative solution that we can add a control file
> >>   *new_channel* at the same level of channel directories, and add a 
> >>   control file *new_marker* in each channel directory.
> >>   Making use of the following, we can create a marker even if it 
> >>   doesn't exist by now.
> >>
> >>   cd /mnt/ltt/markers
> >>   echo "channel_name" > new_channel
> >>   echo "marker_name" > channel_name/new_marker
> > 
> > No.
> > good user interface is important than easy implementation.
> > 
> > this UI is really ugly.
> 
>   Hi Kosaki-san,
> 
>   Thank you for your kind comments.
>   I say *not easy* becasue debugfs itself doesn't support *mkdir* completely.
>   If we are going to implement mkdir in marker control, we have to modify debugfs
>   itself. IMHO, it's not reasonable and i guess most of the debugfs subsystems don't 
>   like to see this change.
>   On the other hand, it's very natual to use a control file to do something in debugfs.
>   So, I choose this solution. I hope that what I say will clarify the situation.:)
>   Mathieu, What do you say?
> 

Hi Gui,

I would recommend adding mkdir support in our own directories. That
means we have to override debugfs use of "simple" libfs basic
operations. We want to use the same operations, but also define a mkdir
callback. See

fs/debugfs/inode.c:
debugfs_get_inode :
it sets inode->i_op = &simple_dir_inode_operations;

We will need to override this with our own. See
fs/libfs.c: simple_dir_inode_operations

I fully agree with Kosaki : the quality of the user interface is what
matters and is much more important than implementation difficulty.

Thanks,

Mathieu

> 
> > 
> > 
> > 
> > 
> > 
> > 
> 
> -- 
> Regards
> Gui Jianfeng
> 
> 
> _______________________________________________
> 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

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

[Gui Jianfeng]

Mathieu Desnoyers wrote:
> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>> KOSAKI Motohiro wrote:
>>>>   Hi Mathieu,
>>>>
>>>>   I guess it's not easy to implement the *mkdir* for marker control.
>>>>   But I have an alternative solution that we can add a control file
>>>>   *new_channel* at the same level of channel directories, and add a 
>>>>   control file *new_marker* in each channel directory.
>>>>   Making use of the following, we can create a marker even if it 
>>>>   doesn't exist by now.
>>>>
>>>>   cd /mnt/ltt/markers
>>>>   echo "channel_name" > new_channel
>>>>   echo "marker_name" > channel_name/new_marker
>>> No.
>>> good user interface is important than easy implementation.
>>>
>>> this UI is really ugly.
>>   Hi Kosaki-san,
>>
>>   Thank you for your kind comments.
>>   I say *not easy* becasue debugfs itself doesn't support *mkdir* completely.
>>   If we are going to implement mkdir in marker control, we have to modify debugfs
>>   itself. IMHO, it's not reasonable and i guess most of the debugfs subsystems don't 
>>   like to see this change.
>>   On the other hand, it's very natual to use a control file to do something in debugfs.
>>   So, I choose this solution. I hope that what I say will clarify the situation.:)
>>   Mathieu, What do you say?
>>
> 
> Hi Gui,
> 
> I would recommend adding mkdir support in our own directories. That
> means we have to override debugfs use of "simple" libfs basic
> operations. We want to use the same operations, but also define a mkdir
> callback. See
> 
> fs/debugfs/inode.c:
> debugfs_get_inode :
> it sets inode->i_op = &simple_dir_inode_operations;
> 
> We will need to override this with our own. See
> fs/libfs.c: simple_dir_inode_operations
> 
  Ah, I see now, thanks for the suggestion, 

> I fully agree with Kosaki : the quality of the user interface is what
> matters and is much more important than implementation difficulty.

  Me too :)

> 
> Thanks,
> 
> Mathieu
> 
>>>
>>>
>>>
>>>
>>>
>> -- 
>> Regards
>> Gui Jianfeng
>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
> 

-- 
Regards
Gui Jianfeng