Re: [PATCH] tracing/ring-buffer: fix non cpu hotplug case

[Steven Rostedt]

On Thu, 19 Mar 2009, Frederic Weisbecker wrote:

> Impact: fix warning with irqsoff tracer
> 
> The ring buffer allocates its buffers on pre-smp time (early_initcall).
> It means that, at first, only the boot cpu buffer is allocated and
> the ring-buffer cpumask only has the boot cpu set (cpu_online_mask).
> 
> Later, the secondary cpu will show up and the ring-buffer will be notified
> about this event: the appropriate buffer will be allocated and the cpumask
> will be updated.
> 
> Unfortunately, if !CONFIG_CPU_HOTPLUG, the ring-buffer will not be
> notified about the secondary cpus, meaning that the cpumask will have
> only the cpu boot set, and only one cpu buffer allocated.
> 
> We fix that by using cpu_possible_mask if !CONFIG_CPU_HOTPLUG.
> 
> This patch fixes the following warning with irqsoff tracer running:
> 
> [  169.317794] WARNING: at kernel/trace/trace.c:466 update_max_tr_single+0xcc/0xf3()
> [  169.318002] Hardware name: AMILO Li 2727
> [  169.318002] Modules linked in:
> [  169.318002] Pid: 5624, comm: bash Not tainted 2.6.29-rc8-tip-02636-g6aafa6c #11
> [  169.318002] Call Trace:
> [  169.318002]  [<ffffffff81036182>] warn_slowpath+0xea/0x13d
> [  169.318002]  [<ffffffff8100b9d6>] ? ftrace_call+0x5/0x2b
> [  169.318002]  [<ffffffff8100b9d6>] ? ftrace_call+0x5/0x2b
> [  169.318002]  [<ffffffff8100b9d1>] ? ftrace_call+0x0/0x2b
> [  169.318002]  [<ffffffff8101ef10>] ? ftrace_modify_code+0xa9/0x108
> [  169.318002]  [<ffffffff8106e27f>] ? trace_hardirqs_off+0x25/0x27
> [  169.318002]  [<ffffffff8149afe7>] ? _spin_unlock_irqrestore+0x1f/0x2d
> [  169.318002]  [<ffffffff81064f52>] ? ring_buffer_reset_cpu+0xf6/0xfb
> [  169.318002]  [<ffffffff8106637c>] ? ring_buffer_reset+0x36/0x48
> [  169.318002]  [<ffffffff8106aeda>] update_max_tr_single+0xcc/0xf3
> [  169.318002]  [<ffffffff8100bc17>] ? sysret_check+0x22/0x5d
> [  169.318002]  [<ffffffff8106e3ea>] stop_critical_timing+0x142/0x204
> [  169.318002]  [<ffffffff8106e4cf>] trace_hardirqs_on_caller+0x23/0x25
> [  169.318002]  [<ffffffff8149ac28>] trace_hardirqs_on_thunk+0x3a/0x3c
> [  169.318002]  [<ffffffff8100bc17>] ? sysret_check+0x22/0x5d
> [  169.318002] ---[ end trace db76cbf775a750cf ]---
> 
> Because this tracer may try to swap two cpu ring buffers for an
> unregistered cpu on the ring buffer.
> 
> This patch might also fix a fair loss of traces due to unallocated buffers
> for secondary cpus.

Thanks Frederic!

Acked-by: Steven Rostedt <srostedt@xxxxxxxxxx>

-- Steve

> 
> Reported-by: Ingo Molnar <mingo@xxxxxxx>
> Signed-off-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> ---
>  kernel/trace/ring_buffer.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index c9ca8a8..bc74f00 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -578,8 +578,17 @@ struct ring_buffer *ring_buffer_alloc(unsigned long size, unsigned flags)
>  	if (buffer->pages == 1)
>  		buffer->pages++;
>  
> +	/*
> +	 * In case of non-hotplug cpu, if the ring-buffer is allocated
> +	 * in early initcall, it will not be notified of secondary cpus.
> +	 * In that off case, we need to allocate for all possible cpus.
> +	 */
> +#ifdef CONFIG_HOTPLUG_CPU
>  	get_online_cpus();
>  	cpumask_copy(buffer->cpumask, cpu_online_mask);
> +#else
> +	cpumask_copy(buffer->cpumask, cpu_possible_mask);
> +#endif
>  	buffer->cpus = nr_cpu_ids;
>  
>  	bsize = sizeof(void *) * nr_cpu_ids;
> -- 
> 1.6.1
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/