* BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler
@ 2010-11-01 12:39 Jan Kiszka
2010-11-01 15:37 ` Cyrill Gorcunov
2010-11-01 15:45 ` Don Zickus
0 siblings, 2 replies; 6+ messages in thread
From: Jan Kiszka @ 2010-11-01 12:39 UTC (permalink / raw)
To: Don Zickus
Cc: Ingo Molnar, Peter Zijlstra, Cyrill Gorcunov, Eric Paris,
Randy Dunlap, Frederic Weisbecker, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 2073 bytes --]
Hi,
I was getting this BUG while running into a GPF:
BUG: using smp_processor_id() in preemptible [00000000] code: qemu-system-x86/2248
caller is arch_trigger_all_cpu_backtrace_handler+0x1d/0xf7
Pid: 2248, comm: qemu-system-x86 Not tainted 2.6.36+ #12
Call Trace:
[<ffffffff81203194>] debug_smp_processor_id+0xd8/0xf4
[<ffffffff813803ab>] arch_trigger_all_cpu_backtrace_handler+0x1d/0xf7
[<ffffffff81381e64>] notifier_call_chain+0xa4/0xdb
[<ffffffff81381efe>] __atomic_notifier_call_chain+0x63/0x95
[<ffffffff81381e9b>] ? __atomic_notifier_call_chain+0x0/0x95
[<ffffffff81381cfb>] ? sub_preempt_count+0x97/0xaa
[<ffffffff8121de8c>] ? pfn_to_dma_pte+0x73/0x190
[<ffffffff81381f44>] atomic_notifier_call_chain+0x14/0x16
[<ffffffff81381f74>] notify_die+0x2e/0x30
[<ffffffff8137f498>] do_general_protection+0x121/0x142
[<ffffffff8137ec04>] ? irq_return+0x0/0xc
[<ffffffff8137ede5>] general_protection+0x25/0x30
[<ffffffff8121de8c>] ? pfn_to_dma_pte+0x73/0x190
[<ffffffff8121df5b>] ? pfn_to_dma_pte+0x142/0x190
[<ffffffff8121dfbe>] intel_iommu_iova_to_phys+0x15/0x2a
[<ffffffff812a9573>] iommu_iova_to_phys+0x13/0x15
[<ffffffffa04e91d0>] kvm_iommu_map_pages+0x77/0x194 [kvm]
[<ffffffff8111404d>] ? __vmalloc_node+0x86/0x9b
[<ffffffffa04e30e2>] __kvm_set_memory_region+0x4e5/0x787 [kvm]
[<ffffffff81081ee8>] ? mark_held_locks+0x50/0x72
[<ffffffff8137c983>] ? mutex_lock_nested+0x325/0x34d
[<ffffffffa04e33bb>] kvm_set_memory_region+0x37/0x50 [kvm]
[<ffffffffa04e4c15>] kvm_vm_ioctl_set_memory_region+0x18/0x1a [kvm]
[<ffffffffa04e4e44>] kvm_vm_ioctl+0x22d/0x3b1 [kvm]
[<ffffffff811355aa>] ? fget_light+0x17b/0x31f
[<ffffffff81143bd7>] do_vfs_ioctl+0x4c6/0x507
[<ffffffff81135732>] ? fget_light+0x303/0x31f
[<ffffffff811355aa>] ? fget_light+0x17b/0x31f
[<ffffffff8137ebb9>] ? retint_swapgs+0x13/0x1b
[<ffffffff81143c6e>] sys_ioctl+0x56/0x7c
[<ffffffff81002df2>] system_call_fastpath+0x16/0x1b
I guess this is not intended to trigger here, specifically as it showed
up first and may be misinterpreted as the core of the issue.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler
2010-11-01 12:39 BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler Jan Kiszka
@ 2010-11-01 15:37 ` Cyrill Gorcunov
2010-11-01 15:45 ` Don Zickus
1 sibling, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2010-11-01 15:37 UTC (permalink / raw)
To: Jan Kiszka
Cc: Don Zickus, Ingo Molnar, Peter Zijlstra, Eric Paris, Randy Dunlap,
Frederic Weisbecker, Linux Kernel Mailing List
On Mon, Nov 01, 2010 at 01:39:56PM +0100, Jan Kiszka wrote:
> Hi,
>
> I was getting this BUG while running into a GPF:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: qemu-system-x86/2248
> caller is arch_trigger_all_cpu_backtrace_handler+0x1d/0xf7
> Pid: 2248, comm: qemu-system-x86 Not tainted 2.6.36+ #12
> Call Trace:
> [<ffffffff81203194>] debug_smp_processor_id+0xd8/0xf4
> [<ffffffff813803ab>] arch_trigger_all_cpu_backtrace_handler+0x1d/0xf7
...
Thanks for report Jan. I'll take a look as only get ability.
Cyrill
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler
2010-11-01 12:39 BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler Jan Kiszka
2010-11-01 15:37 ` Cyrill Gorcunov
@ 2010-11-01 15:45 ` Don Zickus
2010-11-01 15:51 ` Cyrill Gorcunov
1 sibling, 1 reply; 6+ messages in thread
From: Don Zickus @ 2010-11-01 15:45 UTC (permalink / raw)
To: Jan Kiszka
Cc: Ingo Molnar, Peter Zijlstra, Cyrill Gorcunov, Eric Paris,
Randy Dunlap, Frederic Weisbecker, Linux Kernel Mailing List
On Mon, Nov 01, 2010 at 01:39:56PM +0100, Jan Kiszka wrote:
> Hi,
>
> I was getting this BUG while running into a GPF:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: qemu-system-x86/2248
> caller is arch_trigger_all_cpu_backtrace_handler+0x1d/0xf7
> Pid: 2248, comm: qemu-system-x86 Not tainted 2.6.36+ #12
> Call Trace:
> [<ffffffff81203194>] debug_smp_processor_id+0xd8/0xf4
> [<ffffffff813803ab>] arch_trigger_all_cpu_backtrace_handler+0x1d/0xf7
> [<ffffffff81381e64>] notifier_call_chain+0xa4/0xdb
> [<ffffffff81381efe>] __atomic_notifier_call_chain+0x63/0x95
> [<ffffffff81381e9b>] ? __atomic_notifier_call_chain+0x0/0x95
> [<ffffffff81381cfb>] ? sub_preempt_count+0x97/0xaa
> [<ffffffff8121de8c>] ? pfn_to_dma_pte+0x73/0x190
> [<ffffffff81381f44>] atomic_notifier_call_chain+0x14/0x16
> [<ffffffff81381f74>] notify_die+0x2e/0x30
> [<ffffffff8137f498>] do_general_protection+0x121/0x142
> [<ffffffff81002df2>] system_call_fastpath+0x16/0x1b
<snip>
>
> I guess this is not intended to trigger here, specifically as it showed
> up first and may be misinterpreted as the core of the issue.
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;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler
2010-11-01 15:45 ` Don Zickus
@ 2010-11-01 15:51 ` Cyrill Gorcunov
2010-11-05 16:06 ` Frederic Weisbecker
0 siblings, 1 reply; 6+ messages in thread
From: Cyrill Gorcunov @ 2010-11-01 15:51 UTC (permalink / raw)
To: Don Zickus
Cc: Jan Kiszka, Ingo Molnar, Peter Zijlstra, Eric Paris, Randy Dunlap,
Frederic Weisbecker, Linux Kernel Mailing List
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.
Cyrill
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler
2010-11-01 15:51 ` Cyrill Gorcunov
@ 2010-11-05 16:06 ` Frederic Weisbecker
2010-11-05 16:37 ` Cyrill Gorcunov
0 siblings, 1 reply; 6+ messages in thread
From: Frederic Weisbecker @ 2010-11-05 16:06 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Don Zickus, Jan Kiszka, Ingo Molnar, Peter Zijlstra, Eric Paris,
Randy Dunlap, Linux Kernel Mailing List
2010/11/1 Cyrill Gorcunov <gorcunov@gmail.com>:
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler
2010-11-05 16:06 ` Frederic Weisbecker
@ 2010-11-05 16:37 ` Cyrill Gorcunov
0 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2010-11-05 16:37 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Don Zickus, Jan Kiszka, Ingo Molnar, Peter Zijlstra, Eric Paris,
Randy Dunlap, Linux Kernel Mailing List
On Fri, Nov 05, 2010 at 05:06:55PM +0100, Frederic Weisbecker wrote:
> 2010/11/1 Cyrill Gorcunov <gorcunov@gmail.com>:
...
> > 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.
>
It will make it more straight and clean as minimum which is already
quite a benefit ;) There were patches floating from Don with notify
priorities which are good start. Though I didn't manage to look into
most of patches I've been CC'ed yet :(
> 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, ....
>
As the comment says on top of the enum they are "Grossly misnamed" ;)
Though we could consider them not as "dying" but rather in terms of say
on-die signals (or something like that).
> 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.
>
Cyrill
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-05 16:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-01 12:39 BUG: using smp_processor_id() in preemptible arch_trigger_all_cpu_backtrace_handler Jan Kiszka
2010-11-01 15:37 ` Cyrill Gorcunov
2010-11-01 15:45 ` Don Zickus
2010-11-01 15:51 ` Cyrill Gorcunov
2010-11-05 16:06 ` Frederic Weisbecker
2010-11-05 16:37 ` Cyrill Gorcunov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox