Re: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set

2015-06-25 Thread Tomas Henzl
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

2015-06-23 Thread Mahesh Rajashekhara
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

2015-06-18 Thread Rajinikanth Pandurangan
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

2015-06-10 Thread rajinikanth.pandurangan
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