[PATCH] Tracing: fix regression of trace_pipe

[PATCH] Tracing: fix regression of trace_pipe

[Liu Bo]

commit 0fb9656d changes the behaviour of trace_pipe, ie. it makes trace_pipe
return if we've read something and tracing is enabled, and this means that
we have to 'cat trace_pipe' again and again while running tests.

IMO the right way is if tracing is enabled, we always block and wait for
ring buffer, or we may lose what we want since ring buffer's size is limited.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 kernel/trace/trace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e512567..fc76beb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3452,7 +3452,7 @@ static int tracing_wait_pipe(struct file *filp)
 			return -EINTR;
 
 		/*
-		 * We block until we read something and tracing is enabled.
+		 * We block until we read something and tracing is disabled.
 		 * We still block if tracing is disabled, but we have never
 		 * read anything. This allows a user to cat this file, and
 		 * then enable tracing. But after we have read something,
@@ -3460,7 +3460,7 @@ static int tracing_wait_pipe(struct file *filp)
 		 *
 		 * iter->pos will be 0 if we haven't read anything.
 		 */
-		if (tracing_is_enabled() && iter->pos)
+		if (!tracing_is_enabled() && iter->pos)
 			break;
 	}
 
-- 
1.7.7.6

Re: [PATCH] Tracing: fix regression of trace_pipe

[Steven Rostedt]

On Mon, 2013-01-14 at 10:54 +0800, Liu Bo wrote:
> commit 0fb9656d changes the behaviour of trace_pipe, ie. it makes trace_pipe
> return if we've read something and tracing is enabled, and this means that
> we have to 'cat trace_pipe' again and again while running tests.

Bah, this is the second regression that this commit caused. I fixed the
first one and was about to send it to Linus. I'll add this one too and
rerun my tests again.

> 
> IMO the right way is if tracing is enabled, we always block and wait for
> ring buffer, or we may lose what we want since ring buffer's size is limited.

Hmm, I shouldn't have made this change with this commit. I may have
meant to do it as a separate commit, but committed both changes in this
one. It's actually unrelated to the change that was made in the change
log :-(

I'll see if I can add a test that checks for this regression, and add
that too to my test suite.

Thanks,

-- Steve

> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>  kernel/trace/trace.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index e512567..fc76beb 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3452,7 +3452,7 @@ static int tracing_wait_pipe(struct file *filp)
>  			return -EINTR;
>  
>  		/*
> -		 * We block until we read something and tracing is enabled.
> +		 * We block until we read something and tracing is disabled.
>  		 * We still block if tracing is disabled, but we have never
>  		 * read anything. This allows a user to cat this file, and
>  		 * then enable tracing. But after we have read something,
> @@ -3460,7 +3460,7 @@ static int tracing_wait_pipe(struct file *filp)
>  		 *
>  		 * iter->pos will be 0 if we haven't read anything.
>  		 */
> -		if (tracing_is_enabled() && iter->pos)
> +		if (!tracing_is_enabled() && iter->pos)
>  			break;
>  	}
>