From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Wed, 4 Feb 2009 02:12:55 -0500 Subject: [ltt-dev] [PATCH] LTTng: Make mark-control work in debugfs In-Reply-To: <49893957.20303@cn.fujitsu.com> References: <4988F393.8080605@cn.fujitsu.com> <20090204062051.GB10772@Krystal> <49893957.20303@cn.fujitsu.com> Message-ID: <20090204071255.GA12864@Krystal> * 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 Signed-off-by: Mathieu Desnoyers --- 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 + * Copyright 2008 - Zhaolei + * + * Copyright 2009 - Gui Jianfeng + * Make mark-control work in debugfs */ /* @@ -16,14 +18,17 @@ #include #include #include +#include #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