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

From: Frederic Weisbecker
Date: Mon Feb 09 2009 - 12:34:19 EST


On Mon, Feb 09, 2009 at 12:28:32PM -0500, Steven Rostedt wrote:
> From: Eduard - Gabriel Munteanu <eduard.munteanu@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Steven Rostedt <srostedt@xxxxxxxxxx>
> ---
> 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().

--
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/