[ltt-dev] [PATCH v2] LTTNG: Make marker enabled in advance
Mathieu Desnoyers
compudj at krystal.dyndns.org
Fri Mar 13 22:49:14 EDT 2009
* 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
More information about the lttng-dev
mailing list