1. Patchew
  2. linux
  3. View series

[GIT PULL] ring-buffer: Revert previous fix as it wasn't the actual fix

Steven Rostedt posted 1 patch 1 week, 1 day ago 
kernel/trace/ring_buffer.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
[GIT PULL] ring-buffer: Revert previous fix as it wasn't the actual fix
Posted by Steven Rostedt 1 week, 1 day ago 


Linus,

Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"

- A crash that happened on cpu hotplug was actually caused by the incorrect
  ref counting that was fixed by commit 2cf9733891a4 ("ring-buffer: Fix
  refcount setting of boot mapped buffers"). The removal of calling cpu
  hotplug callbacks on memory mapped buffers was not an issue even though
  the tests at the time pointed toward it. But in fact, there's a check in
  that code that tests to see if the buffers are already allocated or not,
  and will not allocate them again if they are. Not calling the cpu hotplug
  callbacks ended up not initializing the non boot CPU buffers.

  Simply remove that change.


Please pull the latest trace-ringbuffer-v6.12-rc7 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace-ringbuffer-v6.12-rc7

Tag SHA1: 81ccc26cac30f76e5fb6d76aec178677c3e209ad
Head SHA1: 580bb355bcae7e9a6606ce9644af09b2a793f1bb


Steven Rostedt (1):
      Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"

----
 kernel/trace/ring_buffer.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
---------------------------
commit 580bb355bcae7e9a6606ce9644af09b2a793f1bb
Author: Steven Rostedt <rostedt@goodmis.org>
Date:   Wed Nov 13 23:08:39 2024 -0500

    Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
    
    A crash happened when testing cpu hotplug with respect to the memory
    mapped ring buffers. It was assumed that the hot plug code was adding a
    per CPU buffer that was already created that caused the crash. The real
    problem was due to ref counting and was fixed by commit 2cf9733891a4
    ("ring-buffer: Fix refcount setting of boot mapped buffers").
    
    When a per CPU buffer is created, it will not be created again even with
    CPU hotplug, so the fix to not use CPU hotplug was a red herring. In fact,
    it caused only the boot CPU buffer to be created, leaving the other CPU
    per CPU buffers disabled.
    
    Revert that change as it was not the culprit of the fix it was intended to
    be.
    
    Cc: Masami Hiramatsu <mhiramat@kernel.org>
    Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
    Link: https://lore.kernel.org/20241113230839.6c03640f@gandalf.local.home
    Fixes: 912da2c384d5 ("ring-buffer: Do not have boot mapped buffers hook to CPU hotplug")
    Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 3ea4f7bb1837..5807116bcd0b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2337,12 +2337,9 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 	if (!buffer->buffers[cpu])
 		goto fail_free_buffers;
 
-	/* If already mapped, do not hook to CPU hotplug */
-	if (!start) {
-		ret = cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
-		if (ret < 0)
-			goto fail_free_buffers;
-	}
+	ret = cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
+	if (ret < 0)
+		goto fail_free_buffers;
 
 	mutex_init(&buffer->mutex);
Re: [GIT PULL] ring-buffer: Revert previous fix as it wasn't the actual fix
Posted by pr-tracker-bot@kernel.org 6 days, 2 hours ago 
The pull request you sent on Thu, 14 Nov 2024 10:41:49 -0500:

> git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git trace-ringbuffer-v6.12-rc7

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/09663753bb7c50b33f8e5fa562c20ce275b88237

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] ring-buffer: Revert previous fix as it wasn't the actual fix
Posted by Steven Rostedt 1 week, 1 day ago 
Hi Linus,

You can hold off on pulling this. I just found another issue I want to fix
and will likely have another pull request tomorrow on top of this one.

-- Steve


On Thu, 14 Nov 2024 10:41:49 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Linus,
> 
> Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
> 
> - A crash that happened on cpu hotplug was actually caused by the incorrect
>   ref counting that was fixed by commit 2cf9733891a4 ("ring-buffer: Fix
>   refcount setting of boot mapped buffers"). The removal of calling cpu
>   hotplug callbacks on memory mapped buffers was not an issue even though
>   the tests at the time pointed toward it. But in fact, there's a check in
>   that code that tests to see if the buffers are already allocated or not,
>   and will not allocate them again if they are. Not calling the cpu hotplug
>   callbacks ended up not initializing the non boot CPU buffers.
> 
>   Simply remove that change.
> 
> 
> Please pull the latest trace-ringbuffer-v6.12-rc7 tree, which can be found at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
> trace-ringbuffer-v6.12-rc7
> 
> Tag SHA1: 81ccc26cac30f76e5fb6d76aec178677c3e209ad
> Head SHA1: 580bb355bcae7e9a6606ce9644af09b2a793f1bb
> 
> 
> Steven Rostedt (1):
>       Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
> 
> ----
>  kernel/trace/ring_buffer.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> ---------------------------
> commit 580bb355bcae7e9a6606ce9644af09b2a793f1bb
> Author: Steven Rostedt <rostedt@goodmis.org>
> Date:   Wed Nov 13 23:08:39 2024 -0500
> 
>     Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
>     
>     A crash happened when testing cpu hotplug with respect to the memory
>     mapped ring buffers. It was assumed that the hot plug code was adding a
>     per CPU buffer that was already created that caused the crash. The real
>     problem was due to ref counting and was fixed by commit 2cf9733891a4
>     ("ring-buffer: Fix refcount setting of boot mapped buffers").
>     
>     When a per CPU buffer is created, it will not be created again even with
>     CPU hotplug, so the fix to not use CPU hotplug was a red herring. In fact,
>     it caused only the boot CPU buffer to be created, leaving the other CPU
>     per CPU buffers disabled.
>     
>     Revert that change as it was not the culprit of the fix it was intended to
>     be.
>     
>     Cc: Masami Hiramatsu <mhiramat@kernel.org>
>     Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>     Link: https://lore.kernel.org/20241113230839.6c03640f@gandalf.local.home
>     Fixes: 912da2c384d5 ("ring-buffer: Do not have boot mapped buffers hook to CPU hotplug")
>     Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 3ea4f7bb1837..5807116bcd0b 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -2337,12 +2337,9 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  	if (!buffer->buffers[cpu])
>  		goto fail_free_buffers;
>  
> -	/* If already mapped, do not hook to CPU hotplug */
> -	if (!start) {
> -		ret = cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
> -		if (ret < 0)
> -			goto fail_free_buffers;
> -	}
> +	ret = cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
> +	if (ret < 0)
> +		goto fail_free_buffers;
>  
>  	mutex_init(&buffer->mutex);
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.