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

From: Steven Rostedt
Date: Thu Nov 14 2024 - 10:59:33 EST



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@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> 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@xxxxxxxxxx>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> Link: https://lore.kernel.org/20241113230839.6c03640f@xxxxxxxxxxxxxxxxxx
> Fixes: 912da2c384d5 ("ring-buffer: Do not have boot mapped buffers hook to CPU hotplug")
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
>
> 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);
>