Messages in this thread | | | Date | Mon, 11 Nov 2013 16:52:04 -0500 | From | Steven Rostedt <> | 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@google.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@google.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); >
| |