| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 25 Aug 2009 22:35:30 -0400 (EDT)
From: Steven Rostedt <rostedt@...dmis.org>
To: Zhaolei <zhaolei@...fujitsu.com>
cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Ingo Molnar <mingo@...e.hu>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] ftrace: Move setting of clock-source out of
options
Hi,
I like the series and I'll pull it, but there's one thing that bothers me.
We should fix it too...
On Tue, 25 Aug 2009, Zhaolei wrote:
> There are many clock source for tracing system but we can only
> enable/disable one bit by trace/options file.
> We can move setting of clock-source out of options and add a separate
> file for it:
> # cat trace_clock
> [local] global
> # echo global > trace_clock
> # cat trace_clock
> local [global]
>
> Signed-off-by: Zhao Lei <zhaolei@...fujitsu.com>
> ---
> kernel/trace/trace.c | 92 ++++++++++++++++++++++++++++++++++++++++---------
> kernel/trace/trace.h | 7 ++--
> 2 files changed, 79 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index b7d873b..5af6f83 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -323,12 +323,21 @@ static const char *trace_options[] = {
> "printk-msg-only",
> "context-info",
> "latency-format",
> - "global-clock",
> "sleep-time",
> "graph-time",
> NULL
> };
>
> +static struct {
> + u64 (*func)(void);
> + const char *name;
> +} trace_clocks[] = {
> + { trace_clock_local, "local" },
> + { trace_clock_global, "global" },
> +};
> +
> +int trace_clock_id;
> +
> /*
> * ftrace_max_lock is used to protect the swapping of buffers
> * when taking a max snapshot. The buffers themselves are
> @@ -2159,22 +2168,6 @@ static void set_tracer_flags(unsigned int mask, int enabled)
> trace_flags |= mask;
> else
> trace_flags &= ~mask;
> -
> - if (mask == TRACE_ITER_GLOBAL_CLK) {
> - u64 (*func)(void);
> -
> - if (enabled)
> - func = trace_clock_global;
> - else
> - func = trace_clock_local;
> -
> - mutex_lock(&trace_types_lock);
> - ring_buffer_set_clock(global_trace.buffer, func);
> -
> - if (max_tr.buffer)
> - ring_buffer_set_clock(max_tr.buffer, func);
> - mutex_unlock(&trace_types_lock);
> - }
> }
>
> static ssize_t
> @@ -3142,6 +3135,62 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> return cnt;
> }
>
> +static ssize_t tracing_clock_read(struct file *filp, char __user *ubuf,
> + size_t cnt, loff_t *ppos)
> +{
> + char buf[64];
What happens if we add more clocks. Our list can only be 64 bytes long. It
would be a good idea to change this to do something like the trace_options
uses. That is, it dynamically allocates buf, by figuring out how much it
needs. This should do that too.
Thanks,
-- Steve
> + int bufiter = 0;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(trace_clocks); i++)
> + bufiter += snprintf(buf + bufiter, sizeof(buf) - bufiter,
> + "%s%s%s%s", i ? " " : "",
> + i == trace_clock_id ? "[" : "", trace_clocks[i].name,
> + i == trace_clock_id ? "]" : "");
> + bufiter += snprintf(buf + bufiter, sizeof(buf) - bufiter, "\n");
> +
> + return simple_read_from_buffer(ubuf, cnt, ppos, buf, bufiter);
> +}
> +
> +static ssize_t tracing_clock_write(struct file *filp, const char __user *ubuf,
> + size_t cnt, loff_t *fpos)
> +{
> + char buf[64];
> + const char *clockstr;
> + int i;
> +
> + if (cnt >= sizeof(buf))
> + return -EINVAL;
> +
> + if (copy_from_user(&buf, ubuf, cnt))
> + return -EFAULT;
> +
> + buf[cnt] = 0;
> +
> + clockstr = strstrip(buf);
> +
> + for (i = 0; i < ARRAY_SIZE(trace_clocks); i++) {
> + if (strcmp(trace_clocks[i].name, clockstr) == 0)
> + break;
> + }
> + if (i == ARRAY_SIZE(trace_clocks))
> + return -EINVAL;
> +
> + trace_clock_id = i;
> +
> + mutex_lock(&trace_types_lock);
> +
> + ring_buffer_set_clock(global_trace.buffer, trace_clocks[i].func);
> + if (max_tr.buffer)
> + ring_buffer_set_clock(max_tr.buffer, trace_clocks[i].func);
> +
> + mutex_unlock(&trace_types_lock);
> +
> + *fpos += cnt;
> +
> + return cnt;
> +}
> +
> static const struct file_operations tracing_max_lat_fops = {
> .open = tracing_open_generic,
> .read = tracing_max_lat_read,
> @@ -3179,6 +3228,12 @@ static const struct file_operations tracing_mark_fops = {
> .write = tracing_mark_write,
> };
>
> +static const struct file_operations trace_clock_fops = {
> + .open = tracing_open_generic,
> + .read = tracing_clock_read,
> + .write = tracing_clock_write,
> +};
> +
> struct ftrace_buffer_info {
> struct trace_array *tr;
> void *spare;
> @@ -3910,6 +3965,9 @@ static __init int tracer_init_debugfs(void)
> trace_create_file("saved_cmdlines", 0444, d_tracer,
> NULL, &tracing_saved_cmdlines_fops);
>
> + trace_create_file("trace_clock", 0644, d_tracer, NULL,
> + &trace_clock_fops);
> +
> #ifdef CONFIG_DYNAMIC_FTRACE
> trace_create_file("dyn_ftrace_total_info", 0444, d_tracer,
> &ftrace_update_tot_cnt, &tracing_dyn_info_fops);
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 606073c..0545c2b 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -586,6 +586,8 @@ trace_vprintk(unsigned long ip, const char *fmt, va_list args);
>
> extern unsigned long trace_flags;
>
> +extern int trace_clock_id;
> +
> /* Standard output formatting function used for function return traces */
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> extern enum print_line_t print_graph_function(struct trace_iterator *iter);
> @@ -674,9 +676,8 @@ enum trace_iterator_flags {
> TRACE_ITER_PRINTK_MSGONLY = 0x10000,
> TRACE_ITER_CONTEXT_INFO = 0x20000, /* Print pid/cpu/time */
> TRACE_ITER_LATENCY_FMT = 0x40000,
> - TRACE_ITER_GLOBAL_CLK = 0x80000,
> - TRACE_ITER_SLEEP_TIME = 0x100000,
> - TRACE_ITER_GRAPH_TIME = 0x200000,
> + TRACE_ITER_SLEEP_TIME = 0x80000,
> + TRACE_ITER_GRAPH_TIME = 0x100000,
> };
>
> /*
> --
> 1.5.5.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists