[ltt-dev] [PATCH v2] LTTNG: Make marker enabled in advance

[ltt-dev] [PATCH v2] LTTNG: Make marker enabled in advance

[Gui Jianfeng]

This patch makes marker enabled in advance.
IOW, even if the marker isn't present for the moment,
you can still enable it for future use.
As soon as the marker is inserted into kernel, tracing 
work can be started immediately.

Here is an example for using the user interface:
This patch assumes the marker control patch is applied.
[root at localhost markers]# cd /mnt/debugfs/ltt/markers
[root at localhost markers]# mkdir fs
[root at localhost markers]# cd fs/
[root at localhost fs]# mkdir close
[root at localhost fs]# cd close/
[root at localhost close]# echo 1 > enable
[root at localhost close]# cat enable
2
[root at localhost close]# cat info
Marker Pre-enabled
[root at localhost close]# modprobe fs-trace
[root at localhost close]# cat enable
1
[root at localhost close]# cat info
Location: fs_trace
format: "fd %u"
state: 1
event_id: 0
call: 0xc0468192
probe single : 0xc0520ed8

You can also remove a marker directory by "rmdir" if there
is no actual marker backed and isn't enabled in advance. 
A channel directory can be remove if there are not any marker
directories in it.

Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
---
 include/linux/marker.h  |    1 +
 kernel/marker.c         |   29 ++++++++
 ltt/ltt-trace-control.c |  168 ++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 197 insertions(+), 1 deletions(-)

diff --git a/include/linux/marker.h b/include/linux/marker.h
index 2d78113..692805c 100644
--- a/include/linux/marker.h
+++ b/include/linux/marker.h
@@ -261,5 +261,6 @@ extern void marker_iter_reset(struct marker_iter *iter);
 extern int marker_get_iter_range(struct marker **marker, struct marker *begin,
 	struct marker *end);
 extern int is_marker_enabled(const char *channel, const char *name);
+extern int is_marker_present(const char *channel, const char *name);
 
 #endif
