[PATCH v1 3/9] ring-buffer: Hold buffer->mutex in trace_rb_cpu_prepare()

[Vincent Donnefort <vdonnefort@google.com>]

ring_buffer_reset() walks buffer->cpumask twice across synchronize_rcu()
under buffer->mutex. If trace_rb_cpu_prepare() adds a CPU in between,
the second loop resets an undisabled buffer.

Hold buffer->mutex in trace_rb_cpu_prepare() to synchronise it with the
ring buffer reset instead of locking the entire hotplug path. As a
consequence, the cpus_read_lock() in ring_buffer_subbuf_order_set() can
be dropped. The one in ring_buffer_resize() still stands because it
schedules insertion or deletion of ring buffer pages on each CPU.

Fixes: b23d7a5f4a07 ("ring-buffer: speed up buffer resets by avoiding synchronize_rcu for each CPU")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
---
 kernel/trace/ring_buffer.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 32d3c47dbdf0..80f500ffd7ac 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2824,10 +2824,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 	buffer->clock = trace_clock_local;
 	buffer->reader_lock_key = key;
 
+	mutex_init(&buffer->mutex);
 	init_irq_work(&buffer->irq_work.work, rb_wake_up_waiters);
 	init_waitqueue_head(&buffer->irq_work.waiters);
 
-
 	bsize = sizeof(void *) * nr_cpu_ids;
 	buffer->buffers = kzalloc(ALIGN(bsize, cache_line_size()),
 				  GFP_KERNEL);
@@ -2923,8 +2923,6 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 	if (ret < 0)
 		goto fail_free_buffers;
 
-	mutex_init(&buffer->mutex);
-
 	/* Persistent ring buffer needs to flush cache before reboot. */
 	if (start && end) {
 		buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
@@ -3330,8 +3328,9 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 		return 0;
 
 	/*
-	 * Keep CPUs from coming online while resizing to synchronize
-	 * with new per CPU buffers being created.
+	 * Insertion or removal of pages in a ring buffer is scheduled on its
+	 * own CPU if online, so cpu_online_mask must stay stable. Must be taken
+	 * before buffer->mutex.
 	 */
 	guard(cpus_read_lock)();
 
@@ -7475,8 +7474,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 	if (psize > RB_WRITE_MASK + 1)
 		return -EINVAL;
 
-	guard(cpus_read_lock)();
-
 	/* prevent another thread from changing buffer sizes */
 	guard(mutex)(&buffer->mutex);
 
@@ -8077,6 +8074,8 @@ int trace_rb_cpu_prepare(unsigned int cpu, struct hlist_node *node)
 	if (cpumask_test_cpu(cpu, buffer->cpumask))
 		return 0;
 
+	guard(mutex)(&buffer->mutex);
+
 	nr_pages = 0;
 	nr_pages_same = 1;
 	/* check if all cpu sizes are same */
-- 
2.56.0.567.gdb80ae8cd5-goog