2013-03-07 15:51:44

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH 0/2] [GIT PULL][3.9] tracing: Fix in snapshot API


Ingo,

The snapshot feature has been added in the 3.9 merge window. But after
playing with it some more, I've come to realize that it needs a
self documenting help (I had to look at the code a few times to remember
how to use it, or look in Documentation), and it also gives an awkward
error code if you go to clear the buffer and the buffer does not exist.

These two patches fix this issues, and I strongly suggest they go in
before snapshot is part of a major release.

The first patch gives the user a nice description to how to use snapshot:

# tracer: nop
#
#
# * Snapshot is freed *
#
# Snapshot commands:
# echo 0 > snapshot : Clears and frees snapshot buffer
# echo 1 > snapshot : Allocates snapshot buffer, if not already allocated.
# Takes a snapshot of the main buffer.
# echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
# (Doesn't have to be '2' works with any number that
# is not a '0' or '1')

as well as the state of the snapshot buffer (freed or allocated).

The second patch, returns success on a reset of the buffer even if
the buffer wasn't allocated. Returning -EINVAL is just confusing.

These should go into mainline before 3.9 is released.

Thanks!

Please pull the latest tip/perf/urgent tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
tip/perf/urgent

Head SHA1: c9960e48543799f168c4c9486f9790fb686ce5a8


Steven Rostedt (Red Hat) (2):
tracing: Add help of snapshot feature when snapshot is empty
tracing: Do not return EINVAL in snapshot when not allocated

----
kernel/trace/trace.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)


Attachments:
signature.asc (490.00 B)
This is a digitally signed message part

2013-03-08 07:34:08

by Hiraku Toyooka

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL][3.9] tracing: Fix in snapshot API

Steven,

(03/08/2013 12:34 AM), Steven Rostedt wrote:
> The second patch, returns success on a reset of the buffer even if
> the buffer wasn't allocated. Returning -EINVAL is just confusing.

I realized we should update the snapshot documentation together with
this change.
I attached a patch to update the documentation. Could you include this
patch?

Thanks,
Hiraku Toyooka


From: Hiraku Toyooka <[email protected]>
Subject: [PATCH] tracing: update documentation of snapshot utility

Now, "snapshot" file returns success on a reset of snapshot buffer
even if the buffer wasn't allocated, instead of returning EINVAL.
This patch updates snapshot desctiption according to the change.

Signed-off-by: Hiraku Toyooka <[email protected]>
---
Documentation/trace/ftrace.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
index 53d6a3c..a372304 100644
--- a/Documentation/trace/ftrace.txt
+++ b/Documentation/trace/ftrace.txt
@@ -1873,7 +1873,7 @@ feature:

status\input | 0 | 1 | else |
--------------+------------+------------+------------+
- not allocated |(do nothing)| alloc+swap | EINVAL |
+ not allocated |(do nothing)| alloc+swap |(do nothing)|
--------------+------------+------------+------------+
allocated | free | swap | clear |
--------------+------------+------------+------------+
--
1.7.11.7

2013-03-08 13:17:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/2] [GIT PULL][3.9] tracing: Fix in snapshot API


* Hiraku Toyooka <[email protected]> wrote:

> Steven,
>
> (03/08/2013 12:34 AM), Steven Rostedt wrote:
> >The second patch, returns success on a reset of the buffer even if
> >the buffer wasn't allocated. Returning -EINVAL is just confusing.
>
> I realized we should update the snapshot documentation together with
> this change.
> I attached a patch to update the documentation. Could you include this
> patch?
>
> Thanks,
> Hiraku Toyooka
>
>
> From: Hiraku Toyooka <[email protected]>
> Subject: [PATCH] tracing: update documentation of snapshot utility
>
> Now, "snapshot" file returns success on a reset of snapshot buffer
> even if the buffer wasn't allocated, instead of returning EINVAL.
> This patch updates snapshot desctiption according to the change.
>
> Signed-off-by: Hiraku Toyooka <[email protected]>
> ---
> Documentation/trace/ftrace.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
> index 53d6a3c..a372304 100644
> --- a/Documentation/trace/ftrace.txt
> +++ b/Documentation/trace/ftrace.txt
> @@ -1873,7 +1873,7 @@ feature:
>
> status\input | 0 | 1 | else |
> --------------+------------+------------+------------+
> - not allocated |(do nothing)| alloc+swap | EINVAL |
> + not allocated |(do nothing)| alloc+swap |(do nothing)|
> --------------+------------+------------+------------+
> allocated | free | swap | clear |
> --------------+------------+------------+------------+

I pulled all the updates - thanks guys!

Ingo

Subject: [tip:perf/urgent] tracing: update documentation of snapshot utility

Commit-ID: 1abccd7419de9829bcdf9ab1f81d5f6cf74d55d3
Gitweb: http://git.kernel.org/tip/1abccd7419de9829bcdf9ab1f81d5f6cf74d55d3
Author: Hiraku Toyooka <[email protected]>
AuthorDate: Fri, 8 Mar 2013 16:32:25 +0900
Committer: Steven Rostedt <[email protected]>
CommitDate: Fri, 8 Mar 2013 06:27:11 -0500

tracing: update documentation of snapshot utility

Now, "snapshot" file returns success on a reset of snapshot buffer
even if the buffer wasn't allocated, instead of returning EINVAL.
This patch updates snapshot desctiption according to the change.

Link: http://lkml.kernel.org/r/[email protected]

Signed-off-by: Hiraku Toyooka <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
Documentation/trace/ftrace.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
index 53d6a3c..a372304 100644
--- a/Documentation/trace/ftrace.txt
+++ b/Documentation/trace/ftrace.txt
@@ -1873,7 +1873,7 @@ feature:

status\input | 0 | 1 | else |
--------------+------------+------------+------------+
- not allocated |(do nothing)| alloc+swap | EINVAL |
+ not allocated |(do nothing)| alloc+swap |(do nothing)|
--------------+------------+------------+------------+
allocated | free | swap | clear |
--------------+------------+------------+------------+