diff --git a/kernel/marker.c b/kernel/marker.c
index 7d11393..0c4d5c9 100644
--- a/kernel/marker.c
+++ b/kernel/marker.c
@@ -651,6 +651,35 @@ static void disable_marker(struct marker *elem)
 }
 
 /*
+ * is_marker_present - Check if a marker is present in kernel
+ * @channel: channel name
+ * @name: marker name
+ *
+ * Returns 1 if the marker is present, 0 if not.
+ */
+int is_marker_present(const char *channel, const char *name)
+{
+	int ret;
+	struct marker_iter iter;
+
+	ret = 0;
+	marker_iter_reset(&iter);
+	marker_iter_start(&iter);
+	for (; iter.marker != NULL; marker_iter_next(&iter)) {
+		if (!strcmp(iter.marker->channel, channel) &&
+		    !strcmp(iter.marker->name, name)) {
+			ret = 1;
+			goto end;
+		}
+	}
+	marker_iter_stop(&iter);
+
+end:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(is_marker_present);
+
+/*
  * is_marker_enabled - Check if a marker is enabled
  * @channel: channel name
  * @name: marker name
diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
index c1c44c6..050c49f 100644
--- a/ltt/ltt-trace-control.c
+++ b/ltt/ltt-trace-control.c
@@ -756,6 +756,12 @@ static const struct file_operations ltt_destroy_trace_operations = {
 	.write = destroy_trace_write,
 };
 
+static void init_marker_dir(struct dentry *dentry,
+			    const struct inode_operations *opt)
+{
+	dentry->d_inode->i_op = opt;
+}
+
 static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
 			    size_t cnt, loff_t *ppos)
 {
@@ -769,10 +775,15 @@ static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
 	len = 0;
 	buf = (char *)__get_free_page(GFP_KERNEL);
 
-	if (is_marker_enabled(channel, marker))
+	if (is_marker_enabled(channel, marker) &&
+	    is_marker_present(channel, marker))
 		len = snprintf(buf, PAGE_SIZE, "%d\n", 1);
+	else if (is_marker_enabled(channel, marker) &&
+		 !is_marker_present(channel, marker))
+		len = snprintf(buf, PAGE_SIZE, "%d\n", 2);
 	else
 		len = snprintf(buf, PAGE_SIZE, "%d\n", 0);
+
 	if (len >= PAGE_SIZE) {
 		len = PAGE_SIZE;
 		buf[PAGE_SIZE] = '\0';
@@ -846,6 +857,13 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
 	len = 0;
 	buf = (char *)__get_free_page(GFP_KERNEL);
 
+	if (is_marker_enabled(channel, marker) &&
+	    !is_marker_present(channel, marker)) {
+		len += snprintf(buf + len, PAGE_SIZE - len,
+				"Marker Pre-enabled\n");
+		goto out;
+	}
+
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
 	for (; iter.marker != NULL; marker_iter_next(&iter)) {
@@ -872,6 +890,7 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
 	}
 	marker_iter_stop(&iter);
 
+out:
 	if (len >= PAGE_SIZE) {
 		len = PAGE_SIZE;
 		buf[PAGE_SIZE] = '\0';
@@ -887,6 +906,149 @@ static const struct file_operations info_fops = {
 	.read = marker_info_read,
 };
 
+static int marker_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+{
+	struct dentry *marker_d, *enable_d, *info_d, *channel_d;
+	int ret;
+
+	ret = 0;
+	channel_d = (struct dentry *)dir->i_private;
+	mutex_unlock(&dir->i_mutex);
+
+	marker_d = debugfs_create_dir(dentry->d_name.name,
+				      channel_d);
+	if (IS_ERR(marker_d)) {
+		ret = PTR_ERR(marker_d);
+		goto out;
+	}
+
+	enable_d = debugfs_create_file("enable", 0644, marker_d,
+				       NULL, &enable_fops);
+	if (IS_ERR(enable_d) || !enable_d) {
+		printk(KERN_ERR
+		       "%s: create file of %s failed\n",
+		       __func__, "enable");
+		ret = -ENOMEM;
+		goto remove_marker_dir;
+	}
+
+	info_d = debugfs_create_file("info", 0644, marker_d,
+				     NULL, &info_fops);
+	if (IS_ERR(info_d) || !info_d) {
+		printk(KERN_ERR
+		       "%s: create file of %s failed\n",
+		       __func__, "info");
+		ret = -ENOMEM;
+		goto remove_enable_dir;
+	}
+
+	goto out;
+
+remove_enable_dir:
+	debugfs_remove(enable_d);
+remove_marker_dir:
+	debugfs_remove(marker_d);
+out:
+	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
+	return ret;
+}
+
+static int marker_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	struct dentry *marker_d, *channel_d;
+	const char *channel, *name;
+	int ret;
+
+	ret = 0;
+
+	channel_d = (struct dentry *)dir->i_private;
+	channel = channel_d->d_name.name;
+
+	marker_d = dir_lookup(channel_d, dentry->d_name.name);
+
+	if (!marker_d) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	name = marker_d->d_name.name;
+
+	if (is_marker_present(channel, name) ||
+	    (!is_marker_present(channel, name) &&
+	     is_marker_enabled(channel, name))) {
+		ret = -EPERM;
+		goto out;
+	}
+
+	mutex_unlock(&dir->i_mutex);
+	mutex_unlock(&dentry->d_inode->i_mutex);
+	debugfs_remove_recursive(marker_d);
+	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
+	mutex_lock(&dentry->d_inode->i_mutex);
+out:
+	return ret;
+}
+
+const struct inode_operations channel_dir_opt = {
+	.lookup = simple_lookup,
+	.mkdir = marker_mkdir,
+	.rmdir = marker_rmdir,
+};
+
+static int channel_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+{
+	struct dentry *channel_d;
+	int ret;
+
+	ret = 0;
+	mutex_unlock(&dir->i_mutex);
+
+	channel_d = debugfs_create_dir(dentry->d_name.name,
+				       markers_control_dir);
+	if (IS_ERR(channel_d)) {
+		ret = PTR_ERR(channel_d);
+		goto out;
+	}
+
+	channel_d->d_inode->i_private = (void *)channel_d;
+	init_marker_dir(channel_d, &channel_dir_opt);
+out:
+	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
+	return ret;
+}
+
+static int channel_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	struct dentry *channel_d;
+	int ret;
+
+	ret = 0;
+
+	channel_d = dir_lookup(markers_control_dir, dentry->d_name.name);
+	if (!channel_d) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	if (list_empty(&channel_d->d_subdirs)) {
+		mutex_unlock(&dir->i_mutex);
+		mutex_unlock(&dentry->d_inode->i_mutex);
+		debugfs_remove(channel_d);
+		mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
+		mutex_lock(&dentry->d_inode->i_mutex);
+	} else
+		ret = -EPERM;
+
+out:
+	return ret;
+}
+
+const struct inode_operations root_dir_opt = {
+	.lookup = simple_lookup,
+	.mkdir = channel_mkdir,
+	.rmdir = channel_rmdir
+};
+
 static int build_marker_file(struct marker *marker)
 {
 	struct dentry *channel_d, *marker_d, *enable_d, *info_d;
@@ -903,6 +1065,8 @@ static int build_marker_file(struct marker *marker)
 			err = -ENOMEM;
 			goto err_build_fail;
 		}
+		channel_d->d_inode->i_private = (void *)channel_d;
+		init_marker_dir(channel_d, &channel_dir_opt);
 	}
 
 	marker_d  = dir_lookup(channel_d, marker->name);
@@ -1118,6 +1282,8 @@ static int __init ltt_trace_control_init(void)
 		goto err_create_marker_control_dir;
 	}
 
+	init_marker_dir(markers_control_dir, &root_dir_opt);
+
 	if (build_marker_control_files())
 		goto err_build_fail;
 
-- 
1.5.4.rc3

[ltt-dev] [PATCH v2] LTTNG: Make marker enabled in advance

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> This patch makes marker enabled in advance.
> IOW, even if the marker isn't present for the moment,
> you can still enable it for future use.
> As soon as the marker is inserted into kernel, tracing 
> work can be started immediately.
> 
> Here is an example for using the user interface:
> This patch assumes the marker control patch is applied.
> [root at localhost markers]# cd /mnt/debugfs/ltt/markers
> [root at localhost markers]# mkdir fs
> [root at localhost markers]# cd fs/
> [root at localhost fs]# mkdir close
> [root at localhost fs]# cd close/
> [root at localhost close]# echo 1 > enable
> [root at localhost close]# cat enable
> 2
> [root at localhost close]# cat info
> Marker Pre-enabled
> [root at localhost close]# modprobe fs-trace
> [root at localhost close]# cat enable
> 1
> [root at localhost close]# cat info
> Location: fs_trace
> format: "fd %u"
> state: 1
> event_id: 0
> call: 0xc0468192
> probe single : 0xc0520ed8
> 
> You can also remove a marker directory by "rmdir" if there
> is no actual marker backed and isn't enabled in advance. 
> A channel directory can be remove if there are not any marker
> directories in it.
> 
> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
> ---
>  include/linux/marker.h  |    1 +
>  kernel/marker.c         |   29 ++++++++
>  ltt/ltt-trace-control.c |  168 ++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 197 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/marker.h b/include/linux/marker.h
> index 2d78113..692805c 100644
> --- a/include/linux/marker.h
> +++ b/include/linux/marker.h
> @@ -261,5 +261,6 @@ extern void marker_iter_reset(struct marker_iter *iter);
>  extern int marker_get_iter_range(struct marker **marker, struct marker *begin,
>  	struct marker *end);
>  extern int is_marker_enabled(const char *channel, const char *name);
> +extern int is_marker_present(const char *channel, const char *name);
>  
>  #endif
> diff --git a/kernel/marker.c b/kernel/marker.c
> index 7d11393..0c4d5c9 100644
> --- a/kernel/marker.c
> +++ b/kernel/marker.c
> @@ -651,6 +651,35 @@ static void disable_marker(struct marker *elem)
>  }
>  
>  /*
> + * is_marker_present - Check if a marker is present in kernel
> + * @channel: channel name
> + * @name: marker name
> + *
> + * Returns 1 if the marker is present, 0 if not.
> + */
> +int is_marker_present(const char *channel, const char *name)
> +{
> +	int ret;
> +	struct marker_iter iter;
> +
> +	ret = 0;

Shouldn't this take a markers_mutex ?

> +	marker_iter_reset(&iter);
> +	marker_iter_start(&iter);
> +	for (; iter.marker != NULL; marker_iter_next(&iter)) {
> +		if (!strcmp(iter.marker->channel, channel) &&
> +		    !strcmp(iter.marker->name, name)) {
> +			ret = 1;
> +			goto end;
> +		}
> +	}
> +	marker_iter_stop(&iter);
> +

> +end:
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(is_marker_present);
> +
> +/*
>   * is_marker_enabled - Check if a marker is enabled
>   * @channel: channel name
>   * @name: marker name
> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
> index c1c44c6..050c49f 100644
> --- a/ltt/ltt-trace-control.c
> +++ b/ltt/ltt-trace-control.c
> @@ -756,6 +756,12 @@ static const struct file_operations ltt_destroy_trace_operations = {
>  	.write = destroy_trace_write,
>  };
>  
> +static void init_marker_dir(struct dentry *dentry,
> +			    const struct inode_operations *opt)
> +{
> +	dentry->d_inode->i_op = opt;
> +}
> +
>  static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
>  			    size_t cnt, loff_t *ppos)
>  {
> @@ -769,10 +775,15 @@ static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
>  	len = 0;
>  	buf = (char *)__get_free_page(GFP_KERNEL);
>  
> -	if (is_marker_enabled(channel, marker))
> +	if (is_marker_enabled(channel, marker) &&
> +	    is_marker_present(channel, marker))
>  		len = snprintf(buf, PAGE_SIZE, "%d\n", 1);
> +	else if (is_marker_enabled(channel, marker) &&
> +		 !is_marker_present(channel, marker))
> +		len = snprintf(buf, PAGE_SIZE, "%d\n", 2);
>  	else
>  		len = snprintf(buf, PAGE_SIZE, "%d\n", 0);
> +
>  	if (len >= PAGE_SIZE) {
>  		len = PAGE_SIZE;
>  		buf[PAGE_SIZE] = '\0';
> @@ -846,6 +857,13 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
>  	len = 0;
>  	buf = (char *)__get_free_page(GFP_KERNEL);
>  
> +	if (is_marker_enabled(channel, marker) &&
> +	    !is_marker_present(channel, marker)) {
> +		len += snprintf(buf + len, PAGE_SIZE - len,
> +				"Marker Pre-enabled\n");
> +		goto out;
> +	}
> +

Hrm, I did not notice it, but marker_info_read, as it is currently
implemented in LTTng, should take lock_markers() before iterating on the
markers. Can you provide a separate patch that fixes this ? (I don't
want to work under your feet...)

The same should be done for build_marker_control_files() and
remove_marker_control_dir().

The rest looks good. I look forward to see V3.

Thanks !

Mathieu

>  	marker_iter_reset(&iter);
>  	marker_iter_start(&iter);
>  	for (; iter.marker != NULL; marker_iter_next(&iter)) {
> @@ -872,6 +890,7 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
>  	}
>  	marker_iter_stop(&iter);
>  
> +out:
>  	if (len >= PAGE_SIZE) {
>  		len = PAGE_SIZE;
>  		buf[PAGE_SIZE] = '\0';
> @@ -887,6 +906,149 @@ static const struct file_operations info_fops = {
>  	.read = marker_info_read,
>  };
>  
> +static int marker_mkdir(struct inode *dir, struct dentry *dentry, int mode)
> +{
> +	struct dentry *marker_d, *enable_d, *info_d, *channel_d;
> +	int ret;
> +
> +	ret = 0;
> +	channel_d = (struct dentry *)dir->i_private;
> +	mutex_unlock(&dir->i_mutex);
> +
> +	marker_d = debugfs_create_dir(dentry->d_name.name,
> +				      channel_d);
> +	if (IS_ERR(marker_d)) {
> +		ret = PTR_ERR(marker_d);
> +		goto out;
> +	}
> +
> +	enable_d = debugfs_create_file("enable", 0644, marker_d,
> +				       NULL, &enable_fops);
> +	if (IS_ERR(enable_d) || !enable_d) {
> +		printk(KERN_ERR
> +		       "%s: create file of %s failed\n",
> +		       __func__, "enable");
> +		ret = -ENOMEM;
> +		goto remove_marker_dir;
> +	}
> +
> +	info_d = debugfs_create_file("info", 0644, marker_d,
> +				     NULL, &info_fops);
> +	if (IS_ERR(info_d) || !info_d) {
> +		printk(KERN_ERR
> +		       "%s: create file of %s failed\n",
> +		       __func__, "info");
> +		ret = -ENOMEM;
> +		goto remove_enable_dir;
> +	}
> +
> +	goto out;
> +
> +remove_enable_dir:
> +	debugfs_remove(enable_d);
> +remove_marker_dir:
> +	debugfs_remove(marker_d);
> +out:
> +	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
> +	return ret;
> +}
> +
> +static int marker_rmdir(struct inode *dir, struct dentry *dentry)
> +{
> +	struct dentry *marker_d, *channel_d;
> +	const char *channel, *name;
> +	int ret;
> +
> +	ret = 0;
> +
> +	channel_d = (struct dentry *)dir->i_private;
> +	channel = channel_d->d_name.name;
> +
> +	marker_d = dir_lookup(channel_d, dentry->d_name.name);
> +
> +	if (!marker_d) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	name = marker_d->d_name.name;
> +
> +	if (is_marker_present(channel, name) ||
> +	    (!is_marker_present(channel, name) &&
> +	     is_marker_enabled(channel, name))) {
> +		ret = -EPERM;
> +		goto out;
> +	}
> +
> +	mutex_unlock(&dir->i_mutex);
> +	mutex_unlock(&dentry->d_inode->i_mutex);
> +	debugfs_remove_recursive(marker_d);
> +	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
> +	mutex_lock(&dentry->d_inode->i_mutex);
> +out:
> +	return ret;
> +}
> +
> +const struct inode_operations channel_dir_opt = {
> +	.lookup = simple_lookup,
> +	.mkdir = marker_mkdir,
> +	.rmdir = marker_rmdir,
> +};
> +
> +static int channel_mkdir(struct inode *dir, struct dentry *dentry, int mode)
> +{
> +	struct dentry *channel_d;
> +	int ret;
> +
> +	ret = 0;
> +	mutex_unlock(&dir->i_mutex);
> +
> +	channel_d = debugfs_create_dir(dentry->d_name.name,
> +				       markers_control_dir);
> +	if (IS_ERR(channel_d)) {
> +		ret = PTR_ERR(channel_d);
> +		goto out;
> +	}
> +
> +	channel_d->d_inode->i_private = (void *)channel_d;
> +	init_marker_dir(channel_d, &channel_dir_opt);
> +out:
> +	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
> +	return ret;
> +}
> +
> +static int channel_rmdir(struct inode *dir, struct dentry *dentry)
> +{
> +	struct dentry *channel_d;
> +	int ret;
> +
> +	ret = 0;
> +
> +	channel_d = dir_lookup(markers_control_dir, dentry->d_name.name);
> +	if (!channel_d) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	if (list_empty(&channel_d->d_subdirs)) {
> +		mutex_unlock(&dir->i_mutex);
> +		mutex_unlock(&dentry->d_inode->i_mutex);
> +		debugfs_remove(channel_d);
> +		mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
> +		mutex_lock(&dentry->d_inode->i_mutex);
> +	} else
> +		ret = -EPERM;
> +
> +out:
> +	return ret;
> +}
> +
> +const struct inode_operations root_dir_opt = {
> +	.lookup = simple_lookup,
> +	.mkdir = channel_mkdir,
> +	.rmdir = channel_rmdir
> +};
> +
>  static int build_marker_file(struct marker *marker)
>  {
>  	struct dentry *channel_d, *marker_d, *enable_d, *info_d;
> @@ -903,6 +1065,8 @@ static int build_marker_file(struct marker *marker)
>  			err = -ENOMEM;
>  			goto err_build_fail;
>  		}
> +		channel_d->d_inode->i_private = (void *)channel_d;
> +		init_marker_dir(channel_d, &channel_dir_opt);
>  	}
>  
>  	marker_d  = dir_lookup(channel_d, marker->name);
> @@ -1118,6 +1282,8 @@ static int __init ltt_trace_control_init(void)
>  		goto err_create_marker_control_dir;
>  	}
>  
> +	init_marker_dir(markers_control_dir, &root_dir_opt);
> +
>  	if (build_marker_control_files())
>  		goto err_build_fail;
>  
> -- 
> 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 v2] LTTNG: Make marker enabled in advance

[Gui Jianfeng]

Mathieu Desnoyers wrote:
...
> Hrm, I did not notice it, but marker_info_read, as it is currently
> implemented in LTTng, should take lock_markers() before iterating on the
> markers. Can you provide a separate patch that fixes this ? (I don't
> want to work under your feet...)
> 
> The same should be done for build_marker_control_files() and
> remove_marker_control_dir().
> 
  Ok, will fix.
 
> The rest looks good. I look forward to see V3.
> 
> Thanks !
> 
> Mathieu
> 
>>  	marker_iter_reset(&iter);
>>  	marker_iter_start(&iter);
>>  	for (; iter.marker != NULL; marker_iter_next(&iter)) {
>> @@ -872,6 +890,7 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
>>  	}
>>  	marker_iter_stop(&iter);
>>  
>> +out:
>>  	if (len >= PAGE_SIZE) {
>>  		len = PAGE_SIZE;
>>  		buf[PAGE_SIZE] = '\0';
>> @@ -887,6 +906,149 @@ static const struct file_operations info_fops = {
>>  	.read = marker_info_read,
>>  };
>>  
>> +static int marker_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>> +{
>> +	struct dentry *marker_d, *enable_d, *info_d, *channel_d;
>> +	int ret;
>> +
>> +	ret = 0;
>> +	channel_d = (struct dentry *)dir->i_private;
>> +	mutex_unlock(&dir->i_mutex);
>> +
>> +	marker_d = debugfs_create_dir(dentry->d_name.name,
>> +				      channel_d);
>> +	if (IS_ERR(marker_d)) {
>> +		ret = PTR_ERR(marker_d);
>> +		goto out;
>> +	}
>> +
>> +	enable_d = debugfs_create_file("enable", 0644, marker_d,
>> +				       NULL, &enable_fops);
>> +	if (IS_ERR(enable_d) || !enable_d) {
>> +		printk(KERN_ERR
>> +		       "%s: create file of %s failed\n",
>> +		       __func__, "enable");
>> +		ret = -ENOMEM;
>> +		goto remove_marker_dir;
>> +	}
>> +
>> +	info_d = debugfs_create_file("info", 0644, marker_d,
>> +				     NULL, &info_fops);
>> +	if (IS_ERR(info_d) || !info_d) {
>> +		printk(KERN_ERR
>> +		       "%s: create file of %s failed\n",
>> +		       __func__, "info");
>> +		ret = -ENOMEM;
>> +		goto remove_enable_dir;
>> +	}
>> +
>> +	goto out;
>> +
>> +remove_enable_dir:
>> +	debugfs_remove(enable_d);
>> +remove_marker_dir:
>> +	debugfs_remove(marker_d);
>> +out:
>> +	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
>> +	return ret;
>> +}
>> +
>> +static int marker_rmdir(struct inode *dir, struct dentry *dentry)
>> +{
>> +	struct dentry *marker_d, *channel_d;
>> +	const char *channel, *name;
>> +	int ret;
>> +
>> +	ret = 0;
>> +
>> +	channel_d = (struct dentry *)dir->i_private;
>> +	channel = channel_d->d_name.name;
>> +
>> +	marker_d = dir_lookup(channel_d, dentry->d_name.name);
>> +
>> +	if (!marker_d) {
>> +		ret = -ENOENT;
>> +		goto out;
>> +	}
>> +
>> +	name = marker_d->d_name.name;
>> +
>> +	if (is_marker_present(channel, name) ||
>> +	    (!is_marker_present(channel, name) &&
>> +	     is_marker_enabled(channel, name))) {
>> +		ret = -EPERM;
>> +		goto out;
>> +	}
>> +
>> +	mutex_unlock(&dir->i_mutex);
>> +	mutex_unlock(&dentry->d_inode->i_mutex);
>> +	debugfs_remove_recursive(marker_d);
>> +	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
>> +	mutex_lock(&dentry->d_inode->i_mutex);
>> +out:
>> +	return ret;
>> +}
>> +
>> +const struct inode_operations channel_dir_opt = {
>> +	.lookup = simple_lookup,
>> +	.mkdir = marker_mkdir,
>> +	.rmdir = marker_rmdir,
>> +};
>> +
>> +static int channel_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>> +{
>> +	struct dentry *channel_d;
>> +	int ret;
>> +
>> +	ret = 0;
>> +	mutex_unlock(&dir->i_mutex);
>> +
>> +	channel_d = debugfs_create_dir(dentry->d_name.name,
>> +				       markers_control_dir);
>> +	if (IS_ERR(channel_d)) {
>> +		ret = PTR_ERR(channel_d);
>> +		goto out;
>> +	}
>> +
>> +	channel_d->d_inode->i_private = (void *)channel_d;
>> +	init_marker_dir(channel_d, &channel_dir_opt);
>> +out:
>> +	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
>> +	return ret;
>> +}
>> +
>> +static int channel_rmdir(struct inode *dir, struct dentry *dentry)
>> +{
>> +	struct dentry *channel_d;
>> +	int ret;
>> +
>> +	ret = 0;
>> +
>> +	channel_d = dir_lookup(markers_control_dir, dentry->d_name.name);
>> +	if (!channel_d) {
>> +		ret = -ENOENT;
>> +		goto out;
>> +	}
>> +
>> +	if (list_empty(&channel_d->d_subdirs)) {
>> +		mutex_unlock(&dir->i_mutex);
>> +		mutex_unlock(&dentry->d_inode->i_mutex);
>> +		debugfs_remove(channel_d);
>> +		mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
>> +		mutex_lock(&dentry->d_inode->i_mutex);
>> +	} else
>> +		ret = -EPERM;
>> +
>> +out:
>> +	return ret;
>> +}
>> +
>> +const struct inode_operations root_dir_opt = {
>> +	.lookup = simple_lookup,
>> +	.mkdir = channel_mkdir,
>> +	.rmdir = channel_rmdir
>> +};
>> +
>>  static int build_marker_file(struct marker *marker)
>>  {
>>  	struct dentry *channel_d, *marker_d, *enable_d, *info_d;
>> @@ -903,6 +1065,8 @@ static int build_marker_file(struct marker *marker)
>>  			err = -ENOMEM;
>>  			goto err_build_fail;
>>  		}
>> +		channel_d->d_inode->i_private = (void *)channel_d;
>> +		init_marker_dir(channel_d, &channel_dir_opt);
>>  	}
>>  
>>  	marker_d  = dir_lookup(channel_d, marker->name);
>> @@ -1118,6 +1282,8 @@ static int __init ltt_trace_control_init(void)
>>  		goto err_create_marker_control_dir;
>>  	}
>>  
>> +	init_marker_dir(markers_control_dir, &root_dir_opt);
>> +
>>  	if (build_marker_control_files())
>>  		goto err_build_fail;
>>  
>> -- 
>> 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 v2] LTTNG: Make marker enabled in advance

[Gui Jianfeng]

Mathieu Desnoyers wrote:
> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>> This patch makes marker enabled in advance.
>> IOW, even if the marker isn't present for the moment,
>> you can still enable it for future use.
>> As soon as the marker is inserted into kernel, tracing 
>> work can be started immediately.
>>
>> Here is an example for using the user interface:
>> This patch assumes the marker control patch is applied.
>> [root at localhost markers]# cd /mnt/debugfs/ltt/markers
>> [root at localhost markers]# mkdir fs
>> [root at localhost markers]# cd fs/
>> [root at localhost fs]# mkdir close
>> [root at localhost fs]# cd close/
>> [root at localhost close]# echo 1 > enable
>> [root at localhost close]# cat enable
>> 2
>> [root at localhost close]# cat info
>> Marker Pre-enabled
>> [root at localhost close]# modprobe fs-trace
>> [root at localhost close]# cat enable
>> 1
>> [root at localhost close]# cat info
>> Location: fs_trace
>> format: "fd %u"
>> state: 1
>> event_id: 0
>> call: 0xc0468192
>> probe single : 0xc0520ed8
>>
>> You can also remove a marker directory by "rmdir" if there
>> is no actual marker backed and isn't enabled in advance. 
>> A channel directory can be remove if there are not any marker
>> directories in it.
>>
>> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
>> ---
>>  include/linux/marker.h  |    1 +
>>  kernel/marker.c         |   29 ++++++++
>>  ltt/ltt-trace-control.c |  168 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 197 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/marker.h b/include/linux/marker.h
>> index 2d78113..692805c 100644
>> --- a/include/linux/marker.h
>> +++ b/include/linux/marker.h
>> @@ -261,5 +261,6 @@ extern void marker_iter_reset(struct marker_iter *iter);
>>  extern int marker_get_iter_range(struct marker **marker, struct marker *begin,
>>  	struct marker *end);
>>  extern int is_marker_enabled(const char *channel, const char *name);
>> +extern int is_marker_present(const char *channel, const char *name);
>>  
>>  #endif
>> diff --git a/kernel/marker.c b/kernel/marker.c
>> index 7d11393..0c4d5c9 100644
>> --- a/kernel/marker.c
>> +++ b/kernel/marker.c
>> @@ -651,6 +651,35 @@ static void disable_marker(struct marker *elem)
>>  }
>>  
>>  /*
>> + * is_marker_present - Check if a marker is present in kernel
>> + * @channel: channel name
>> + * @name: marker name
>> + *
>> + * Returns 1 if the marker is present, 0 if not.
>> + */
>> +int is_marker_present(const char *channel, const char *name)
>> +{
>> +	int ret;
>> +	struct marker_iter iter;
>> +
>> +	ret = 0;
> 
> Shouldn't this take a markers_mutex ?

  Does this place really need to take markers_mutex?
  IMHO, This mutex is used to protect the hash table, am i right.
  
> 
>> +	marker_iter_reset(&iter);
>> +	marker_iter_start(&iter);
>> +	for (; iter.marker != NULL; marker_iter_next(&iter)) {
>> +		if (!strcmp(iter.marker->channel, channel) &&
>> +		    !strcmp(iter.marker->name, name)) {
>> +			ret = 1;
>> +			goto end;
>> +		}
>> +	}
>> +	marker_iter_stop(&iter);
>> +
> 
>> +end:
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(is_marker_present);
>> +
>> +/*
>>   * is_marker_enabled - Check if a marker is enabled
>>   * @channel: channel name
>>   * @name: marker name
>> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
>> index c1c44c6..050c49f 100644
>> --- a/ltt/ltt-trace-control.c
>> +++ b/ltt/ltt-trace-control.c
>> @@ -756,6 +756,12 @@ static const struct file_operations ltt_destroy_trace_operations = {
>>  	.write = destroy_trace_write,
>>  };
>>  
>> +static void init_marker_dir(struct dentry *dentry,
>> +			    const struct inode_operations *opt)
>> +{
>> +	dentry->d_inode->i_op = opt;
>> +}
>> +
>>  static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
>>  			    size_t cnt, loff_t *ppos)
>>  {
>> @@ -769,10 +775,15 @@ static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
>>  	len = 0;
>>  	buf = (char *)__get_free_page(GFP_KERNEL);
>>  
>> -	if (is_marker_enabled(channel, marker))
>> +	if (is_marker_enabled(channel, marker) &&
>> +	    is_marker_present(channel, marker))
>>  		len = snprintf(buf, PAGE_SIZE, "%d\n", 1);
>> +	else if (is_marker_enabled(channel, marker) &&
>> +		 !is_marker_present(channel, marker))
>> +		len = snprintf(buf, PAGE_SIZE, "%d\n", 2);
>>  	else
>>  		len = snprintf(buf, PAGE_SIZE, "%d\n", 0);
>> +
>>  	if (len >= PAGE_SIZE) {
>>  		len = PAGE_SIZE;
>>  		buf[PAGE_SIZE] = '\0';
>> @@ -846,6 +857,13 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
>>  	len = 0;
>>  	buf = (char *)__get_free_page(GFP_KERNEL);
>>  
>> +	if (is_marker_enabled(channel, marker) &&
>> +	    !is_marker_present(channel, marker)) {
>> +		len += snprintf(buf + len, PAGE_SIZE - len,
>> +				"Marker Pre-enabled\n");
>> +		goto out;
>> +	}
>> +
> 
> Hrm, I did not notice it, but marker_info_read, as it is currently
> implemented in LTTng, should take lock_markers() before iterating on the
> markers. Can you provide a separate patch that fixes this ? (I don't
> want to work under your feet...)

  Here is same, does it really need?

> 
> The same should be done for build_marker_control_files() and
> remove_marker_control_dir().

-- 
Regards
Gui Jianfeng

[ltt-dev] [PATCH v2] LTTNG: Make marker enabled in advance

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> >> This patch makes marker enabled in advance.
> >> IOW, even if the marker isn't present for the moment,
> >> you can still enable it for future use.
> >> As soon as the marker is inserted into kernel, tracing 
> >> work can be started immediately.
> >>
> >> Here is an example for using the user interface:
> >> This patch assumes the marker control patch is applied.
> >> [root at localhost markers]# cd /mnt/debugfs/ltt/markers
> >> [root at localhost markers]# mkdir fs
> >> [root at localhost markers]# cd fs/
> >> [root at localhost fs]# mkdir close
> >> [root at localhost fs]# cd close/
> >> [root at localhost close]# echo 1 > enable
> >> [root at localhost close]# cat enable
> >> 2
> >> [root at localhost close]# cat info
> >> Marker Pre-enabled
> >> [root at localhost close]# modprobe fs-trace
> >> [root at localhost close]# cat enable
> >> 1
> >> [root at localhost close]# cat info
> >> Location: fs_trace
> >> format: "fd %u"
> >> state: 1
> >> event_id: 0
> >> call: 0xc0468192
> >> probe single : 0xc0520ed8
> >>
> >> You can also remove a marker directory by "rmdir" if there
> >> is no actual marker backed and isn't enabled in advance. 
> >> A channel directory can be remove if there are not any marker
> >> directories in it.
> >>
> >> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
> >> ---
> >>  include/linux/marker.h  |    1 +
> >>  kernel/marker.c         |   29 ++++++++
> >>  ltt/ltt-trace-control.c |  168 ++++++++++++++++++++++++++++++++++++++++++++++-
> >>  3 files changed, 197 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/linux/marker.h b/include/linux/marker.h
> >> index 2d78113..692805c 100644
> >> --- a/include/linux/marker.h
> >> +++ b/include/linux/marker.h
> >> @@ -261,5 +261,6 @@ extern void marker_iter_reset(struct marker_iter *iter);
> >>  extern int marker_get_iter_range(struct marker **marker, struct marker *begin,
> >>  	struct marker *end);
> >>  extern int is_marker_enabled(const char *channel, const char *name);
> >> +extern int is_marker_present(const char *channel, const char *name);
> >>  
> >>  #endif
> >> diff --git a/kernel/marker.c b/kernel/marker.c
> >> index 7d11393..0c4d5c9 100644
> >> --- a/kernel/marker.c
> >> +++ b/kernel/marker.c
> >> @@ -651,6 +651,35 @@ static void disable_marker(struct marker *elem)
> >>  }
> >>  
> >>  /*
> >> + * is_marker_present - Check if a marker is present in kernel
> >> + * @channel: channel name
> >> + * @name: marker name
> >> + *
> >> + * Returns 1 if the marker is present, 0 if not.
> >> + */
> >> +int is_marker_present(const char *channel, const char *name)
> >> +{
> >> +	int ret;
> >> +	struct marker_iter iter;
> >> +
> >> +	ret = 0;
> > 
> > Shouldn't this take a markers_mutex ?
> 
>   Does this place really need to take markers_mutex?
>   IMHO, This mutex is used to protect the hash table, am i right.
>   
> > 
> >> +	marker_iter_reset(&iter);
> >> +	marker_iter_start(&iter);
> >> +	for (; iter.marker != NULL; marker_iter_next(&iter)) {
> >> +		if (!strcmp(iter.marker->channel, channel) &&
> >> +		    !strcmp(iter.marker->name, name)) {
> >> +			ret = 1;
> >> +			goto end;
> >> +		}
> >> +	}
> >> +	marker_iter_stop(&iter);
> >> +
> > 
> >> +end:
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(is_marker_present);
> >> +
> >> +/*
> >>   * is_marker_enabled - Check if a marker is enabled
> >>   * @channel: channel name
> >>   * @name: marker name
> >> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
> >> index c1c44c6..050c49f 100644
> >> --- a/ltt/ltt-trace-control.c
> >> +++ b/ltt/ltt-trace-control.c
> >> @@ -756,6 +756,12 @@ static const struct file_operations ltt_destroy_trace_operations = {
> >>  	.write = destroy_trace_write,
> >>  };
> >>  
> >> +static void init_marker_dir(struct dentry *dentry,
> >> +			    const struct inode_operations *opt)
> >> +{
> >> +	dentry->d_inode->i_op = opt;
> >> +}
> >> +
> >>  static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
> >>  			    size_t cnt, loff_t *ppos)
> >>  {
> >> @@ -769,10 +775,15 @@ static ssize_t marker_enable_read(struct file *filp, char __user *ubuf,
> >>  	len = 0;
> >>  	buf = (char *)__get_free_page(GFP_KERNEL);
> >>  
> >> -	if (is_marker_enabled(channel, marker))
> >> +	if (is_marker_enabled(channel, marker) &&
> >> +	    is_marker_present(channel, marker))
> >>  		len = snprintf(buf, PAGE_SIZE, "%d\n", 1);
> >> +	else if (is_marker_enabled(channel, marker) &&
> >> +		 !is_marker_present(channel, marker))
> >> +		len = snprintf(buf, PAGE_SIZE, "%d\n", 2);
> >>  	else
> >>  		len = snprintf(buf, PAGE_SIZE, "%d\n", 0);
> >> +
> >>  	if (len >= PAGE_SIZE) {
> >>  		len = PAGE_SIZE;
> >>  		buf[PAGE_SIZE] = '\0';
> >> @@ -846,6 +857,13 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
> >>  	len = 0;
> >>  	buf = (char *)__get_free_page(GFP_KERNEL);
> >>  
> >> +	if (is_marker_enabled(channel, marker) &&
> >> +	    !is_marker_present(channel, marker)) {
> >> +		len += snprintf(buf + len, PAGE_SIZE - len,
> >> +				"Marker Pre-enabled\n");
> >> +		goto out;
> >> +	}
> >> +
> > 
> > Hrm, I did not notice it, but marker_info_read, as it is currently
> > implemented in LTTng, should take lock_markers() before iterating on the
> > markers. Can you provide a separate patch that fixes this ? (I don't
> > want to work under your feet...)
> 
>   Here is same, does it really need?
> 

The marker mutex is also used in marker_update_probe_range(), called
upon module load/unload, to insure consistency of the module sections.
So it's needed everywhere where the marker sections are read.

Mathieu

> > 
> > The same should be done for build_marker_control_files() and
> > remove_marker_control_dir().
> 
> -- 
> Regards
> Gui Jianfeng
> 

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

[ltt-dev] [PATCH] LTTNG: hold markers_mutex during marker section iteration

[Gui Jianfeng]

Hold the markers_mutex during marker section iteration.

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

diff --git a/kernel/marker.c b/kernel/marker.c
index 7d11393..93ffa8e 100644
--- a/kernel/marker.c
+++ b/kernel/marker.c
@@ -44,11 +44,13 @@ void lock_markers(void)
 {
 	mutex_lock(&markers_mutex);
 }
+EXPORT_SYMBOL_GPL(lock_markers);
 
 void unlock_markers(void)
 {
 	mutex_unlock(&markers_mutex);
 }
+EXPORT_SYMBOL_GPL(unlock_markers);
 
 /*
  * Marker hash table, containing the active markers.
diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
index c1c44c6..63cbb29 100644
--- a/ltt/ltt-trace-control.c
+++ b/ltt/ltt-trace-control.c
@@ -846,6 +846,8 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
 	len = 0;
 	buf = (char *)__get_free_page(GFP_KERNEL);
 
+	lock_markers();
+
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
 	for (; iter.marker != NULL; marker_iter_next(&iter)) {
@@ -872,6 +874,8 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
 	}
 	marker_iter_stop(&iter);
 
+	unlock_markers();
+
 	if (len >= PAGE_SIZE) {
 		len = PAGE_SIZE;
 		buf[PAGE_SIZE] = '\0';
@@ -958,6 +962,8 @@ static int build_marker_control_files(void)
 	if (!markers_control_dir)
 		return -EEXIST;
 
+	lock_markers();
+
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
 	for (; iter.marker != NULL; marker_iter_next(&iter)) {
@@ -966,6 +972,9 @@ static int build_marker_control_files(void)
 			goto err_build_fail;
 	}
 	marker_iter_stop(&iter);
+
+	unlock_markers();
+
 	return 0;
 
 err_build_fail:
@@ -991,6 +1000,8 @@ static int remove_marker_control_dir(struct module *mod, struct marker *marker)
 		return -ENOENT;
 	name = marker_d->d_name.name;
 
+	lock_markers();
+
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
 	for (; iter.marker != NULL; marker_iter_next(&iter)) {
@@ -1008,6 +1019,7 @@ static int remove_marker_control_dir(struct module *mod, struct marker *marker)
 
 end:
 	marker_iter_stop(&iter);
+	unlock_markers();
 	return 0;
 }
 
-- 
1.5.4.rc3

[ltt-dev] [PATCH] LTTNG: hold markers_mutex during marker section iteration

[Mathieu Desnoyers]

* Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
> Hold the markers_mutex during marker section iteration.
> 

Will merge, thanks !

Mathieu

> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
> ---
>  kernel/marker.c         |    2 ++
>  ltt/ltt-trace-control.c |   12 ++++++++++++
>  2 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/marker.c b/kernel/marker.c
> index 7d11393..93ffa8e 100644
> --- a/kernel/marker.c
> +++ b/kernel/marker.c
> @@ -44,11 +44,13 @@ void lock_markers(void)
>  {
>  	mutex_lock(&markers_mutex);
>  }
> +EXPORT_SYMBOL_GPL(lock_markers);
>  
>  void unlock_markers(void)
>  {
>  	mutex_unlock(&markers_mutex);
>  }
> +EXPORT_SYMBOL_GPL(unlock_markers);
>  
>  /*
>   * Marker hash table, containing the active markers.
> diff --git a/ltt/ltt-trace-control.c b/ltt/ltt-trace-control.c
> index c1c44c6..63cbb29 100644
> --- a/ltt/ltt-trace-control.c
> +++ b/ltt/ltt-trace-control.c
> @@ -846,6 +846,8 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
>  	len = 0;
>  	buf = (char *)__get_free_page(GFP_KERNEL);
>  
> +	lock_markers();
> +
>  	marker_iter_reset(&iter);
>  	marker_iter_start(&iter);
>  	for (; iter.marker != NULL; marker_iter_next(&iter)) {
> @@ -872,6 +874,8 @@ static ssize_t marker_info_read(struct file *filp, char __user *ubuf,
>  	}
>  	marker_iter_stop(&iter);
>  
> +	unlock_markers();
> +
>  	if (len >= PAGE_SIZE) {
>  		len = PAGE_SIZE;
>  		buf[PAGE_SIZE] = '\0';
> @@ -958,6 +962,8 @@ static int build_marker_control_files(void)
>  	if (!markers_control_dir)
>  		return -EEXIST;
>  
> +	lock_markers();
> +
>  	marker_iter_reset(&iter);
>  	marker_iter_start(&iter);
>  	for (; iter.marker != NULL; marker_iter_next(&iter)) {
> @@ -966,6 +972,9 @@ static int build_marker_control_files(void)
>  			goto err_build_fail;
>  	}
>  	marker_iter_stop(&iter);
> +
> +	unlock_markers();
> +
>  	return 0;
>  
>  err_build_fail:
> @@ -991,6 +1000,8 @@ static int remove_marker_control_dir(struct module *mod, struct marker *marker)
>  		return -ENOENT;
>  	name = marker_d->d_name.name;
>  
> +	lock_markers();
> +
>  	marker_iter_reset(&iter);
>  	marker_iter_start(&iter);
>  	for (; iter.marker != NULL; marker_iter_next(&iter)) {
> @@ -1008,6 +1019,7 @@ static int remove_marker_control_dir(struct module *mod, struct marker *marker)
>  
>  end:
>  	marker_iter_stop(&iter);
> +	unlock_markers();
>  	return 0;
>  }
>  
> -- 
> 1.5.4.rc3
> 
> 
> 

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