[PATCH v1] ring-buffer: Clean up resize_disabled checks

[PATCH v1] ring-buffer: Clean up resize_disabled checks

[Vincent Donnefort]

ring_buffer_subbuf_order_set() checks for resize_disabled twice under
the same buffer->mutex hold. Moreover, this check duplicates the logic
in ring_buffer_resize(). Create a common helper rb_resize_disabled() to
factor out this code.

Additionally, remove the unnecessary cpumask_test_cpu in
ring_buffer_subbuf_order_set().
for_each_buffer_cpu() already iterates over buffer->cpumask.

Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
---
 kernel/trace/ring_buffer.c | 67 ++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 39 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 04bb94c29f58..37801ac5e92e 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3280,6 +3280,21 @@ static void update_pages_handler(struct work_struct *work)
 	complete(&cpu_buffer->update_done);
 }
 
+static bool rb_resize_disabled(struct trace_buffer *buffer, int cpu)
+{
+	lockdep_assert_held(&buffer->mutex);
+
+	if (cpu != RING_BUFFER_ALL_CPUS)
+		return atomic_read(&buffer->buffers[cpu]->resize_disabled);
+
+	for_each_buffer_cpu(buffer, cpu) {
+		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
+			return true;
+	}
+
+	return false;
+}
+
 /**
  * ring_buffer_resize - resize the ring buffer
  * @buffer: the buffer to resize.
@@ -3324,20 +3339,17 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 	if (nr_pages < 2)
 		nr_pages = 2;
 
-	if (cpu_id == RING_BUFFER_ALL_CPUS) {
-		/*
-		 * Don't succeed if resizing is disabled, as a reader might be
-		 * manipulating the ring buffer and is expecting a sane state while
-		 * this is true.
-		 */
-		for_each_buffer_cpu(buffer, cpu) {
-			cpu_buffer = buffer->buffers[cpu];
-			if (atomic_read(&cpu_buffer->resize_disabled)) {
-				err = -EBUSY;
-				goto out_err_unlock;
-			}
-		}
+	/*
+	 * Don't succeed if resizing is disabled, as a reader might be
+	 * manipulating the ring buffer and is expecting a sane state while
+	 * this is true.
+	 */
+	if (rb_resize_disabled(buffer, cpu_id)) {
+		err = -EBUSY;
+		goto out_err_unlock;
+	}
 
