Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler

[Frederic Weisbecker]

2010/11/1 Cyrill Gorcunov <gorcunov@xxxxxxxxx>:
> On Mon, Nov 01, 2010 at 11:45:36AM -0400, Don Zickus wrote:
> ...
>> Heh.  Yeah when I migrated the code, I completely forgot the notifier
>> chain could be called from a preemptible context (ie not NMI).
>>
>> This patch should fix it and I think it is the correct fix.  Let me know
>> how it works out.
>>
>> Cheers,
>> Don
>>
>> diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
>> index c7c9ae4..1bdd0b5 100644
>> --- a/arch/x86/kernel/apic/hw_nmi.c
>> +++ b/arch/x86/kernel/apic/hw_nmi.c
>> @@ -63,7 +63,7 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
>>  {
>>       struct die_args *args = __args;
>>       struct pt_regs *regs;
>> -     int cpu = smp_processor_id();
>> +     int cpu;
>>
>>       switch (cmd) {
>>       case DIE_NMI:
>> @@ -74,6 +74,7 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
>>       }
>>
>>       regs = args->regs;
>> +     cpu = smp_processor_id();
>>
>>       if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
>>               static arch_spinlock_t lock = __ARCH_SPIN_LOCK_UNLOCKED;
>>
>
>  yup, this will do the trick for a while. In general I believe we might have
> kind of NMI exclusive chain so we wouldn't need the 'case:'s.


Yeah.  And seperating NMIs from the rest of the DIE notifiers would
probably improve
performance of things like PMI handling.

And I've always been confused with this "die" notifier semantic. We
are not dying when
we handle a counter overflow interrupt.
The same applies to DIE_INT3, DIE_TRAP, DIE_DEBUG, ....

But until then, as having a seperate notifier is quite a refactoring,
we should enqueue Don's fix.
Don, can you resend it with usual SOB and changelog?

Thanks.
--
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/