* [PATCH 1/1] ipr: improve interrupt service routine performance [not found] <20100519184827.824039221@linux.vnet.ibm.com> @ 2010-05-19 18:56 ` Wayne Boyer 2010-05-20 13:56 ` Brian King 0 siblings, 1 reply; 4+ messages in thread From: Wayne Boyer @ 2010-05-19 18:56 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, Brian King During performance testing on P7 machines it was observed that the interrupt service routine was doing unnecessary MMIO operations. This patch rearranges the logic of the routine and moves some of the code out of the main routine. The result is that there are now fewer MMIO operations in the performance path of the code. Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com> --- drivers/scsi/ipr.c | 63 +++++++++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 33 deletions(-) Index: b/drivers/scsi/ipr.c =================================================================== --- a/drivers/scsi/ipr.c 2010-05-18 09:17:00.000000000 -0700 +++ b/drivers/scsi/ipr.c 2010-05-19 11:44:18.000000000 -0700 @@ -4815,15 +4815,39 @@ static int ipr_eh_abort(struct scsi_cmnd /** * ipr_handle_other_interrupt - Handle "other" interrupts * @ioa_cfg: ioa config struct - * @int_reg: interrupt register * * Return value: * IRQ_NONE / IRQ_HANDLED **/ -static irqreturn_t ipr_handle_other_interrupt(struct ipr_ioa_cfg *ioa_cfg, - volatile u32 int_reg) +static irqreturn_t ipr_handle_other_interrupt(struct ipr_ioa_cfg *ioa_cfg) { irqreturn_t rc = IRQ_HANDLED; + volatile u32 int_reg, int_mask_reg; + + int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg32); + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; + + /* If an interrupt on the adapter did not occur, ignore it. + * Or in the case of SIS 64, check for a stage change interrupt. + */ + if ((int_reg & IPR_PCII_OPER_INTERRUPTS) == 0) { + if (ioa_cfg->sis64) { + int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg); + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; + if (int_reg & IPR_PCII_IPL_STAGE_CHANGE) { + + /* clear stage change */ + writel(IPR_PCII_IPL_STAGE_CHANGE, ioa_cfg->regs.clr_interrupt_reg); + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; + list_del(&ioa_cfg->reset_cmd->queue); + del_timer(&ioa_cfg->reset_cmd->timer); + ipr_reset_ioa_job(ioa_cfg->reset_cmd); + return IRQ_HANDLED; + } + } + + return IRQ_NONE; + } if (int_reg & IPR_PCII_IOA_TRANS_TO_OPER) { /* Mask the interrupt */ @@ -4884,7 +4908,7 @@ static irqreturn_t ipr_isr(int irq, void { struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)devp; unsigned long lock_flags = 0; - volatile u32 int_reg, int_mask_reg; + volatile u32 int_reg; u32 ioasc; u16 cmd_index; int num_hrrq = 0; @@ -4899,33 +4923,6 @@ static irqreturn_t ipr_isr(int irq, void return IRQ_NONE; } - int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg32); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; - - /* If an interrupt on the adapter did not occur, ignore it. - * Or in the case of SIS 64, check for a stage change interrupt. - */ - if (unlikely((int_reg & IPR_PCII_OPER_INTERRUPTS) == 0)) { - if (ioa_cfg->sis64) { - int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; - if (int_reg & IPR_PCII_IPL_STAGE_CHANGE) { - - /* clear stage change */ - writel(IPR_PCII_IPL_STAGE_CHANGE, ioa_cfg->regs.clr_interrupt_reg); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; - list_del(&ioa_cfg->reset_cmd->queue); - del_timer(&ioa_cfg->reset_cmd->timer); - ipr_reset_ioa_job(ioa_cfg->reset_cmd); - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); - return IRQ_HANDLED; - } - } - - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); - return IRQ_NONE; - } - while (1) { ipr_cmd = NULL; @@ -4965,7 +4962,7 @@ static irqreturn_t ipr_isr(int irq, void /* Clear the PCI interrupt */ do { writel(IPR_PCII_HRRQ_UPDATED, ioa_cfg->regs.clr_interrupt_reg32); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32); } while (int_reg & IPR_PCII_HRRQ_UPDATED && num_hrrq++ < IPR_MAX_HRRQ_RETRIES); @@ -4980,7 +4977,7 @@ static irqreturn_t ipr_isr(int irq, void } if (unlikely(rc == IRQ_NONE)) - rc = ipr_handle_other_interrupt(ioa_cfg, int_reg); + rc = ipr_handle_other_interrupt(ioa_cfg); spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); return rc; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] ipr: improve interrupt service routine performance 2010-05-19 18:56 ` [PATCH 1/1] ipr: improve interrupt service routine performance Wayne Boyer @ 2010-05-20 13:56 ` Brian King 0 siblings, 0 replies; 4+ messages in thread From: Brian King @ 2010-05-20 13:56 UTC (permalink / raw) To: Wayne Boyer; +Cc: James Bottomley, linux-scsi Acked-by: Brian King <brking@linux.vnet.ibm.com> On 05/19/2010 01:56 PM, Wayne Boyer wrote: > During performance testing on P7 machines it was observed that the interrupt > service routine was doing unnecessary MMIO operations. > > This patch rearranges the logic of the routine and moves some of the code out > of the main routine. The result is that there are now fewer MMIO operations in > the performance path of the code. > > Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com> > --- > > drivers/scsi/ipr.c | 63 +++++++++++++++++++++++++---------------------------- > 1 file changed, 30 insertions(+), 33 deletions(-) > > Index: b/drivers/scsi/ipr.c > =================================================================== > --- a/drivers/scsi/ipr.c 2010-05-18 09:17:00.000000000 -0700 > +++ b/drivers/scsi/ipr.c 2010-05-19 11:44:18.000000000 -0700 > @@ -4815,15 +4815,39 @@ static int ipr_eh_abort(struct scsi_cmnd > /** > * ipr_handle_other_interrupt - Handle "other" interrupts > * @ioa_cfg: ioa config struct > - * @int_reg: interrupt register > * > * Return value: > * IRQ_NONE / IRQ_HANDLED > **/ > -static irqreturn_t ipr_handle_other_interrupt(struct ipr_ioa_cfg *ioa_cfg, > - volatile u32 int_reg) > +static irqreturn_t ipr_handle_other_interrupt(struct ipr_ioa_cfg *ioa_cfg) > { > irqreturn_t rc = IRQ_HANDLED; > + volatile u32 int_reg, int_mask_reg; > + > + int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg32); > + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; > + > + /* If an interrupt on the adapter did not occur, ignore it. > + * Or in the case of SIS 64, check for a stage change interrupt. > + */ > + if ((int_reg & IPR_PCII_OPER_INTERRUPTS) == 0) { > + if (ioa_cfg->sis64) { > + int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg); > + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; > + if (int_reg & IPR_PCII_IPL_STAGE_CHANGE) { > + > + /* clear stage change */ > + writel(IPR_PCII_IPL_STAGE_CHANGE, ioa_cfg->regs.clr_interrupt_reg); > + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; > + list_del(&ioa_cfg->reset_cmd->queue); > + del_timer(&ioa_cfg->reset_cmd->timer); > + ipr_reset_ioa_job(ioa_cfg->reset_cmd); > + return IRQ_HANDLED; > + } > + } > + > + return IRQ_NONE; > + } > > if (int_reg & IPR_PCII_IOA_TRANS_TO_OPER) { > /* Mask the interrupt */ > @@ -4884,7 +4908,7 @@ static irqreturn_t ipr_isr(int irq, void > { > struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)devp; > unsigned long lock_flags = 0; > - volatile u32 int_reg, int_mask_reg; > + volatile u32 int_reg; > u32 ioasc; > u16 cmd_index; > int num_hrrq = 0; > @@ -4899,33 +4923,6 @@ static irqreturn_t ipr_isr(int irq, void > return IRQ_NONE; > } > > - int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg32); > - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; > - > - /* If an interrupt on the adapter did not occur, ignore it. > - * Or in the case of SIS 64, check for a stage change interrupt. > - */ > - if (unlikely((int_reg & IPR_PCII_OPER_INTERRUPTS) == 0)) { > - if (ioa_cfg->sis64) { > - int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg); > - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; > - if (int_reg & IPR_PCII_IPL_STAGE_CHANGE) { > - > - /* clear stage change */ > - writel(IPR_PCII_IPL_STAGE_CHANGE, ioa_cfg->regs.clr_interrupt_reg); > - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; > - list_del(&ioa_cfg->reset_cmd->queue); > - del_timer(&ioa_cfg->reset_cmd->timer); > - ipr_reset_ioa_job(ioa_cfg->reset_cmd); > - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); > - return IRQ_HANDLED; > - } > - } > - > - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); > - return IRQ_NONE; > - } > - > while (1) { > ipr_cmd = NULL; > > @@ -4965,7 +4962,7 @@ static irqreturn_t ipr_isr(int irq, void > /* Clear the PCI interrupt */ > do { > writel(IPR_PCII_HRRQ_UPDATED, ioa_cfg->regs.clr_interrupt_reg32); > - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; > + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32); > } while (int_reg & IPR_PCII_HRRQ_UPDATED && > num_hrrq++ < IPR_MAX_HRRQ_RETRIES); > > @@ -4980,7 +4977,7 @@ static irqreturn_t ipr_isr(int irq, void > } > > if (unlikely(rc == IRQ_NONE)) > - rc = ipr_handle_other_interrupt(ioa_cfg, int_reg); > + rc = ipr_handle_other_interrupt(ioa_cfg); > > spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); > return rc; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Brian King Linux on Power Virtualization IBM Linux Technology Center ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20110407202324.649117389@linux.vnet.ibm.com>]
* [PATCH 1/1] ipr: improve interrupt service routine performance [not found] <20110407202324.649117389@linux.vnet.ibm.com> @ 2011-04-12 17:29 ` Wayne Boyer 2011-04-22 18:26 ` Brian King 0 siblings, 1 reply; 4+ messages in thread From: Wayne Boyer @ 2011-04-12 17:29 UTC (permalink / raw) To: James Bottomley; +Cc: Brian King, linux-scsi During performance testing on P7 machines it was observed that the interrupt service routine was doing unnecessary MMIO operations. This patch rearranges the logic of the routine and moves some of the code out of the main routine. The result is that there are now fewer MMIO operations in the performance path of the code. As a result of the above change, an existing condition was exposed where the driver could get an "unexpected" hrrq interrupt. The original code would flag the interrupt as unexpected and then reset the adapter. After further analysis it was confirmed that this condition can occasionally occur and that the interrupt can safely be ignored. Additional code in this patch detects this condition, clears the interrupt and allows the driver to continue without resetting the adapter. Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com> --- drivers/scsi/ipr.c | 68 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 29 deletions(-) Index: b/drivers/scsi/ipr.c =================================================================== --- a/drivers/scsi/ipr.c 2011-04-07 12:08:00.519593071 -0700 +++ b/drivers/scsi/ipr.c 2011-04-07 12:46:35.289623710 -0700 @@ -4956,6 +4956,32 @@ static irqreturn_t ipr_handle_other_inte u32 int_reg) { irqreturn_t rc = IRQ_HANDLED; + u32 int_mask_reg; + + int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg32); + int_reg &= ~int_mask_reg; + + /* If an interrupt on the adapter did not occur, ignore it. + * Or in the case of SIS 64, check for a stage change interrupt. + */ + if ((int_reg & IPR_PCII_OPER_INTERRUPTS) == 0) { + if (ioa_cfg->sis64) { + int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg); + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; + if (int_reg & IPR_PCII_IPL_STAGE_CHANGE) { + + /* clear stage change */ + writel(IPR_PCII_IPL_STAGE_CHANGE, ioa_cfg->regs.clr_interrupt_reg); + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; + list_del(&ioa_cfg->reset_cmd->queue); + del_timer(&ioa_cfg->reset_cmd->timer); + ipr_reset_ioa_job(ioa_cfg->reset_cmd); + return IRQ_HANDLED; + } + } + + return IRQ_NONE; + } if (int_reg & IPR_PCII_IOA_TRANS_TO_OPER) { /* Mask the interrupt */ @@ -4968,6 +4994,13 @@ static irqreturn_t ipr_handle_other_inte list_del(&ioa_cfg->reset_cmd->queue); del_timer(&ioa_cfg->reset_cmd->timer); ipr_reset_ioa_job(ioa_cfg->reset_cmd); + } else if ((int_reg & IPR_PCII_HRRQ_UPDATED) == int_reg) { + if (ipr_debug && printk_ratelimit()) + dev_err(&ioa_cfg->pdev->dev, + "Spurious interrupt detected. 0x%08X\n", int_reg); + writel(IPR_PCII_HRRQ_UPDATED, ioa_cfg->regs.clr_interrupt_reg32); + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32); + return IRQ_NONE; } else { if (int_reg & IPR_PCII_IOA_UNIT_CHECKED) ioa_cfg->ioa_unit_checked = 1; @@ -5016,10 +5049,11 @@ static irqreturn_t ipr_isr(int irq, void { struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)devp; unsigned long lock_flags = 0; - u32 int_reg, int_mask_reg; + u32 int_reg = 0; u32 ioasc; u16 cmd_index; int num_hrrq = 0; + int irq_none = 0; struct ipr_cmnd *ipr_cmd; irqreturn_t rc = IRQ_NONE; @@ -5031,33 +5065,6 @@ static irqreturn_t ipr_isr(int irq, void return IRQ_NONE; } - int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg32); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; - - /* If an interrupt on the adapter did not occur, ignore it. - * Or in the case of SIS 64, check for a stage change interrupt. - */ - if (unlikely((int_reg & IPR_PCII_OPER_INTERRUPTS) == 0)) { - if (ioa_cfg->sis64) { - int_mask_reg = readl(ioa_cfg->regs.sense_interrupt_mask_reg); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; - if (int_reg & IPR_PCII_IPL_STAGE_CHANGE) { - - /* clear stage change */ - writel(IPR_PCII_IPL_STAGE_CHANGE, ioa_cfg->regs.clr_interrupt_reg); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg) & ~int_mask_reg; - list_del(&ioa_cfg->reset_cmd->queue); - del_timer(&ioa_cfg->reset_cmd->timer); - ipr_reset_ioa_job(ioa_cfg->reset_cmd); - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); - return IRQ_HANDLED; - } - } - - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); - return IRQ_NONE; - } - while (1) { ipr_cmd = NULL; @@ -5097,7 +5104,7 @@ static irqreturn_t ipr_isr(int irq, void /* Clear the PCI interrupt */ do { writel(IPR_PCII_HRRQ_UPDATED, ioa_cfg->regs.clr_interrupt_reg32); - int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32) & ~int_mask_reg; + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32); } while (int_reg & IPR_PCII_HRRQ_UPDATED && num_hrrq++ < IPR_MAX_HRRQ_RETRIES); @@ -5107,6 +5114,9 @@ static irqreturn_t ipr_isr(int irq, void return IRQ_HANDLED; } + } else if (rc == IRQ_NONE && irq_none == 0) { + int_reg = readl(ioa_cfg->regs.sense_interrupt_reg32); + irq_none++; } else break; } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] ipr: improve interrupt service routine performance 2011-04-12 17:29 ` Wayne Boyer @ 2011-04-22 18:26 ` Brian King 0 siblings, 0 replies; 4+ messages in thread From: Brian King @ 2011-04-22 18:26 UTC (permalink / raw) To: Wayne Boyer; +Cc: James Bottomley, linux-scsi Acked-by: Brian King <brking@linux.vnet.ibm.com> -- Brian King Linux on Power Virtualization IBM Linux Technology Center ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-04-22 18:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20100519184827.824039221@linux.vnet.ibm.com>
2010-05-19 18:56 ` [PATCH 1/1] ipr: improve interrupt service routine performance Wayne Boyer
2010-05-20 13:56 ` Brian King
[not found] <20110407202324.649117389@linux.vnet.ibm.com>
2011-04-12 17:29 ` Wayne Boyer
2011-04-22 18:26 ` Brian King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).