+	if (cpu_id == RING_BUFFER_ALL_CPUS) {
 		/* calculate the pages to update */
 		for_each_buffer_cpu(buffer, cpu) {
 			cpu_buffer = buffer->buffers[cpu];
@@ -3409,16 +3421,6 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 		if (nr_pages == cpu_buffer->nr_pages)
 			goto out;
 
-		/*
-		 * Don't succeed if resizing is disabled, as a reader might be
-		 * manipulating the ring buffer and is expecting a sane state while
-		 * this is true.
-		 */
-		if (atomic_read(&cpu_buffer->resize_disabled)) {
-			err = -EBUSY;
-			goto out_err_unlock;
-		}
-
 		cpu_buffer->nr_pages_to_update = nr_pages -
 						cpu_buffer->nr_pages;
 
@@ -7473,13 +7475,9 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 
 	old_capacity = rb_subbuf_capacity(buffer);
 
-	/* The mmap fast path reads subbuf_order without buffer->mutex. */
-	for_each_buffer_cpu(buffer, cpu) {
-		if (!cpumask_test_cpu(cpu, buffer->cpumask))
-			continue;
-		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
-			return -EBUSY;
-	}
+	/* Check it is resizable before we touch subbuf_order */
+	if (rb_resize_disabled(buffer, RING_BUFFER_ALL_CPUS))
+		return -EBUSY;
 
 	atomic_inc(&buffer->record_disabled);
 
@@ -7490,17 +7488,8 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 
 	/* Make sure all new buffers are allocated, before deleting the old ones */
 	for_each_buffer_cpu(buffer, cpu) {
-
-		if (!cpumask_test_cpu(cpu, buffer->cpumask))
-			continue;
-
 		cpu_buffer = buffer->buffers[cpu];
 
-		if (atomic_read(&cpu_buffer->resize_disabled)) {
-			err = -EBUSY;
-			goto error;
-		}
-
 		/* Update the number of pages to match the new size */
 		nr_pages = old_capacity * buffer->buffers[cpu]->nr_pages;
 		nr_pages = DIV_ROUND_UP(nr_pages, rb_subbuf_capacity(buffer));

base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.55.0.1032.g73a4cd73de-goog

Re: [PATCH v1] ring-buffer: Clean up resize_disabled checks

[David CARLIER]

Hi Vincent.

> +   if (rb_resize_disabled(buffer, cpu_id)) {
> +           err = -EBUSY;
> +           goto out_err_unlock;
> +   }

For a single CPU, this now happens before the nr_pages == cpu_buffer->nr_pages
early exit, so writing the same size to per_cpu/cpuN/buffer_size_kb on a
mapped or persistent instance CPU fails with EBUSY instead of succeeding.
Intended ?

> Additionally, remove the unnecessary cpumask_test_cpu in
> ring_buffer_subbuf_order_set().

The install loop still has one.

Otherwise removing the second check is fine, the hotplug window is
covered by the cpus_read_lock() patch I sent separately.

Cheers.

Re: [PATCH v1] ring-buffer: Clean up resize_disabled checks

[Vincent Donnefort]

On Mon, Sep 14, 2026 at 08:09:24PM +0100, David CARLIER wrote:
> Hi Vincent.
> 
> > +   if (rb_resize_disabled(buffer, cpu_id)) {
> > +           err = -EBUSY;
> > +           goto out_err_unlock;
> > +   }
> 
> For a single CPU, this now happens before the nr_pages == cpu_buffer->nr_pages
> early exit, so writing the same size to per_cpu/cpuN/buffer_size_kb on a
> mapped or persistent instance CPU fails with EBUSY instead of succeeding.
> Intended ?

Actually no, I didn't see that it is also "fixing" this discrepancy between the
per_cpu buffer_size_kb and the global one.

It seems to me better to align the behaviour for both interface, but then it is
touching something that is user interface... 

Steven, WDYS?

> 
> > Additionally, remove the unnecessary cpumask_test_cpu in
> > ring_buffer_subbuf_order_set().
> 
> The install loop still has one.
> 
> Otherwise removing the second check is fine, the hotplug window is
> covered by the cpus_read_lock() patch I sent separately.
> 
> Cheers.

-- 
Vincent

Re: [PATCH v1] ring-buffer: Clean up resize_disabled checks

[Steven Rostedt]

On Tue, 15 Sep 2026 08:23:55 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> > For a single CPU, this now happens before the nr_pages == cpu_buffer->nr_pages
> > early exit, so writing the same size to per_cpu/cpuN/buffer_size_kb on a
> > mapped or persistent instance CPU fails with EBUSY instead of succeeding.
> > Intended ?  
> 
> Actually no, I didn't see that it is also "fixing" this discrepancy between the
> per_cpu buffer_size_kb and the global one.
> 
> It seems to me better to align the behaviour for both interface, but then it is
> touching something that is user interface... 
> 
> Steven, WDYS?

I don't think we should worry about it. If something is mapped, then we
shouldn't be touching that file. Even writing the same value should
error out. There's no need to do that.

Hopefully it doesn't break anything because if it does, then yeah, we
will need to do something different here.

Oh, and can you break this up into two patches. One that adds this and
and one that does the:

   Additionally, remove the unnecessary cpumask_test_cpu in
   ring_buffer_subbuf_order_set(). for_each_buffer_cpu() already
   iterates over buffer->cpumask.

Hey, breaking up patches improves your commit count ;-)

-- Steve