[PATCH] tracing: Allow changing default ring buffer size for ftrace instances.

[PATCH] tracing: Allow changing default ring buffer size for ftrace instances.

[Bharath Ravi]

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
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);
 
-- 
1.8.4.1

Re: [PATCH] tracing: Allow changing default ring buffer size for ftrace instances.

[Steven Rostedt]

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);
>