Skip to content

Commit 39a7dc2

Browse files
committed
tracing: Fix blocked reader of snapshot buffer
If an application blocks on the snapshot or snapshot_raw files, expecting to be woken up when a snapshot occurs, it will not happen. Or it may happen with an unexpected result. That result is that the application will be reading the main buffer instead of the snapshot buffer. That is because when the snapshot occurs, the main and snapshot buffers are swapped. But the reader has a descriptor still pointing to the buffer that it originally connected to. This is fine for the main buffer readers, as they may be blocked waiting for a watermark to be hit, and when a snapshot occurs, the data that the main readers want is now on the snapshot buffer. But for waiters of the snapshot buffer, they are waiting for an event to occur that will trigger the snapshot and they can then consume it quickly to save the snapshot before the next snapshot occurs. But to do this, they need to read the new snapshot buffer, not the old one that is now receiving new data. Also, it does not make sense to have a watermark "buffer_percent" on the snapshot buffer, as the snapshot buffer is static and does not receive new data except all at once. Link: https://lore.kernel.org/linux-trace-kernel/20231228095149.77f5b45d@gandalf.local.home Cc: stable@vger.kernel.org Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Mark Rutland <mark.rutland@arm.com> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Fixes: debdd57 ("tracing: Make a snapshot feature available from userspace") Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent 623b1f8 commit 39a7dc2

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

kernel/trace/ring_buffer.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,8 @@ void ring_buffer_wake_waiters(struct trace_buffer *buffer, int cpu)
949949
/* make sure the waiters see the new index */
950950
smp_wmb();
951951

952-
rb_wake_up_waiters(&rbwork->work);
952+
/* This can be called in any context */
953+
irq_work_queue(&rbwork->work);
953954
}
954955

955956
/**

kernel/trace/trace.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1894,6 +1894,9 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu,
18941894
__update_max_tr(tr, tsk, cpu);
18951895

18961896
arch_spin_unlock(&tr->max_lock);
1897+
1898+
/* Any waiters on the old snapshot buffer need to wake up */
1899+
ring_buffer_wake_waiters(tr->array_buffer.buffer, RING_BUFFER_ALL_CPUS);
18971900
}
18981901

18991902
/**
@@ -1945,12 +1948,23 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
19451948

19461949
static int wait_on_pipe(struct trace_iterator *iter, int full)
19471950
{
1951+
int ret;
1952+
19481953
/* Iterators are static, they should be filled or empty */
19491954
if (trace_buffer_iter(iter, iter->cpu_file))
19501955
return 0;
19511956

1952-
return ring_buffer_wait(iter->array_buffer->buffer, iter->cpu_file,
1953-
full);
1957+
ret = ring_buffer_wait(iter->array_buffer->buffer, iter->cpu_file, full);
1958+
1959+
#ifdef CONFIG_TRACER_MAX_TRACE
1960+
/*
1961+
* Make sure this is still the snapshot buffer, as if a snapshot were
1962+
* to happen, this would now be the main buffer.
1963+
*/
1964+
if (iter->snapshot)
1965+
iter->array_buffer = &iter->tr->max_buffer;
1966+
#endif
1967+
return ret;
19541968
}
19551969

19561970
#ifdef CONFIG_FTRACE_STARTUP_TEST
@@ -8517,7 +8531,7 @@ tracing_buffers_splice_read(struct file *file, loff_t *ppos,
85178531

85188532
wait_index = READ_ONCE(iter->wait_index);
85198533

8520-
ret = wait_on_pipe(iter, iter->tr->buffer_percent);
8534+
ret = wait_on_pipe(iter, iter->snapshot ? 0 : iter->tr->buffer_percent);
85218535
if (ret)
85228536
goto out;
85238537

0 commit comments

Comments
 (0)