From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751476AbZKCNpa (ORCPT ); Tue, 3 Nov 2009 08:45:30 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750838AbZKCNp3 (ORCPT ); Tue, 3 Nov 2009 08:45:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51303 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750745AbZKCNp2 (ORCPT ); Tue, 3 Nov 2009 08:45:28 -0500 Date: Tue, 3 Nov 2009 08:45:32 -0500 From: Prarit Bhargava To: linux-kernel@vger.kernel.org, ebiederm@xmission.com, akpm@linux-foundation.org Cc: Prarit Bhargava Message-Id: <20091103134342.24414.82986.sendpatchset@prarit.bos.redhat.com> Subject: [PATCH]: use spin_lock_irqsave in try_one_irq() Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org (Prarit: Second try at this one, not sure if this made it to LKML or not. Sending to a wider audience this time) Booting 2.6.32-rc5 on some IBM systems results in Disabling IRQ #19 ================================= [ INFO: inconsistent lock state ] 2.6.32-rc5 #1 --------------------------------- inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes: (&irq_desc_lock_class){?.-...}, at: [] try_one_irq+0x32/0x138 {IN-HARDIRQ-W} state was registered at: [] __lock_acquire+0x2fc/0xd5d [] lock_acquire+0xf3/0x12d [] _spin_lock+0x40/0x89 [] handle_level_irq+0x30/0x105 [] handle_irq+0x95/0xb7 [] do_IRQ+0x6a/0xe0 [] ret_from_intr+0x0/0x16 irq event stamp: 195096 hardirqs last enabled at (195096): [] _spin_unlock_irq+0x3a/0x5c hardirqs last disabled at (195095): [] _spin_lock_irq+0x29/0x95 softirqs last enabled at (195088): [] __do_softirq+0x1c1/0x1ef softirqs last disabled at (195093): [] call_softirq+0x1c/0x30 other info that might help us debug this: 1 lock held by swapper/0: #0: (kernel/irq/spurious.c:21){+.-...}, at: [] run_timer_softirq+0x1a9/0x315 stack backtrace: Pid: 0, comm: swapper Not tainted 2.6.32-rc5 #1 Call Trace: [] valid_state+0x187/0x1ae [] ? check_usage_backwards+0x0/0xa3 [] mark_lock+0x129/0x253 [] __lock_acquire+0x370/0xd5d [] ? try_one_irq+0x32/0x138 [] ? save_trace+0x4e/0xcd [] lock_acquire+0xf3/0x12d [] ? try_one_irq+0x32/0x138 [] ? run_timer_softirq+0x1a9/0x315 [] ? try_one_irq+0x32/0x138 [] _spin_lock+0x40/0x89 [] ? try_one_irq+0x32/0x138 [] try_one_irq+0x32/0x138 [] poll_all_shared_irqs+0x41/0x6d [] poll_spurious_irqs+0x1c/0x49 [] run_timer_softirq+0x239/0x315 [] ? run_timer_softirq+0x1a9/0x315 [] ? poll_spurious_irqs+0x0/0x49 [] __do_softirq+0x102/0x1ef [] ? tick_dev_program_event+0x46/0xcc [] call_softirq+0x1c/0x30 [] do_softirq+0x59/0xca [] irq_exit+0x58/0xae [] smp_apic_timer_interrupt+0x94/0xba [] apic_timer_interrupt+0x13/0x20 [] ? mwait_idle+0x8c/0xb5 [] ? mwait_idle+0x83/0xb5 [] ? cpu_idle+0xbe/0x100 [] ? start_secondary+0x219/0x270 This happens because the &desc->lock is taken with spin_lock_irqsave and just a spin_lock. In the try_one_irq(), this lock really should be a spin_lock_irqsave(). I have not yet narrowed down the reason for the spurious interrupt (although I suspect it maybe to do with the radeon driver). Successfully tested by me. Signed-off-by: Prarit Bhargava --- linux-2.6.31.x86_64.orig/kernel/irq/spurious.c 2009-09-09 18:13:59.000000000 -0400 +++ linux-2.6.31.x86_64/kernel/irq/spurious.c 2009-10-26 10:55:56.709845786 -0400 @@ -27,8 +27,9 @@ static int try_one_irq(int irq, struct i { struct irqaction *action; int ok = 0, work = 0; + unsigned long flags; - spin_lock(&desc->lock); + spin_lock_irqsave(&desc->lock, flags); /* Already running on another processor */ if (desc->status & IRQ_INPROGRESS) { /* @@ -37,13 +38,13 @@ static int try_one_irq(int irq, struct i */ if (desc->action && (desc->action->flags & IRQF_SHARED)) desc->status |= IRQ_PENDING; - spin_unlock(&desc->lock); + spin_unlock_irqrestore(&desc->lock, flags); return ok; } /* Honour the normal IRQ locking */ desc->status |= IRQ_INPROGRESS; action = desc->action; - spin_unlock(&desc->lock); + spin_unlock_irqrestore(&desc->lock, flags); while (action) { /* Only shared IRQ handlers are safe to call */ @@ -56,7 +57,7 @@ static int try_one_irq(int irq, struct i } local_irq_disable(); /* Now clean up the flags */ - spin_lock(&desc->lock); + spin_lock_irqsave(&desc->lock, flags); action = desc->action; /* @@ -68,9 +69,9 @@ static int try_one_irq(int irq, struct i * Perform real IRQ processing for the IRQ we deferred */ work = 1; - spin_unlock(&desc->lock); + spin_unlock_irqrestore(&desc->lock, flags); handle_IRQ_event(irq, action); - spin_lock(&desc->lock); + spin_lock_irqsave(&desc->lock, flags); desc->status &= ~IRQ_PENDING; } desc->status &= ~IRQ_INPROGRESS; @@ -80,7 +81,7 @@ static int try_one_irq(int irq, struct i */ if (work && desc->chip && desc->chip->end) desc->chip->end(irq); - spin_unlock(&desc->lock); + spin_unlock_irqrestore(&desc->lock, flags); return ok; }