Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756213Ab0BBTr1 (ORCPT ); Tue, 2 Feb 2010 14:47:27 -0500 Received: from avexch1.qlogic.com ([198.70.193.115]:55061 "EHLO avexch1.qlogic.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756004Ab0BBTrZ (ORCPT ); Tue, 2 Feb 2010 14:47:25 -0500 Cc: "linux-scsi@vger.kernel.org" , Linux Driver , "linux-kernel@vger.kernel.org" , Andrew Vasquez , "James E.J. Bottomley" , Anirban Chakraborty , Santosh Vernekar Message-Id: <1B3FF4A7-1C85-4E27-B807-8053F0EA7BAB@qlogic.com> From: Giridhar Malavali To: Xiaotian Feng In-Reply-To: <1264759770-4093-1-git-send-email-dfeng@redhat.com> Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Mime-Version: 1.0 (Apple Message framework v912) Subject: Re: [PATCH] qla2xxx: make msix interrupt handler safe for irq Date: Tue, 2 Feb 2010 11:47:23 -0800 References: <1264759770-4093-1-git-send-email-dfeng@redhat.com> X-Mailer: Apple Mail (2.912) X-OriginalArrivalTime: 02 Feb 2010 19:47:23.0927 (UTC) FILETIME=[8A568A70:01CAA440] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5488 Lines: 144 Acked-by: Giridhar Malavali On Jan 29, 2010, at 2:09 AM, Xiaotian Feng wrote: > Yinghai has reported a lockdep warning on qla2xxx: > > [ 77.965784] WARNING: at kernel/lockdep.c:2332 > trace_hardirqs_on_caller+0xc6/0x14b() > [ 77.977492] Hardware name: Sun > [ 77.979485] Modules linked in: > [ 77.994337] Pid: 0, comm: swapper Not tainted > 2.6.33-rc4-tip-yh-03949-g3a8e3f5-dirty #64 > [ 78.000120] Call Trace: > [ 78.013298] [] warn_slowpath_common > +0x7c/0x94 > [ 78.017746] [] ? _raw_spin_unlock_irq+0x30/0x36 > [ 78.035171] [] warn_slowpath_null+0x14/0x16 > [ 78.040152] [] trace_hardirqs_on_caller > +0xc6/0x14b > [ 78.055400] [] trace_hardirqs_on+0xd/0xf > [ 78.058951] [] _raw_spin_unlock_irq+0x30/0x36 > [ 78.074889] [] qla24xx_msix_default+0x243/0x281 > [ 78.091598] [] ? __lock_release+0xa5/0xae > [ 78.096799] [] handle_IRQ_event+0x53/0x113 > [ 78.111568] [] handle_edge_irq+0xf3/0x13b > [ 78.116255] [] handle_irq+0x24/0x2f > [ 78.132063] [] do_IRQ+0x5c/0xc3 > [ 78.134684] [] ret_from_intr+0x0/0xf > [ 78.137903] [] ? mwait_idle+0xaf/0xbb > [ 78.155674] [] ? mwait_idle+0xa6/0xbb > [ 78.158600] [] cpu_idle+0x61/0xa1 > [ 78.174333] [] rest_init+0x7e/0x80 > [ 78.178122] [] start_kernel+0x316/0x31d > [ 78.193623] [] x86_64_start_reservations > +0xa7/0xab > [ 78.198924] [] x86_64_start_kernel+0xe4/0xeb > [ 78.214540] ---[ end trace be4529f30a2e4ef5 ]--- > > This was happened when qla2xxx msix interrupt handler is trying to > enable > IRQs by spin_unlock_irq(). We should make interrupt handler safe for > IRQs, > use spin_lock_irqsave/spin_unlock_irqrestore, this will not break > the IRQs > status in interrupt handler. > > Reported-by: Yinghai Lu > Signed-off-by: Xiaotian Feng > Cc: Andrew Vasquez > Cc: James E.J. Bottomley > Cc: Giridhar Malavali > Cc: Anirban Chakraborty > Cc: Santosh Vernekar > --- > drivers/scsi/qla2xxx/qla_isr.c | 15 +++++++++------ > 1 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/ > qla_isr.c > index ffd0efd..0ced91c 100644 > --- a/drivers/scsi/qla2xxx/qla_isr.c > +++ b/drivers/scsi/qla2xxx/qla_isr.c > @@ -1917,6 +1917,7 @@ qla24xx_msix_rsp_q(int irq, void *dev_id) > struct rsp_que *rsp; > struct device_reg_24xx __iomem *reg; > struct scsi_qla_host *vha; > + unsigned long flags; > > rsp = (struct rsp_que *) dev_id; > if (!rsp) { > @@ -1927,7 +1928,7 @@ qla24xx_msix_rsp_q(int irq, void *dev_id) > ha = rsp->hw; > reg = &ha->iobase->isp24; > > - spin_lock_irq(&ha->hardware_lock); > + spin_lock_irqsave(&ha->hardware_lock, flags); > > vha = qla25xx_get_host(rsp); > qla24xx_process_response_queue(vha, rsp); > @@ -1935,7 +1936,7 @@ qla24xx_msix_rsp_q(int irq, void *dev_id) > WRT_REG_DWORD(®->hccr, HCCRX_CLR_RISC_INT); > RD_REG_DWORD_RELAXED(®->hccr); > } > - spin_unlock_irq(&ha->hardware_lock); > + spin_unlock_irqrestore(&ha->hardware_lock, flags); > > return IRQ_HANDLED; > } > @@ -1946,6 +1947,7 @@ qla25xx_msix_rsp_q(int irq, void *dev_id) > struct qla_hw_data *ha; > struct rsp_que *rsp; > struct device_reg_24xx __iomem *reg; > + unsigned long flags; > > rsp = (struct rsp_que *) dev_id; > if (!rsp) { > @@ -1958,10 +1960,10 @@ qla25xx_msix_rsp_q(int irq, void *dev_id) > /* Clear the interrupt, if enabled, for this response queue */ > if (rsp->options & ~BIT_6) { > reg = &ha->iobase->isp24; > - spin_lock_irq(&ha->hardware_lock); > + spin_lock_irqsave(&ha->hardware_lock, flags); > WRT_REG_DWORD(®->hccr, HCCRX_CLR_RISC_INT); > RD_REG_DWORD_RELAXED(®->hccr); > - spin_unlock_irq(&ha->hardware_lock); > + spin_unlock_irqrestore(&ha->hardware_lock, flags); > } > queue_work_on((int) (rsp->id - 1), ha->wq, &rsp->q_work); > > @@ -1979,6 +1981,7 @@ qla24xx_msix_default(int irq, void *dev_id) > uint32_t stat; > uint32_t hccr; > uint16_t mb[4]; > + unsigned long flags; > > rsp = (struct rsp_que *) dev_id; > if (!rsp) { > @@ -1990,7 +1993,7 @@ qla24xx_msix_default(int irq, void *dev_id) > reg = &ha->iobase->isp24; > status = 0; > > - spin_lock_irq(&ha->hardware_lock); > + spin_lock_irqsave(&ha->hardware_lock, flags); > vha = pci_get_drvdata(ha->pdev); > do { > stat = RD_REG_DWORD(®->host_status); > @@ -2039,7 +2042,7 @@ qla24xx_msix_default(int irq, void *dev_id) > } > WRT_REG_DWORD(®->hccr, HCCRX_CLR_RISC_INT); > } while (0); > - spin_unlock_irq(&ha->hardware_lock); > + spin_unlock_irqrestore(&ha->hardware_lock, flags); > > if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) && > (status & MBX_INTERRUPT) && ha->flags.mbox_int) { > -- > 1.6.6 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/