Re: [PATCH] scsi: aacraid: Fix PD performance regression over incorrect qd being set

[Martin K. Petersen]

Raghava,

> The driver fails to set the correct queue depth for native devices,
> due to failing to set the device type prior to calling
> aac_set_safw_target_qd(). This results in slave configure setting the
> queue depth to 1.
>
> This causes around 30% performance degradation. Fixed by setting the
> dev type before trying to set queue depth.

Applied to 4.18/scsi-fixes. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: aacraid: Fix PD performance regression over incorrect qd being set

[Ewan D. Milne]

On Fri, 2018-06-22 at 06:55 -0700, Raghava Aditya Renukunta wrote:
> The driver fails to set the correct queue depth for native devices, due
> to failing to set the device type prior to calling
> aac_set_safw_target_qd(). This results in slave configure setting the
> queue depth to 1.
> 
> This causes around 30% performance degradation. Fixed by setting the
> dev type before trying to set queue depth.
> 
> Reported-by: Steve Best 
> Fixes: 0bcb45fb20c2a ("scsi: aacraid: Add helper function to set queue depth")

s/0bcb45fb20c2a/0bcb45fb20c21/

> cc: sta...@vger.kernel.org
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Reviewed-by: David Carroll 
> ---
>  drivers/scsi/aacraid/aachba.c |   15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index a9831bd37a73..a57f3a7d4748 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -1974,7 +1974,6 @@ static void aac_set_safw_attr_all_targets(struct 
> aac_dev *dev)
>   u32 lun_count, nexus;
>   u32 i, bus, target;
>   u8 expose_flag, attribs;
> - u8 devtype;
>  
>   lun_count = aac_get_safw_phys_lun_count(dev);
>  
> @@ -1992,23 +1991,23 @@ static void aac_set_safw_attr_all_targets(struct 
> aac_dev *dev)
>   continue;
>  
>   if (expose_flag != 0) {
> - devtype = AAC_DEVTYPE_RAID_MEMBER;
> - goto update_devtype;
> + dev->hba_map[bus][target].devtype =
> + AAC_DEVTYPE_RAID_MEMBER;
> + continue;
>   }
>  
>   if (nexus != 0 && (attribs & 8)) {
> - devtype = AAC_DEVTYPE_NATIVE_RAW;
> + dev->hba_map[bus][target].devtype =
> + AAC_DEVTYPE_NATIVE_RAW;
>   dev->hba_map[bus][target].rmw_nexus =
>   nexus;
>   } else
> - devtype = AAC_DEVTYPE_ARC_RAW;
> + dev->hba_map[bus][target].devtype =
> + AAC_DEVTYPE_ARC_RAW;
>  
>   dev->hba_map[bus][target].scan_counter = dev->scan_counter;
>  
>   aac_set_safw_target_qd(dev, bus, target);
> -
> -update_devtype:
> - dev->hba_map[bus][target].devtype = devtype;
>   }
>  }
>  
> 

The "Fixes:" tag above does not look correct to me, I've put in
what I see in Martin's tree.

Fixes a very noticeable performance regression.

Reviewed-by: Ewan D. Milne 

[PATCH] scsi: aacraid: Fix PD performance regression over incorrect qd being set

[Raghava Aditya Renukunta]

The driver fails to set the correct queue depth for native devices, due
to failing to set the device type prior to calling
aac_set_safw_target_qd(). This results in slave configure setting the
queue depth to 1.

This causes around 30% performance degradation. Fixed by setting the
dev type before trying to set queue depth.

Reported-by: Steve Best 
Fixes: 0bcb45fb20c2a ("scsi: aacraid: Add helper function to set queue depth")
cc: sta...@vger.kernel.org
Signed-off-by: Raghava Aditya Renukunta 
Reviewed-by: David Carroll 
---
 drivers/scsi/aacraid/aachba.c |   15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index a9831bd37a73..a57f3a7d4748 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1974,7 +1974,6 @@ static void aac_set_safw_attr_all_targets(struct aac_dev 
*dev)
u32 lun_count, nexus;
u32 i, bus, target;
u8 expose_flag, attribs;
-   u8 devtype;
 
lun_count = aac_get_safw_phys_lun_count(dev);
 
@@ -1992,23 +1991,23 @@ static void aac_set_safw_attr_all_targets(struct 
aac_dev *dev)
continue;
 
if (expose_flag != 0) {
-   devtype = AAC_DEVTYPE_RAID_MEMBER;
-   goto update_devtype;
+   dev->hba_map[bus][target].devtype =
+   AAC_DEVTYPE_RAID_MEMBER;
+   continue;
}
 
if (nexus != 0 && (attribs & 8)) {
-   devtype = AAC_DEVTYPE_NATIVE_RAW;
+   dev->hba_map[bus][target].devtype =
+   AAC_DEVTYPE_NATIVE_RAW;
dev->hba_map[bus][target].rmw_nexus =
nexus;
} else
-   devtype = AAC_DEVTYPE_ARC_RAW;
+   dev->hba_map[bus][target].devtype =
+   AAC_DEVTYPE_ARC_RAW;
 
dev->hba_map[bus][target].scan_counter = dev->scan_counter;
 
aac_set_safw_target_qd(dev, bus, target);
-
-update_devtype:
-   dev->hba_map[bus][target].devtype = devtype;
}
 }