Re: [PATCH 1/1] ipr: improve interrupt service routine performance

[Brian King <brking@linux.vnet.ibm.com>]

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