Re: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set
On 06/11/2015 03:42 AM, rajinikanth.panduran...@pmcs.com wrote: > From: Rajinikanth Pandurangan > > Description: > If 'IsFastPath' bit is set, then response path assumes no error > and skips error check. > > Signed-off-by: Rajinikanth Pandurangan > --- > drivers/scsi/aacraid/aachba.c | 259 > ++ > 1 file changed, 137 insertions(+), 122 deletions(-) > With this patch applied we get : if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { /* fast response */ srbreply->srb_status = cpu_to_le32(SRB_STATUS_SUCCESS); srbreply->scsi_status = cpu_to_le32(SAM_STAT_GOOD); } else { /* * Calculate resid for sg */ } /* * OR in the scsi status (already shifted up a bit) */ scsicmd->result |= le32_to_cpu(srbreply->scsi_status); aac_fib_complete(fibptr); aac_fib_free(fibptr); scsicmd->scsi_done(scsicmd); } >From that it looks like you do not need set srbreply->srb_status = cpu_to_le32(SRB_STATUS_SUCCESS); because it's never used later. You could you rearrange it to : if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { /* fast response */ srbreply->scsi_status = cpu_to_le32(SAM_STAT_GOOD); goto done; } /* * Calculate resid for sg */ done: /* * OR in the scsi status (already shifted up a bit) */ scsicmd->result |= le32_to_cpu(srbreply->scsi_status); aac_fib_complete(fibptr); aac_fib_free(fibptr); scsicmd->scsi_done(scsicmd); The advantage is that you don't need move ~150 lines one tab to the right and don't have to wrap lines because of the 80 char limit. > diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c > index fe59b00..864e9f6 100644 > --- a/drivers/scsi/aacraid/aachba.c > +++ b/drivers/scsi/aacraid/aachba.c > @@ -2977,11 +2977,16 @@ static void aac_srb_callback(void *context, struct > fib * fibptr) > return; > > BUG_ON(fibptr == NULL); > - > dev = fibptr->dev; > > - srbreply = (struct aac_srb_reply *) fib_data(fibptr); > + scsi_dma_unmap(scsicmd); > > + /* expose physical device if expose_physicald flag is on */ > + if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) > + && expose_physicals > 0) > + aac_expose_phy_device(scsicmd); > + > + srbreply = (struct aac_srb_reply *) fib_data(fibptr); > scsicmd->sense_buffer[0] = '\0'; /* Initialize sense valid flag to > false */ > > if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { > @@ -2994,147 +2999,157 @@ static void aac_srb_callback(void *context, struct > fib * fibptr) >*/ > scsi_set_resid(scsicmd, scsi_bufflen(scsicmd) > - le32_to_cpu(srbreply->data_xfer_length)); > - } > - > - scsi_dma_unmap(scsicmd); > - > - /* expose physical device if expose_physicald flag is on */ > - if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) > - && expose_physicals > 0) > - aac_expose_phy_device(scsicmd); > + /* > + * First check the fib status > + */ > > - /* > - * First check the fib status > - */ > + if (le32_to_cpu(srbreply->status) != ST_OK) { > + int len; > > - if (le32_to_cpu(srbreply->status) != ST_OK){ > - int len; > - printk(KERN_WARNING "aac_srb_callback: srb failed, status = > %d\n", le32_to_cpu(srbreply->status)); > - len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), > - SCSI_SENSE_BUFFERSIZE); > - scsicmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8 | > SAM_STAT_CHECK_CONDITION; > - memcpy(scsicmd->sense_buffer, srbreply->sense_data, len); > - } > + printk(KERN_WARNING "aac_srb_callback: srb failed, > status = %d\n", le32_to_cpu(srbreply->status)); > + len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), > + SCSI_SENSE_BUFFERSIZE); > + scsicmd->result = DID_ERROR << 16 > + | COMMAND_COMPLETE << 8 > + | SAM_STAT_CHECK_CONDITION; > + memcpy(scsicmd->sense_buffer, > + srbreply->sense_data, len); > + } > > - /* > - * Next check the srb status > - */ > - switch( (le32_to_cpu(srbreply->srb_status))&0x3f){ > - case SRB_STATUS_ERROR_RECOVERY: > - case SRB_STATUS_PENDING: > - case SRB_STATUS_SUCCESS: > - scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8; > -
RE: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set
Reviewed-by: Mahesh Rajashekhara -Original Message- From: Rajinikanth Pandurangan Sent: Thursday, June 11, 2015 7:12 AM To: jbottom...@parallels.com; linux-scsi@vger.kernel.org Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim Leubner; Murthy Bhat; Rajinikanth Pandurangan Subject: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set From: Rajinikanth Pandurangan Description: If 'IsFastPath' bit is set, then response path assumes no error and skips error check. Signed-off-by: Rajinikanth Pandurangan --- drivers/scsi/aacraid/aachba.c | 259 ++ 1 file changed, 137 insertions(+), 122 deletions(-) diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c index fe59b00..864e9f6 100644 --- a/drivers/scsi/aacraid/aachba.c +++ b/drivers/scsi/aacraid/aachba.c @@ -2977,11 +2977,16 @@ static void aac_srb_callback(void *context, struct fib * fibptr) return; BUG_ON(fibptr == NULL); - dev = fibptr->dev; - srbreply = (struct aac_srb_reply *) fib_data(fibptr); + scsi_dma_unmap(scsicmd); + /* expose physical device if expose_physicald flag is on */ + if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) + && expose_physicals > 0) + aac_expose_phy_device(scsicmd); + + srbreply = (struct aac_srb_reply *) fib_data(fibptr); scsicmd->sense_buffer[0] = '\0'; /* Initialize sense valid flag to false */ if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { @@ -2994,147 +2999,157 @@ static void aac_srb_callback(void *context, struct fib * fibptr) */ scsi_set_resid(scsicmd, scsi_bufflen(scsicmd) - le32_to_cpu(srbreply->data_xfer_length)); - } - - scsi_dma_unmap(scsicmd); - - /* expose physical device if expose_physicald flag is on */ - if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) - && expose_physicals > 0) - aac_expose_phy_device(scsicmd); + /* +* First check the fib status +*/ - /* -* First check the fib status -*/ + if (le32_to_cpu(srbreply->status) != ST_OK) { + int len; - if (le32_to_cpu(srbreply->status) != ST_OK){ - int len; - printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); - len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), - SCSI_SENSE_BUFFERSIZE); - scsicmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_CHECK_CONDITION; - memcpy(scsicmd->sense_buffer, srbreply->sense_data, len); - } + printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); + len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), + SCSI_SENSE_BUFFERSIZE); + scsicmd->result = DID_ERROR << 16 + | COMMAND_COMPLETE << 8 + | SAM_STAT_CHECK_CONDITION; + memcpy(scsicmd->sense_buffer, + srbreply->sense_data, len); + } - /* -* Next check the srb status -*/ - switch( (le32_to_cpu(srbreply->srb_status))&0x3f){ - case SRB_STATUS_ERROR_RECOVERY: - case SRB_STATUS_PENDING: - case SRB_STATUS_SUCCESS: - scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8; - break; - case SRB_STATUS_DATA_OVERRUN: - switch(scsicmd->cmnd[0]){ - case READ_6: - case WRITE_6: - case READ_10: - case WRITE_10: - case READ_12: - case WRITE_12: - case READ_16: - case WRITE_16: - if (le32_to_cpu(srbreply->data_xfer_length) < scsicmd->underflow) { - printk(KERN_WARNING"aacraid: SCSI CMD underflow\n"); - } else { - printk(KERN_WARNING"aacraid: SCSI CMD Data Overrun\n"); + /* +* Next check the srb status +*/ + switch ((le32_to_cpu(srbreply->srb_status))&0x3f) { + case SRB_STATUS_ERROR_RECOVERY: + case SRB_STATUS_PENDING: + case SRB_STATUS_SUCCESS: +
RE: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set
Hi, Any review comments on this patch. please let us known if any changes are required. Thanks, -Raj P. -Original Message- From: Rajinikanth Pandurangan Sent: Wednesday, June 10, 2015 6:42 PM To: jbottom...@parallels.com; linux-scsi@vger.kernel.org Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim Leubner; Murthy Bhat; Rajinikanth Pandurangan Subject: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set From: Rajinikanth Pandurangan Description: If 'IsFastPath' bit is set, then response path assumes no error and skips error check. Signed-off-by: Rajinikanth Pandurangan --- drivers/scsi/aacraid/aachba.c | 259 ++ 1 file changed, 137 insertions(+), 122 deletions(-) diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c index fe59b00..864e9f6 100644 --- a/drivers/scsi/aacraid/aachba.c +++ b/drivers/scsi/aacraid/aachba.c @@ -2977,11 +2977,16 @@ static void aac_srb_callback(void *context, struct fib * fibptr) return; BUG_ON(fibptr == NULL); - dev = fibptr->dev; - srbreply = (struct aac_srb_reply *) fib_data(fibptr); + scsi_dma_unmap(scsicmd); + /* expose physical device if expose_physicald flag is on */ + if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) + && expose_physicals > 0) + aac_expose_phy_device(scsicmd); + + srbreply = (struct aac_srb_reply *) fib_data(fibptr); scsicmd->sense_buffer[0] = '\0'; /* Initialize sense valid flag to false */ if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { @@ -2994,147 +2999,157 @@ static void aac_srb_callback(void *context, struct fib * fibptr) */ scsi_set_resid(scsicmd, scsi_bufflen(scsicmd) - le32_to_cpu(srbreply->data_xfer_length)); - } - - scsi_dma_unmap(scsicmd); - - /* expose physical device if expose_physicald flag is on */ - if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) - && expose_physicals > 0) - aac_expose_phy_device(scsicmd); + /* +* First check the fib status +*/ - /* -* First check the fib status -*/ + if (le32_to_cpu(srbreply->status) != ST_OK) { + int len; - if (le32_to_cpu(srbreply->status) != ST_OK){ - int len; - printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); - len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), - SCSI_SENSE_BUFFERSIZE); - scsicmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_CHECK_CONDITION; - memcpy(scsicmd->sense_buffer, srbreply->sense_data, len); - } + printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); + len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), + SCSI_SENSE_BUFFERSIZE); + scsicmd->result = DID_ERROR << 16 + | COMMAND_COMPLETE << 8 + | SAM_STAT_CHECK_CONDITION; + memcpy(scsicmd->sense_buffer, + srbreply->sense_data, len); + } - /* -* Next check the srb status -*/ - switch( (le32_to_cpu(srbreply->srb_status))&0x3f){ - case SRB_STATUS_ERROR_RECOVERY: - case SRB_STATUS_PENDING: - case SRB_STATUS_SUCCESS: - scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8; - break; - case SRB_STATUS_DATA_OVERRUN: - switch(scsicmd->cmnd[0]){ - case READ_6: - case WRITE_6: - case READ_10: - case WRITE_10: - case READ_12: - case WRITE_12: - case READ_16: - case WRITE_16: - if (le32_to_cpu(srbreply->data_xfer_length) < scsicmd->underflow) { - printk(KERN_WARNING"aacraid: SCSI CMD underflow\n"); - } else { - printk(KERN_WARNING"aacraid: SCSI CMD Data Overrun\n"); + /* +* Next check the srb status +*/ + switch ((le32_to_cpu(srbreply->srb_status))&0x3f) { + case SRB_STATUS_ERROR_RECOVERY: + case SRB_STATUS_PENDING: +
[Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set
From: Rajinikanth Pandurangan Description: If 'IsFastPath' bit is set, then response path assumes no error and skips error check. Signed-off-by: Rajinikanth Pandurangan --- drivers/scsi/aacraid/aachba.c | 259 ++ 1 file changed, 137 insertions(+), 122 deletions(-) diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c index fe59b00..864e9f6 100644 --- a/drivers/scsi/aacraid/aachba.c +++ b/drivers/scsi/aacraid/aachba.c @@ -2977,11 +2977,16 @@ static void aac_srb_callback(void *context, struct fib * fibptr) return; BUG_ON(fibptr == NULL); - dev = fibptr->dev; - srbreply = (struct aac_srb_reply *) fib_data(fibptr); + scsi_dma_unmap(scsicmd); + /* expose physical device if expose_physicald flag is on */ + if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) + && expose_physicals > 0) + aac_expose_phy_device(scsicmd); + + srbreply = (struct aac_srb_reply *) fib_data(fibptr); scsicmd->sense_buffer[0] = '\0'; /* Initialize sense valid flag to false */ if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { @@ -2994,147 +2999,157 @@ static void aac_srb_callback(void *context, struct fib * fibptr) */ scsi_set_resid(scsicmd, scsi_bufflen(scsicmd) - le32_to_cpu(srbreply->data_xfer_length)); - } - - scsi_dma_unmap(scsicmd); - - /* expose physical device if expose_physicald flag is on */ - if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01) - && expose_physicals > 0) - aac_expose_phy_device(scsicmd); + /* +* First check the fib status +*/ - /* -* First check the fib status -*/ + if (le32_to_cpu(srbreply->status) != ST_OK) { + int len; - if (le32_to_cpu(srbreply->status) != ST_OK){ - int len; - printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); - len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), - SCSI_SENSE_BUFFERSIZE); - scsicmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_CHECK_CONDITION; - memcpy(scsicmd->sense_buffer, srbreply->sense_data, len); - } + printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); + len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), + SCSI_SENSE_BUFFERSIZE); + scsicmd->result = DID_ERROR << 16 + | COMMAND_COMPLETE << 8 + | SAM_STAT_CHECK_CONDITION; + memcpy(scsicmd->sense_buffer, + srbreply->sense_data, len); + } - /* -* Next check the srb status -*/ - switch( (le32_to_cpu(srbreply->srb_status))&0x3f){ - case SRB_STATUS_ERROR_RECOVERY: - case SRB_STATUS_PENDING: - case SRB_STATUS_SUCCESS: - scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8; - break; - case SRB_STATUS_DATA_OVERRUN: - switch(scsicmd->cmnd[0]){ - case READ_6: - case WRITE_6: - case READ_10: - case WRITE_10: - case READ_12: - case WRITE_12: - case READ_16: - case WRITE_16: - if (le32_to_cpu(srbreply->data_xfer_length) < scsicmd->underflow) { - printk(KERN_WARNING"aacraid: SCSI CMD underflow\n"); - } else { - printk(KERN_WARNING"aacraid: SCSI CMD Data Overrun\n"); + /* +* Next check the srb status +*/ + switch ((le32_to_cpu(srbreply->srb_status))&0x3f) { + case SRB_STATUS_ERROR_RECOVERY: + case SRB_STATUS_PENDING: + case SRB_STATUS_SUCCESS: + scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8; + break; + case SRB_STATUS_DATA_OVERRUN: + switch (scsicmd->cmnd[0]) { + case READ_6: + case WRITE_6: + case READ_10: + case WRITE_10: + case READ_12: + case WRITE_12: + case READ_16: + case WRITE_16: + if (le32_to_cpu(srbreply->data_xfer_length) + < scsicmd->under