| 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: Mon, 11 Nov 2013 16:52:04 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Bharath Ravi <rbharath@...gle.com>
Cc: Vaibhav Nagarnaik <vnagarnaik@...gle.com>,
David Sharp <dhsharp@...gle.com>,
Laurent Chavey <chavey@...gle.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: Allow changing default ring buffer size for
ftrace instances.
On Wed, 6 Nov 2013 16:43:02 -0800
Bharath Ravi <rbharath@...gle.com> wrote:
> It is often memory efficient to start instances off with a smaller ring
> buffer size than the current default. This is particularly true on
> systems with many cores, or when multiple ftrace instances are created,
> where the current (higher) default value results in allocation failures.
>
> To allow instances to start off with custom ring buffer sizes, a new
> debugfs file named "instance_default_buffer_size_kb" is added in the
I don't mind the patch, but I hate the interface.
"instance_default_buffer_size_kb", can we get a bigger name?
"instance_buffer_size_kb" may be sufficient, even though I don't even
care for that.
I wonder if we add it to the tracing/options directory, if that would
be better. Grant you, this would be the first option that's not an on
off switch, but I'm not sure that's a bad thing.
Have any better ideas?
-- Steve
> root ftrace directory. A size in kilobytes may be written to this. Any
> ftrace instances created will be initialized with ring buffers of the
> size specified in this file.
>
> This change also modifies allocate_trace_buffer() to use the "size"
> parameter while setting set_buffer_entries(), instead of the actual ring
> ring buffer size (which is "size" rounded up to the nearest page size).
> This makes it consistent with the way writing to the buffer_size_kb
> files works (using ring_buffer_resize()), and ensures that the
> buffer_size_kb value of a newly created instance matches that written
> to the instance_default_buffer_size file, instead of the rounded up
> value.
>
> Tested: Booted into a kernel with these changes, verified reading and
> writing to the new file. Ensured that new ftrace instances created
> used the size specified in instance_default_buffer_size_kb as their
> initial ring buffer size.
>
> Signed-off-by: Bharath Ravi <rbharath@...gle.com>
> ---
> kernel/trace/trace.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 7974ba2..d10afea 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -296,6 +296,14 @@ int tracing_is_enabled(void)
>
> static unsigned long trace_buf_size = TRACE_BUF_SIZE_DEFAULT;
>
> +/*
> + * instance_default_buf_size is the size in bytes that is allocated for an
> + * instance's buffer when it is first created. This is initially set to
> + * TRACE_BUF_SIZE_DEFAULT but may be modified by writing to the
> + * "instance_default_buffer_size_kb" file.
> + */
> +static unsigned long instance_default_buf_size = TRACE_BUF_SIZE_DEFAULT;
> +
> /* trace_types holds a link list of available tracers. */
> static struct tracer *trace_types __read_mostly;
>
> @@ -3165,6 +3173,48 @@ static const struct file_operations show_traces_fops = {
> .llseek = seq_lseek,
> };
>
> +static ssize_t
> +instance_default_buffer_size_read(struct file *filp, char __user *ubuf,
> + size_t cnt, loff_t *ppos)
> +{
> + int r = 0;
> + char buf[64];
> + ssize_t ret;
> +
> + r = sprintf(buf, "%lu\n", instance_default_buf_size >> 10);
> + ret = simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +instance_default_buffer_size_write(struct file *filp, const char __user *ubuf,
> + size_t cnt, loff_t *ppos)
> +{
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
> + if (ret)
> + return ret;
> +
> + /* ubuf must have at least 1 entry */
> + if (!val)
> + return -EINVAL;
> +
> + /* val is in KB */
> + instance_default_buf_size = val << 10;
> +
> + return cnt;
> +}
> +
> +static const struct file_operations instance_default_buffer_size_fops = {
> + .open = tracing_open_generic,
> + .read = instance_default_buffer_size_read,
> + .write = instance_default_buffer_size_write,
> + .llseek = generic_file_llseek,
> +};
> +
> /*
> * The tracer itself will not take this lock, but still we want
> * to provide a consistent cpumask to user-space:
> @@ -5880,8 +5930,7 @@ allocate_trace_buffer(struct trace_array *tr, struct trace_buffer *buf, int size
> }
>
> /* Allocate the first page for all buffers */
> - set_buffer_entries(&tr->trace_buffer,
> - ring_buffer_size(tr->trace_buffer.buffer, 0));
> + set_buffer_entries(&tr->trace_buffer, size);
>
> return 0;
> }
> @@ -5947,7 +5996,7 @@ static int new_instance_create(const char *name)
> INIT_LIST_HEAD(&tr->systems);
> INIT_LIST_HEAD(&tr->events);
>
> - if (allocate_trace_buffers(tr, trace_buf_size) < 0)
> + if (allocate_trace_buffers(tr, instance_default_buf_size) < 0)
> goto out_free_tr;
>
> tr->dir = debugfs_create_dir(name, trace_instance_dir);
> @@ -6177,6 +6226,8 @@ static __init int tracer_init_debugfs(void)
> trace_create_file("dyn_ftrace_total_info", 0444, d_tracer,
> &ftrace_update_tot_cnt, &tracing_dyn_info_fops);
> #endif
> + trace_create_file("instance_default_buffer_size_kb", 0444, d_tracer,
> + NULL, &instance_default_buffer_size_fops);
>
> create_trace_instances(d_tracer);
>
--
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