diff mbox series

[v2] ring-buffer: Prevent inconsistent operation on cpu_buffer->resize_disabled

Message ID 20230409024616.31099-1-Tze-nan.Wu@mediatek.com (mailing list archive)
State Superseded
Headers show
Series [v2] ring-buffer: Prevent inconsistent operation on cpu_buffer->resize_disabled | expand

Commit Message

Tze-nan Wu April 9, 2023, 2:46 a.m. UTC
Write to buffer_size_kb can permanently fail due to cpu_online_mask changed
between two for_each_online_buffer_cpu loops.
The number of increasing and decreasing on cpu_buffer->resize_disable
may be inconsistent, leading that the resize_disabled in some CPUs
becoming none zero after ring_buffer_reset_online_cpus return.

This issue can be reproduced by "echo 0 > trace" and hotplug cpu at the
same time. After reproducing success, we can find out buffer_size_kb
will not be functional anymore.

This patch uses cpus_read_lock() to prevent cpu_online_mask being changed
between two different "for_each_online_buffer_cpu".

Changes in v2:
  Use cpus_read_lock() instead of copying cpu_online_mask at the entry of
  function

Link:
  https://lore.kernel.org/lkml/20230408052226.25268-1-Tze-nan.Wu@mediatek.com/

Cc: stable@vger.kernel.org
Signed-off-by: Tze-nan Wu <Tze-nan.Wu@mediatek.com>
---
 kernel/trace/ring_buffer.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Bagas Sanjaya April 9, 2023, 12:30 p.m. UTC | #1
On 4/9/23 09:46, Tze-nan Wu wrote:
> This issue can be reproduced by "echo 0 > trace" and hotplug cpu at the
> same time. After reproducing success, we can find out buffer_size_kb
> will not be functional anymore.
> 

Do you mean disabling tracing while hotplugging CPU? Or disabling both
tracing and hotplug CPU?

> This patch uses cpus_read_lock() to prevent cpu_online_mask being changed
> between two different "for_each_online_buffer_cpu".
> 

"Use cpu_read_lock() to prevent ..."

> Changes in v2:
>   Use cpus_read_lock() instead of copying cpu_online_mask at the entry of
>   function
> 

To resolve kernel test robot warnings ([1] and [2])? Or have they been fixed?

[1]: https://lore.kernel.org/stable/202304081615.eiaqpbV8-lkp@intel.com/
[2]: https://lore.kernel.org/stable/202304082051.Dp50upfS-lkp@intel.com/

Thanks.
Tze-nan Wu April 10, 2023, 1:35 a.m. UTC | #2
Hi Bagas,

On Sun, 2023-04-09 at 19:30 +0700, Bagas Sanjaya wrote:
> 
> On 4/9/23 09:46, Tze-nan Wu wrote:
> > This issue can be reproduced by "echo 0 > trace" and hotplug cpu at
> > the
> > same time. After reproducing success, we can find out
> > buffer_size_kb
> > will not be functional anymore.
> > 
> 
> Do you mean disabling tracing while hotplugging CPU? Or disabling
> both
> tracing and hotplug CPU?
> 
  I mean disabling tracing while hotplugging CPU, sorry for the
confusion here.

> > This patch uses cpus_read_lock() to prevent cpu_online_mask being
> > changed
> > between two different "for_each_online_buffer_cpu".
> > 
> 
> "Use cpu_read_lock() to prevent ..."
> 
  Thanks for the suggestion 

> > Changes in v2:
> >   Use cpus_read_lock() instead of copying cpu_online_mask at the
> > entry of
> >   function
> > 
> 
  Since I change to fix the issue by using cpus_read_lock(), we don't
need a copy of cpu_online_mask anymore, hence the two warnings will not
meet in the v2 patch.

> To resolve kernel test robot warnings ([1] and [2])? Or have they
> been fixed?
> 
> [1]: 
> https://lore.kernel.org/stable/202304081615.eiaqpbV8-lkp@intel.com/
> [2]: 
> https://lore.kernel.org/stable/202304082051.Dp50upfS-lkp@intel.com/
> 
> Thanks.
> 
> --
> An old man doll... just what I always wanted! - Clara
>
diff mbox series

Patch

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 76a2d91eecad..44d833252fb0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -5357,6 +5357,7 @@  void ring_buffer_reset_online_cpus(struct trace_buffer *buffer)
 
 	/* prevent another thread from changing buffer sizes */
 	mutex_lock(&buffer->mutex);
+	cpus_read_lock();
 
 	for_each_online_buffer_cpu(buffer, cpu) {
 		cpu_buffer = buffer->buffers[cpu];
@@ -5377,6 +5378,7 @@  void ring_buffer_reset_online_cpus(struct trace_buffer *buffer)
 		atomic_dec(&cpu_buffer->resize_disabled);
 	}
 
+	cpus_read_unlock();
 	mutex_unlock(&buffer->mutex);
 }