tracing: Handle ring buffer waiters
Subsystems (3)
Steven Rostedt
maintainercover tracing: Handle ring buffer waiters
While working on libtracefs functions that allow for reading the raw trace files, I found that my test programs would get stuck waiting on the buffer when they should stop. Signals work fine, but closing the file would not break them out of the wait.
I found this confusing and the reading functions would lose a lot of data. This fixes the wake up issue, which should have always been the case. I also added an ioctl() to the trace_pipe_raw that will wake up all waiters on any ioctl.
Steven Rostedt (Google) (5):
ring-buffer: Have the shortest_full queue be the shortest not longest
ring-buffer: Check pending waiters when doing wake ups as well
ring-buffer: Add ring_buffer_wake_waiters()
tracing: Wake up ring buffer waiters on closing of the file
tracing: Add ioctl() to force ring buffer waiters to wake up
----
include/linux/ring_buffer.h | 2 +- include/linux/trace_events.h | 1 + kernel/trace/ring_buffer.c | 39 +++++++++++++++++++++++++++++++++++++-- kernel/trace/trace.c | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-)
Steven Rostedt
maintainerPATCH 1/5 ring-buffer: Have the shortest_full queue be the shortest not longest
From: "Steven Rostedt (Google)" <[email protected]>
The logic to know when the shortest waiters on the ring buffer should be woken up or not has uses a less than instead of a greater than compare, which causes the shortest_full to actually be the longest.
--- kernel/trace/ring_buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/ring_buffer.c +1 -1
@@ -1011,7 +1011,7 @@ int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full) nr_pages = cpu_buffer->nr_pages; dirty =ring_buffer_nr_dirty_pages (buffer, cpu);if (!cpu_buffer->shortest_full || - cpu_buffer->shortest_full < full) + cpu_buffer->shortest_full > full) cpu_buffer->shortest_full = full;raw_spin_unlock_irqrestore (&cpu_buffer->reader_lock, flags); if (!pagebusy &&
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 2/5 ring-buffer: Check pending waiters when doing wake ups as well
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
The wake up waiters only checks the "wakeup_full" variable and not the "full_waiters_pending". The full_waiters_pending is set when a waiter is added to the wait queue. The wakeup_full is only set when an event is triggered, and it clears the full_waiters_pending to avoid multiple calls to irq_work_queue().
The irq_work callback really needs to check both wakeup_full as well as full_waiters_pending such that this code can be used to wake up waiters when a file is closed that represents the ring buffer and the waiters need to be woken up.
--- kernel/trace/ring_buffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/trace/ring_buffer.c +2 -1
@@ -917,8 +917,9 @@ static void rb_wake_up_waiters(struct irq_work *work) struct rb_irq_work *rbwork = container_of(work, struct rb_irq_work, work);wake_up_all (&rbwork->waiters); -if (rbwork->wakeup_full) { +if (rbwork->full_waiters_pending || rbwork->wakeup_full) { rbwork->wakeup_full = false; + rbwork->full_waiters_pending = false;wake_up_all (&rbwork->full_waiters); } }
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 3/5 ring-buffer: Add ring_buffer_wake_waiters()
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
On closing of a file that represents a ring buffer or flushing the file, there may be waiters on the ring buffer that needs to be woken up and exit the ring_buffer_wait() function.
Add ring_buffer_wake_waiters() to wake up the waiters on the ring buffer and allow them to exit the wait loop.
--- include/linux/ring_buffer.h | 2 +- kernel/trace/ring_buffer.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
include/linux/ring_buffer.h +1 -1
@@ -101,7 +101,7 @@ __ring_buffer_alloc(unsigned long size, unsigned flags, struct lock_class_key *kint ring_buffer_wait (struct trace_buffer *buffer,int cpu,int full);__poll_t ring_buffer_poll_wait (struct trace_buffer *buffer,int cpu,struct file *filp,poll_table *poll_table); - +void ring_buffer_wake_waiters (struct trace_buffer *buffer,int cpu);#define RING_BUFFER_ALL_CPUS -1
kernel/trace/ring_buffer.c +34 -0
@@ -413,6 +413,7 @@ struct rb_irq_work {struct irq_work work;wait_queue_head_t waiters;wait_queue_head_t full_waiters; +long wait_index;bool waiters_pending;bool full_waiters_pending;bool wakeup_full;
@@ -924,6 +925,32 @@ static void rb_wake_up_waiters(struct irq_work *work) } } +/** +* ring_buffer_wake_waiters - wake up any waiters on this ring buffer +* @buffer: The ring buffer to wake waiters on +* +* In the case of a file that represents a ring buffer is closing, +* it is prudent to wake up any waiters that are on this. +*/ +void ring_buffer_wake_waiters (struct trace_buffer *buffer,int cpu) +{ +struct ring_buffer_per_cpu *cpu_buffer; +struct rb_irq_work *rbwork; + +if (cpu ==RING_BUFFER_ALL_CPUS ) { + rbwork = &buffer->irq_work; + }else { + cpu_buffer = buffer->buffers[cpu]; + rbwork = &cpu_buffer->irq_work; + } + + rbwork->wait_index++; +/* make sure the waiters see the new index */ +smp_wmb (); + +rb_wake_up_waiters (&rbwork->work); +} +/** * ring_buffer_wait - wait for input to the ring buffer * @buffer: buffer to wait on
@@ -939,6 +966,7 @@ int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full)struct ring_buffer_per_cpu *cpu_buffer;DEFINE_WAIT (wait);struct rb_irq_work *work; +long wait_index;int ret =0 ;/*
@@ -957,6 +985,7 @@ int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full) work = &cpu_buffer->irq_work; } + wait_index =READ_ONCE (work->wait_index);while (true) {if (full)
@@ -1021,6 +1050,11 @@ int ring_buffer_wait(struct trace_buffer *buffer, int cpu, int full) }schedule (); + +/* Make sure to see the new wait index */ +smp_rmb (); +if (wait_index != work->wait_index) +break ; }if (full)
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 4/5 tracing: Wake up ring buffer waiters on closing of the file
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
When the file that represents the ring buffer is closed, there may be waiters waiting on more input from the ring buffer. Call ring_buffer_wake_waiters() to wake up any waiters when the file is closed.
--- include/linux/trace_events.h | 1 + kernel/trace/trace.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)
include/linux/trace_events.h +1 -0
@@ -92,6 +92,7 @@ struct trace_iterator {unsigned int temp_size;char *fmt;/* modified format holder */ unsigned int fmt_size; +long wait_index;/* trace_seq for __print_flags() and __print_symbolic() etc. */ struct trace_seq tmp_seq;
kernel/trace/trace.c +15 -0
@@ -8160,6 +8160,12 @@ static int tracing_buffers_release(struct inode *inode, struct file *file)__trace_array_put (iter->tr); + iter->wait_index++; +/* Make sure the waiters see the new wait_index */ +smp_wmb (); + +ring_buffer_wake_waiters (iter->array_buffer->buffer, iter->cpu_file); +if (info->spare)ring_buffer_free_read_page (iter->array_buffer->buffer, info->spare_cpu, info->spare);
@@ -8313,6 +8319,8 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,/* did we read anything? */ if (!spd.nr_pages) { +long wait_index; +if (ret) goto out;
@@ -8320,10 +8328,17 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,if ((file->f_flags &O_NONBLOCK ) || (flags &SPLICE_F_NONBLOCK )) goto out; + wait_index =READ_ONCE (iter->wait_index); + ret =wait_on_pipe (iter, iter->tr->buffer_percent);if (ret) goto out; +/* Make sure we see the new wait_index */ +smp_rmb (); +if (wait_index != iter->wait_index) + goto out; + goto again; }
Signature · 1 line
2.35.1
Steven Rostedt
maintainerPATCH 5/5 tracing: Add ioctl() to force ring buffer waiters to wake up
In reply to Steven RostedtFrom: "Steven Rostedt (Google)" <[email protected]>
If a process is waiting on the ring buffer for data, there currently isn't a clean way to force it to wake up. Add an ioctl call that will force any tasks that are waiting on the trace_pipe_raw file to wake up.
--- kernel/trace/trace.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
kernel/trace/trace.c +18 -0
@@ -8349,12 +8349,30 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,return ret; } +/* An ioctl call to the ring buffer file will wake up all waiters */ +static long tracing_buffers_ioctl (struct file *file,unsigned int cmd,unsigned long arg) +{ +struct ftrace_buffer_info *info = file->private_data; +struct trace_iterator *iter = &info->iter; + +mutex_lock (&trace_types_lock); + + iter->wait_index++; +/* Make sure the waiters see the new wait_index */ +smp_wmb (); + +ring_buffer_wake_waiters (iter->array_buffer->buffer, iter->cpu_file); +mutex_unlock (&trace_types_lock); +return 0 ; +} +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, + .unlocked_ioctl = tracing_buffers_ioctl, .llseek = no_llseek, };
Signature · 1 line
2.35.1