2013-03-02 03:00:56

by Steven Rostedt

[permalink] [raw]
Subject: [for-next][PATCH 5/7] tracing: Fix polling on trace_pipe_raw

From: Steven Rostedt <[email protected]>

The trace_pipe_raw never implemented polling and this was casing
issues for several utilities. This is now implemented.

Blocked reads still are on the TODO list.

Reported-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace.c | 78 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 51 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 00e5e6e..b958e9d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3503,10 +3503,8 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
}

static unsigned int
-tracing_poll_pipe(struct file *filp, poll_table *poll_table)
+trace_poll(struct trace_iterator *iter, struct file *filp, poll_table *poll_table)
{
- struct trace_iterator *iter = filp->private_data;
-
if (trace_flags & TRACE_ITER_BLOCK) {
/*
* Always select as readable when in blocking mode
@@ -3515,6 +3513,7 @@ tracing_poll_pipe(struct file *filp, poll_table *poll_table)
} else {
if (!trace_empty(iter))
return POLLIN | POLLRDNORM;
+ trace_wakeup_needed = true;
poll_wait(filp, &trace_wait, poll_table);
if (!trace_empty(iter))
return POLLIN | POLLRDNORM;
@@ -3523,6 +3522,14 @@ tracing_poll_pipe(struct file *filp, poll_table *poll_table)
}
}

+static unsigned int
+tracing_poll_pipe(struct file *filp, poll_table *poll_table)
+{
+ struct trace_iterator *iter = filp->private_data;
+
+ return trace_poll(iter, filp, poll_table);
+}
+
/*
* This is a make-shift waitqueue.
* A tracer might use this callback on some rare cases:
@@ -4312,9 +4319,8 @@ static const struct file_operations snapshot_fops = {
#endif /* CONFIG_TRACER_SNAPSHOT */

struct ftrace_buffer_info {
- struct trace_array *tr;
+ struct trace_iterator iter;
void *spare;
- int cpu;
unsigned int read;
};

@@ -4331,22 +4337,32 @@ static int tracing_buffers_open(struct inode *inode, struct file *filp)
if (!info)
return -ENOMEM;

- info->tr = tr;
- info->cpu = tc->cpu;
- info->spare = NULL;
+ info->iter.tr = tr;
+ info->iter.cpu_file = tc->cpu;
+ info->spare = NULL;
/* Force reading ring buffer for first read */
- info->read = (unsigned int)-1;
+ info->read = (unsigned int)-1;

filp->private_data = info;

return nonseekable_open(inode, filp);
}

+static unsigned int
+tracing_buffers_poll(struct file *filp, poll_table *poll_table)
+{
+ struct ftrace_buffer_info *info = filp->private_data;
+ struct trace_iterator *iter = &info->iter;
+
+ return trace_poll(iter, filp, poll_table);
+}
+
static ssize_t
tracing_buffers_read(struct file *filp, char __user *ubuf,
size_t count, loff_t *ppos)
{
struct ftrace_buffer_info *info = filp->private_data;
+ struct trace_iterator *iter = &info->iter;
ssize_t ret;
size_t size;

@@ -4354,7 +4370,7 @@ tracing_buffers_read(struct file *filp, char __user *ubuf,
return 0;

if (!info->spare)
- info->spare = ring_buffer_alloc_read_page(info->tr->buffer, info->cpu);
+ info->spare = ring_buffer_alloc_read_page(iter->tr->buffer, iter->cpu_file);
if (!info->spare)
return -ENOMEM;

@@ -4362,12 +4378,12 @@ tracing_buffers_read(struct file *filp, char __user *ubuf,
if (info->read < PAGE_SIZE)
goto read;

- trace_access_lock(info->cpu);
- ret = ring_buffer_read_page(info->tr->buffer,
+ trace_access_lock(iter->cpu_file);
+ ret = ring_buffer_read_page(iter->tr->buffer,
&info->spare,
count,
- info->cpu, 0);
- trace_access_unlock(info->cpu);
+ iter->cpu_file, 0);
+ trace_access_unlock(iter->cpu_file);
if (ret < 0)
return 0;

@@ -4392,9 +4408,10 @@ read:
static int tracing_buffers_release(struct inode *inode, struct file *file)
{
struct ftrace_buffer_info *info = file->private_data;
+ struct trace_iterator *iter = &info->iter;

if (info->spare)
- ring_buffer_free_read_page(info->tr->buffer, info->spare);
+ ring_buffer_free_read_page(iter->tr->buffer, info->spare);
kfree(info);

return 0;
@@ -4461,6 +4478,7 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
unsigned int flags)
{
struct ftrace_buffer_info *info = file->private_data;
+ struct trace_iterator *iter = &info->iter;
struct partial_page partial_def[PIPE_DEF_BUFFERS];
struct page *pages_def[PIPE_DEF_BUFFERS];
struct splice_pipe_desc spd = {
@@ -4491,8 +4509,9 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
len &= PAGE_MASK;
}

- trace_access_lock(info->cpu);
- entries = ring_buffer_entries_cpu(info->tr->buffer, info->cpu);
+ again:
+ trace_access_lock(iter->cpu_file);
+ entries = ring_buffer_entries_cpu(iter->tr->buffer, iter->cpu_file);

for (i = 0; i < pipe->buffers && len && entries; i++, len -= PAGE_SIZE) {
struct page *page;
@@ -4503,15 +4522,15 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
break;

ref->ref = 1;
- ref->buffer = info->tr->buffer;
- ref->page = ring_buffer_alloc_read_page(ref->buffer, info->cpu);
+ ref->buffer = iter->tr->buffer;
+ ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file);
if (!ref->page) {
kfree(ref);
break;
}

r = ring_buffer_read_page(ref->buffer, &ref->page,
- len, info->cpu, 1);
+ len, iter->cpu_file, 1);
if (r < 0) {
ring_buffer_free_read_page(ref->buffer, ref->page);
kfree(ref);
@@ -4535,20 +4554,24 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
spd.nr_pages++;
*ppos += PAGE_SIZE;

- entries = ring_buffer_entries_cpu(info->tr->buffer, info->cpu);
+ entries = ring_buffer_entries_cpu(iter->tr->buffer, iter->cpu_file);
}

- trace_access_unlock(info->cpu);
+ trace_access_unlock(iter->cpu_file);
spd.nr_pages = i;

/* did we read anything? */
if (!spd.nr_pages) {
- if ((file->f_flags & O_NONBLOCK) || (flags & SPLICE_F_NONBLOCK))
+ if ((file->f_flags & O_NONBLOCK) || (flags & SPLICE_F_NONBLOCK)) {
ret = -EAGAIN;
- else
- ret = 0;
- /* TODO: block */
- goto out;
+ goto out;
+ }
+ default_wait_pipe(iter);
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
+ }
+ goto again;
}

ret = splice_to_pipe(pipe, &spd);
@@ -4560,6 +4583,7 @@ out:
static const struct file_operations tracing_buffers_fops = {
.open = tracing_buffers_open,
.read = tracing_buffers_read,
+ .poll = tracing_buffers_poll,
.release = tracing_buffers_release,
.splice_read = tracing_buffers_splice_read,
.llseek = no_llseek,
--
1.7.10.4


2013-03-12 15:52:44

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [for-next][PATCH 5/7] tracing: Fix polling on trace_pipe_raw

Hi Steven,

Em Fri, 01 Mar 2013 21:48:38 -0500
Steven Rostedt <[email protected]> escreveu:

> From: Steven Rostedt <[email protected]>
>
> The trace_pipe_raw never implemented polling and this was casing
> issues for several utilities. This is now implemented.
>
> Blocked reads still are on the TODO list.

I tested it and it works fine.

> Reported-by: Mauro Carvalho Chehab <[email protected]>

Tested-by: Mauro Carvalho Chehab <[email protected]>

> Signed-off-by: Steven Rostedt <[email protected]>
> ---
> kernel/trace/trace.c | 78 +++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 00e5e6e..b958e9d 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3503,10 +3503,8 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
> }
>
> static unsigned int
> -tracing_poll_pipe(struct file *filp, poll_table *poll_table)
> +trace_poll(struct trace_iterator *iter, struct file *filp, poll_table *poll_table)
> {
> - struct trace_iterator *iter = filp->private_data;
> -
> if (trace_flags & TRACE_ITER_BLOCK) {
> /*
> * Always select as readable when in blocking mode
> @@ -3515,6 +3513,7 @@ tracing_poll_pipe(struct file *filp, poll_table *poll_table)
> } else {
> if (!trace_empty(iter))
> return POLLIN | POLLRDNORM;
> + trace_wakeup_needed = true;
> poll_wait(filp, &trace_wait, poll_table);
> if (!trace_empty(iter))
> return POLLIN | POLLRDNORM;
> @@ -3523,6 +3522,14 @@ tracing_poll_pipe(struct file *filp, poll_table *poll_table)
> }
> }
>
> +static unsigned int
> +tracing_poll_pipe(struct file *filp, poll_table *poll_table)
> +{
> + struct trace_iterator *iter = filp->private_data;
> +
> + return trace_poll(iter, filp, poll_table);
> +}
> +
> /*
> * This is a make-shift waitqueue.
> * A tracer might use this callback on some rare cases:
> @@ -4312,9 +4319,8 @@ static const struct file_operations snapshot_fops = {
> #endif /* CONFIG_TRACER_SNAPSHOT */
>
> struct ftrace_buffer_info {
> - struct trace_array *tr;
> + struct trace_iterator iter;
> void *spare;
> - int cpu;
> unsigned int read;
> };
>
> @@ -4331,22 +4337,32 @@ static int tracing_buffers_open(struct inode *inode, struct file *filp)
> if (!info)
> return -ENOMEM;
>
> - info->tr = tr;
> - info->cpu = tc->cpu;
> - info->spare = NULL;
> + info->iter.tr = tr;
> + info->iter.cpu_file = tc->cpu;
> + info->spare = NULL;
> /* Force reading ring buffer for first read */
> - info->read = (unsigned int)-1;
> + info->read = (unsigned int)-1;
>
> filp->private_data = info;
>
> return nonseekable_open(inode, filp);
> }
>
> +static unsigned int
> +tracing_buffers_poll(struct file *filp, poll_table *poll_table)
> +{
> + struct ftrace_buffer_info *info = filp->private_data;
> + struct trace_iterator *iter = &info->iter;
> +
> + return trace_poll(iter, filp, poll_table);
> +}
> +
> static ssize_t
> tracing_buffers_read(struct file *filp, char __user *ubuf,
> size_t count, loff_t *ppos)
> {
> struct ftrace_buffer_info *info = filp->private_data;
> + struct trace_iterator *iter = &info->iter;
> ssize_t ret;
> size_t size;
>
> @@ -4354,7 +4370,7 @@ tracing_buffers_read(struct file *filp, char __user *ubuf,
> return 0;
>
> if (!info->spare)
> - info->spare = ring_buffer_alloc_read_page(info->tr->buffer, info->cpu);
> + info->spare = ring_buffer_alloc_read_page(iter->tr->buffer, iter->cpu_file);
> if (!info->spare)
> return -ENOMEM;
>
> @@ -4362,12 +4378,12 @@ tracing_buffers_read(struct file *filp, char __user *ubuf,
> if (info->read < PAGE_SIZE)
> goto read;
>
> - trace_access_lock(info->cpu);
> - ret = ring_buffer_read_page(info->tr->buffer,
> + trace_access_lock(iter->cpu_file);
> + ret = ring_buffer_read_page(iter->tr->buffer,
> &info->spare,
> count,
> - info->cpu, 0);
> - trace_access_unlock(info->cpu);
> + iter->cpu_file, 0);
> + trace_access_unlock(iter->cpu_file);
> if (ret < 0)
> return 0;
>
> @@ -4392,9 +4408,10 @@ read:
> static int tracing_buffers_release(struct inode *inode, struct file *file)
> {
> struct ftrace_buffer_info *info = file->private_data;
> + struct trace_iterator *iter = &info->iter;
>
> if (info->spare)
> - ring_buffer_free_read_page(info->tr->buffer, info->spare);
> + ring_buffer_free_read_page(iter->tr->buffer, info->spare);
> kfree(info);
>
> return 0;
> @@ -4461,6 +4478,7 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
> unsigned int flags)
> {
> struct ftrace_buffer_info *info = file->private_data;
> + struct trace_iterator *iter = &info->iter;
> struct partial_page partial_def[PIPE_DEF_BUFFERS];
> struct page *pages_def[PIPE_DEF_BUFFERS];
> struct splice_pipe_desc spd = {
> @@ -4491,8 +4509,9 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
> len &= PAGE_MASK;
> }
>
> - trace_access_lock(info->cpu);
> - entries = ring_buffer_entries_cpu(info->tr->buffer, info->cpu);
> + again:
> + trace_access_lock(iter->cpu_file);
> + entries = ring_buffer_entries_cpu(iter->tr->buffer, iter->cpu_file);
>
> for (i = 0; i < pipe->buffers && len && entries; i++, len -= PAGE_SIZE) {
> struct page *page;
> @@ -4503,15 +4522,15 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
> break;
>
> ref->ref = 1;
> - ref->buffer = info->tr->buffer;
> - ref->page = ring_buffer_alloc_read_page(ref->buffer, info->cpu);
> + ref->buffer = iter->tr->buffer;
> + ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file);
> if (!ref->page) {
> kfree(ref);
> break;
> }
>
> r = ring_buffer_read_page(ref->buffer, &ref->page,
> - len, info->cpu, 1);
> + len, iter->cpu_file, 1);
> if (r < 0) {
> ring_buffer_free_read_page(ref->buffer, ref->page);
> kfree(ref);
> @@ -4535,20 +4554,24 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
> spd.nr_pages++;
> *ppos += PAGE_SIZE;
>
> - entries = ring_buffer_entries_cpu(info->tr->buffer, info->cpu);
> + entries = ring_buffer_entries_cpu(iter->tr->buffer, iter->cpu_file);
> }
>
> - trace_access_unlock(info->cpu);
> + trace_access_unlock(iter->cpu_file);
> spd.nr_pages = i;
>
> /* did we read anything? */
> if (!spd.nr_pages) {
> - if ((file->f_flags & O_NONBLOCK) || (flags & SPLICE_F_NONBLOCK))
> + if ((file->f_flags & O_NONBLOCK) || (flags & SPLICE_F_NONBLOCK)) {
> ret = -EAGAIN;
> - else
> - ret = 0;
> - /* TODO: block */
> - goto out;
> + goto out;
> + }
> + default_wait_pipe(iter);
> + if (signal_pending(current)) {
> + ret = -EINTR;
> + goto out;
> + }
> + goto again;
> }
>
> ret = splice_to_pipe(pipe, &spd);
> @@ -4560,6 +4583,7 @@ out:
> static const struct file_operations tracing_buffers_fops = {
> .open = tracing_buffers_open,
> .read = tracing_buffers_read,
> + .poll = tracing_buffers_poll,
> .release = tracing_buffers_release,
> .splice_read = tracing_buffers_splice_read,
> .llseek = no_llseek,


--

Cheers,
Mauro