lkml.org 
[lkml]   [2009]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 7/7] tracing: add hierarchical enabling of events
> With the current event directory, you can only enable individual events.
> The file debugfs/tracing/set_event is used to be able to enable or
> disable several events at once. But that can still be awkward.
>
> This patch adds hierarchical enabling of events. That is, each directory
> in debugfs/tracing/events has an "enable" file. This file can enable
> or disable all events within the directory and below.
>
> # echo 1 > /debugfs/tracing/events/enable
>
> will enable all events.
>
> # echo 1 > /debugfs/tracing/events/sched/enable
>
> will enable all events in the sched subsystem.
>
> # echo 1 > /debugfs/tracing/events/enable
> # echo 0 > /debugfs/tracing/events/irq/enable
>
> will enable all events, but then disable just the irq subsystem events.
>
> When reading one of these enable files, there are four results:
>
> 0 - all events this file affects are disabled
> 1 - all events this file affects are enabled
> X - there is a mixture of events enabled and disabled
> ? - this file does not affect any event

I would expect reading an enable file will let me know exactly
which events are disabled and which are enabled.

I think this is useful especially for events/system/enable.

Like this:

$ cat events/irq/enable
0 irq_handler_entry
0 irq_handler_exit
1 softirq_entry
1 softirq_exit

> +static ssize_t
> +system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
> + loff_t *ppos)
> +{
> + const char *system = filp->private_data;
> + struct ftrace_event_call *call;
> + char buf[2];
> + int set = -1;
> + int all = 0;
> + int ret;
> +
> + if (system[0] == '*')
> + all = 1;
> +
> + mutex_lock(&event_mutex);
> + list_for_each_entry(call, &ftrace_events, list) {
> + if (!call->name || !call->regfunc)
> + continue;
> +
> + if (!all && strcmp(call->system, system) != 0)
> + continue;
> +
> + /*
> + * We need to find out if all the events are set
> + * or if all events or cleared, or if we have
> + * a mixture.
> + */
> + if (call->enabled) {
> + switch (set) {
> + case -1:
> + set = 1;
> + break;
> + case 0:
> + set = 2;
> + break;
> + }
> + } else {
> + switch (set) {
> + case -1:
> + set = 0;
> + break;
> + case 1:
> + set = 2;
> + break;
> + }
> + }
> + /*
> + * If we have a mixture, no need to look further.
> + */
> + if (set == 2)
> + break;

How about:

int set = 0;

...
set |= (1 << call->enabled);
...

set == 0: '?'
set == 1: '0'
set == 2: '1'
set == 3: 'X'

Will this make the code simpler? :)

Or we can go even further:

char result[4] = { '?', '0', '1', 'X' };
...
buf[0] = result[set];

> + }
> + mutex_unlock(&event_mutex);
> +
> + buf[1] = '\n';
> + switch (set) {
> + case 0:
> + buf[0] = '0';
> + break;
> + case 1:
> + buf[0] = '1';
> + break;
> + case 2:
> + buf[0] = 'X';
> + break;
> + default:
> + buf[0] = '?';
> + }
> +
> + ret = simple_read_from_buffer(ubuf, cnt, ppos, buf, 2);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +system_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
> + loff_t *ppos)
> +{
> + const char *system = filp->private_data;
> + unsigned long val;
> + char *command;
> + char buf[64];
> + ssize_t ret;
> +
> + if (cnt >= sizeof(buf))
> + return -EINVAL;
> +
> + if (copy_from_user(&buf, ubuf, cnt))
> + return -EFAULT;
> +
> + buf[cnt] = 0;
> +
> + ret = strict_strtoul(buf, 10, &val);
> + if (ret < 0)
> + return ret;
> +
> + ret = tracing_update_buffers();
> + if (ret < 0)
> + return ret;
> +
> + switch (val) {
> + case 0:
> + case 1:
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + command = kstrdup(system, GFP_KERNEL);
> + if (!command)
> + return -ENOMEM;
> +
> + ret = ftrace_set_clr_event(command, val);

I think we should pass "sched:" or "sched:*", instead of "sched",
the comment in ftrace_set_clr_event():

* <name> (no ':') means all events in a subsystem with
* the name <name> or any event that matches <name>

> + if (ret)
> + goto out_free;
> +
> + ret = cnt;
> +
> + out_free:
> + kfree(command);
> +
> + *ppos += cnt;
> +
> + return ret;
> +}
> +



\
 
 \ /
  Last update: 2009-05-07 05:53    [W:1.242 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site