[PATCH] ftrace: add 'version' special file for ring buffer format

[PATCH] ftrace: add 'version' special file for ring buffer format

[Slava Pestov]

The intention here is that if the ftrace ring buffer format changes
in the future like it did in the past with ktrace, we can increment
the version number and update clients appropriately.

Tested: Verified that special file contains correct content

This patch is against 2.6.38-rc8 but should apply to any version
from at least 2.6.34 onwards.

Signed-Off-By: Slava Pestov <slavapestov@google.com>
---
 kernel/trace/trace.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index dc53ecb..e56868a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -44,6 +44,12 @@
 #define TRACE_BUFFER_FLAGS	(RB_FL_OVERWRITE)
 
 /*
+ * The version number should be incremented if the ring buffer
+ * format changes in an incompatible way.
+ */
+static const char ftrace_version[] = "1.0.0\n";
+
+/*
  * On boot up, the ring buffer is set to the minimum size, so that
  * we do not waste memory on systems that are not using tracing.
  */
@@ -2624,6 +2630,20 @@ static const struct file_operations tracing_readme_fops = {
 };
 
 static ssize_t
+tracing_version_read(struct file *filp, char __user *ubuf,
+		     size_t cnt, loff_t *ppos)
+{
+	return simple_read_from_buffer(ubuf, cnt, ppos,
+				       ftrace_version, strlen(ftrace_version));
+}
+
+static const struct file_operations tracing_version_fops = {
+	.open		= tracing_open_generic,
+	.read		= tracing_version_read,
+	.llseek		= generic_file_llseek,
+};
+
+static ssize_t
 tracing_saved_cmdlines_read(struct file *file, char __user *ubuf,
 				size_t cnt, loff_t *ppos)
 {
@@ -4336,6 +4356,9 @@ static __init int tracer_init_debugfs(void)
 	trace_create_file("README", 0444, d_tracer,
 			NULL, &tracing_readme_fops);
 
+	trace_create_file("version", 0444, d_tracer,
+			NULL, &tracing_version_fops);
+
 	trace_create_file("trace_pipe", 0444, d_tracer,
 			(void *) TRACE_PIPE_ALL_CPU, &tracing_pipe_fops);
 
-- 
1.7.3.1

Re: [PATCH] ftrace: add 'version' special file for ring buffer format

[Steven Rostedt]

On Mon, 2011-03-14 at 15:42 -0700, Slava Pestov wrote:
> The intention here is that if the ftrace ring buffer format changes
> in the future like it did in the past with ktrace, we can increment
> the version number and update clients appropriately.

I purposely avoided "versions", and instead use the
tracing/events/header_page and .../header_event to describe how the ring
buffer format is laid out. If the format changes, then so should those
files.

-- Steve

> 
> Tested: Verified that special file contains correct content
> 
> This patch is against 2.6.38-rc8 but should apply to any version
> from at least 2.6.34 onwards.
> 
> Signed-Off-By: Slava Pestov <slavapestov@google.com>

Re: [PATCH] ftrace: add 'version' special file for ring buffer format

[David Sharp]

On Mon, Mar 14, 2011 at 7:31 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 2011-03-14 at 15:42 -0700, Slava Pestov wrote:
>> The intention here is that if the ftrace ring buffer format changes
>> in the future like it did in the past with ktrace, we can increment
>> the version number and update clients appropriately.
>
> I purposely avoided "versions", and instead use the
> tracing/events/header_page and .../header_event to describe how the ring
> buffer format is laid out. If the format changes, then so should those
> files.
>
> -- Steve

Those are useful files, but do not encompass the entirety of the API.
Specifically, they do not encode the semantics of those values, nor
the control interface.

For example, when the ring buffer header changed from type:2;len:3 to
type_len:5, while this would have been reflected in header_event, how
to interpret the different versions is not something that could be
automatically understood by a parser.

These files also do not cover the semantics of the control files. For
example, it is imaginable that buffer_size_kb could be changed from
per-core size to a total size, or that it would reflect the actual
total amount of memory used, instead of the "non-header" space. The
grammar of filter expressions is another example of something that
could change without notice.

d#

>
>>
>> Tested: Verified that special file contains correct content
>>
>> This patch is against 2.6.38-rc8 but should apply to any version
>> from at least 2.6.34 onwards.
>>
>> Signed-Off-By: Slava Pestov <slavapestov@google.com>
>
>
>

Re: [PATCH] ftrace: add 'version' special file for ring buffer format

[Steven Rostedt]

On Tue, 2011-03-15 at 17:33 -0700, David Sharp wrote:
> On Mon, Mar 14, 2011 at 7:31 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Mon, 2011-03-14 at 15:42 -0700, Slava Pestov wrote:
> >> The intention here is that if the ftrace ring buffer format changes
> >> in the future like it did in the past with ktrace, we can increment
> >> the version number and update clients appropriately.
> >
> > I purposely avoided "versions", and instead use the
> > tracing/events/header_page and .../header_event to describe how the ring
> > buffer format is laid out. If the format changes, then so should those
> > files.
> >
> > -- Steve
> 
> Those are useful files, but do not encompass the entirety of the API.
> Specifically, they do not encode the semantics of those values, nor
> the control interface.
> 
> For example, when the ring buffer header changed from type:2;len:3 to
> type_len:5, while this would have been reflected in header_event, how
> to interpret the different versions is not something that could be
> automatically understood by a parser.

We do want to make the parser aware of these changes. As that change
happened, I have parse_events understand it.

> 
> These files also do not cover the semantics of the control files. For
> example, it is imaginable that buffer_size_kb could be changed from
> per-core size to a total size, or that it would reflect the actual
> total amount of memory used, instead of the "non-header" space. The
> grammar of filter expressions is another example of something that
> could change without notice.

Then we need to make sure that a change can be detected. Version numbers
for ABI in the kernel has been looked down upon. Again, if a change is
needed, I find a way to make it detected. I thought about adding to
buffer_size_kb:

1408 (total 5632)

or something of that matter.

Again, if we can not determine the features by reading a file or finding
if a syscall exists or not, then we are doing something wrong and need
to fix that.

Lets put it another way. Instead of making it easy for a parser, lets
make it easy for humans to know. If I look at the files I can quickly
figure out what type of features are available. I would be clueless to
know if all I had was "v1.0.2". One more thing: Version numbers are like
documentation. They are great when you start out, but soon become out of
sync with reality.

-- Steve