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@xxxxxxxxxx>
> ---
>  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;
>  	}
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/