Re: [V3 PATCH] x86: avoid calling arch_trigger_all_cpu_backtrace() atthe same time on SMP

[Dongdong Deng]

On Thu, Nov 11, 2010 at 7:23 PM, DDD <dongdong.deng@xxxxxxxxxxxxx> wrote:
> Ingo Molnar wrote:
>>
>> Hm, another thing i noticed is that there's two of these:
>>
>>> Â#ifdef ARCH_HAS_NMI_WATCHDOG
>>> +/* "in progress" flag of arch_trigger_all_cpu_backtrace */
>>> +static unsigned long backtrace_flag;
>>> +
>>> Âvoid arch_trigger_all_cpu_backtrace(void)
>>> Â{
>>> Â Â Â Âint i;
>>> + Â Â Â unsigned long flags;
>>> +
>>> + Â Â Â /*
>>> + Â Â Â Â* Have to disable irq here, as the
>>> + Â Â Â Â* arch_trigger_all_cpu_backtrace() could be
>>> + Â Â Â Â* triggered by "spin_lock()" with irqs on.
>>> + Â Â Â Â*/
>>> + Â Â Â local_irq_save(flags);
>>
>>> +/* "in progress" flag of arch_trigger_all_cpu_backtrace */
>>> +static unsigned long backtrace_flag;
>>> +
>>> Âvoid arch_trigger_all_cpu_backtrace(void)
>>> Â{
>>> Â Â Â Âint i;
>>> + Â Â Â unsigned long flags;
>>> +
>>> + Â Â Â /*
>>> + Â Â Â Â* Have to disable irq here, as the
>>> + Â Â Â Â* arch_trigger_all_cpu_backtrace() could be
>>> + Â Â Â Â* triggered by "spin_lock()" with irqs on.
>>> + Â Â Â Â*/
>>> + Â Â Â local_irq_save(flags);
>>> +
>>> + Â Â Â if (test_and_set_bit(0, &backtrace_flag))
>>
>> A fair amount of code is being duplicated in two places - which is not
>> nice. Lets try to create a shared facility instead?
>
> Yep, It is a good idea, I will try to do that. :-)

Hmm, it is hard to create a shared facility for
arch_trigger_all_cpu_backtrace().

The arch_trigger_all_cpu_backtrace() need a local variable(backtrace_mask)
which was used by the other difference functions in nmi.c and hw_nmi.c.

If we want to create a shared facility for arch_trigger_all_cpu_backtrace(),
we have to create an independent .c file for the single one function
arch_trigger_all_cpu_backtrace(). I think it is worse than before...

Maybe Don could say some things here as he is the owner of hw_nmi.c.  :-)

Thanks,
Dongdong
--
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/