Re: [PATCH 2/3] tracing: Move pipe waiting code out of tracing_read_pipe().

[Frederic Weisbecker <fweisbec@gmail.com>]

On Mon, Feb 09, 2009 at 12:28:32PM -0500, Steven Rostedt wrote:
> From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> 
> This moves the pipe waiting code from tracing_read_pipe() into
> tracing_wait_pipe(), which is useful to implement other fops, like
> splice_read.
> 
> Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  kernel/trace/trace.c |   69 +++++++++++++++++++++++++++++---------------------
>  1 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 9e29fdb..11fde0a 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2388,37 +2388,15 @@ tracing_poll_pipe(struct file *filp, poll_table *poll_table)
>  	}
>  }
>  
> -/*
> - * Consumer reader.
> - */
> -static ssize_t
> -tracing_read_pipe(struct file *filp, char __user *ubuf,
> -		  size_t cnt, loff_t *ppos)
> +/* Must be called with trace_types_lock mutex held. */
> +static int tracing_wait_pipe(struct file *filp)
>  {
>  	struct trace_iterator *iter = filp->private_data;
> -	ssize_t sret;
> -
> -	/* return any leftover data */
> -	sret = trace_seq_to_user(&iter->seq, ubuf, cnt);
> -	if (sret != -EBUSY)
> -		return sret;
> -
> -	trace_seq_reset(&iter->seq);
>  
> -	mutex_lock(&trace_types_lock);
> -	if (iter->trace->read) {
> -		sret = iter->trace->read(iter, filp, ubuf, cnt, ppos);
> -		if (sret)
> -			goto out;
> -	}
> -
> -waitagain:
> -	sret = 0;
>  	while (trace_empty(iter)) {
>  
>  		if ((filp->f_flags & O_NONBLOCK)) {
> -			sret = -EAGAIN;
> -			goto out;
> +			return -EAGAIN;
>  		}
>  
>  		/*
> @@ -2443,12 +2421,11 @@ waitagain:
>  		iter->tr->waiter = NULL;
>  
>  		if (signal_pending(current)) {
> -			sret = -EINTR;
> -			goto out;
> +			return -EINTR;
>  		}
>  
>  		if (iter->trace != current_trace)
> -			goto out;
> +			return 0;
>  
>  		/*
>  		 * We block until we read something and tracing is disabled.
> @@ -2465,9 +2442,43 @@ waitagain:
>  		continue;
>  	}
>  
> +	return 1;
> +}
> +
> +/*
> + * Consumer reader.
> + */
> +static ssize_t
> +tracing_read_pipe(struct file *filp, char __user *ubuf,
> +		  size_t cnt, loff_t *ppos)
> +{
> +	struct trace_iterator *iter = filp->private_data;
> +	ssize_t sret;
> +
> +	/* return any leftover data */
> +	sret = trace_seq_to_user(&iter->seq, ubuf, cnt);
> +	if (sret != -EBUSY)
> +		return sret;
> +
> +	trace_seq_reset(&iter->seq);
> +
> +	mutex_lock(&trace_types_lock);
> +	if (iter->trace->read) {
> +		sret = iter->trace->read(iter, filp, ubuf, cnt, ppos);
> +		if (sret)
> +			goto out;
> +	}
> +
> +waitagain:
> +	sret = tracing_wait_pipe(filp);
> +	if (sret <= 0)
> +		goto out;
> +
>  	/* stop when tracing is finished */
> -	if (trace_empty(iter))
> +	if (trace_empty(iter)) {
> +		sret = 0;
>  		goto out;
> +	}
>  
>  	if (cnt >= PAGE_SIZE)
>  		cnt = PAGE_SIZE - 1;
> -- 
> 1.5.6.5
> 
> -- 

Hmm, ok I will base my changes on top of your tree.
I finally chose to use the common waiting interface, with an
exception concerning sched_switch, function and function_graph.

Even if this can be racy when the tracer is switched, we have a security
inside trace_wake_up().