[SRU,C/B,1/1] scsi: mpt3sas: As per MPI-spec, use combined reply queue for SAS3.5 controllers when HBA supports more than 16 MSI-x vectors.
diff mbox series

Message ID 20190107152127.29939-2-mfo@canonical.com
State New
Headers show
Series
  • mpt3sas: fix periodic resets under high-load activity
Related show

Commit Message

Mauricio Faria de Oliveira Jan. 7, 2019, 3:21 p.m. UTC
From: Chaitra P B <chaitra.basappa@broadcom.com>

BugLink: https://bugs.launchpad.net/bugs/1810781

Presently driver is using combined reply queue feature when MSI-x vectors >
8 for both SAS3 and SAS3.5 controllers.  But as per MPI-spec,

1. For SAS3 controllers, driver should use combined reply queue when HBA
supports more than 8 MSI-x vectors.

2. For SAS3.5 controllers, driver should use combined reply queue when HBA
supports more than 16 MSI-x vectors.

Modified driver code to use combined reply queue for SAS3 controllers when
HBA supports > 8 MSI-x vectors and for SAS3.5 controllers when HBA supports
> 16 MSI-x vectors.

Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
(cherry picked from commit 2b48be65685a23f4ffc7a06858992bc31e98e198)
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 15 ++++++++-------
 drivers/scsi/mpt3sas/mpt3sas_base.h |  1 +
 2 files changed, 9 insertions(+), 7 deletions(-)

Comments

Stefan Bader Jan. 7, 2019, 4:35 p.m. UTC | #1
On 07.01.19 16:21, Mauricio Faria de Oliveira wrote:
> From: Chaitra P B <chaitra.basappa@broadcom.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1810781
> 
> Presently driver is using combined reply queue feature when MSI-x vectors >
> 8 for both SAS3 and SAS3.5 controllers.  But as per MPI-spec,
> 
> 1. For SAS3 controllers, driver should use combined reply queue when HBA
> supports more than 8 MSI-x vectors.
> 
> 2. For SAS3.5 controllers, driver should use combined reply queue when HBA
> supports more than 16 MSI-x vectors.
> 
> Modified driver code to use combined reply queue for SAS3 controllers when
> HBA supports > 8 MSI-x vectors and for SAS3.5 controllers when HBA supports
>> 16 MSI-x vectors.
> 
> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> (cherry picked from commit 2b48be65685a23f4ffc7a06858992bc31e98e198)
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

Driver specific clean cherry pick and so far seems to have no fixup and testable.
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 15 ++++++++-------
>  drivers/scsi/mpt3sas/mpt3sas_base.h |  1 +
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 3c8c17c0b547..7efc7f00c907 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2924,10 +2924,9 @@ mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
>  	_base_free_irq(ioc);
>  	_base_disable_msix(ioc);
>  
> -	if (ioc->combined_reply_queue) {
> -		kfree(ioc->replyPostRegisterIndex);
> -		ioc->replyPostRegisterIndex = NULL;
> -	}
> +	kfree(ioc->replyPostRegisterIndex);
> +	ioc->replyPostRegisterIndex = NULL;
> +
>  
>  	if (ioc->chip_phys) {
>  		iounmap(ioc->chip);
> @@ -3034,7 +3033,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>  	/* Use the Combined reply queue feature only for SAS3 C0 & higher
>  	 * revision HBAs and also only when reply queue count is greater than 8
>  	 */
> -	if (ioc->combined_reply_queue && ioc->reply_queue_count > 8) {
> +	if (ioc->combined_reply_queue) {
>  		/* Determine the Supplemental Reply Post Host Index Registers
>  		 * Addresse. Supplemental Reply Post Host Index Registers
>  		 * starts at offset MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET and
> @@ -3058,8 +3057,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>  			     MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
>  			     (i * MPT3_SUP_REPLY_POST_HOST_INDEX_REG_OFFSET));
>  		}
> -	} else
> -		ioc->combined_reply_queue = 0;
> +	}
>  
>  	if (ioc->is_warpdrive) {
>  		ioc->reply_post_host_index[0] = (resource_size_t __iomem *)
> @@ -5682,6 +5680,9 @@ _base_get_ioc_facts(struct MPT3SAS_ADAPTER *ioc)
>  	facts->WhoInit = mpi_reply.WhoInit;
>  	facts->NumberOfPorts = mpi_reply.NumberOfPorts;
>  	facts->MaxMSIxVectors = mpi_reply.MaxMSIxVectors;
> +	if (ioc->msix_enable && (facts->MaxMSIxVectors <=
> +	    MAX_COMBINED_MSIX_VECTORS(ioc->is_gen35_ioc)))
> +		ioc->combined_reply_queue = 0;
>  	facts->RequestCredit = le16_to_cpu(mpi_reply.RequestCredit);
>  	facts->MaxReplyDescriptorPostQueueDepth =
>  	    le16_to_cpu(mpi_reply.MaxReplyDescriptorPostQueueDepth);
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
> index f02974c0be4a..e3095fb0f77a 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -323,6 +323,7 @@
>   * There are twelve Supplemental Reply Post Host Index Registers
>   * and each register is at offset 0x10 bytes from the previous one.
>   */
> +#define MAX_COMBINED_MSIX_VECTORS(gen35) ((gen35 == 1) ? 16 : 8)
>  #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_COUNT_G3	12
>  #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_COUNT_G35	16
>  #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_OFFSET	(0x10)
>
Khaled Elmously Jan. 7, 2019, 5:06 p.m. UTC | #2
On 2019-01-07 13:21:27 , Mauricio Faria de Oliveira wrote:
> From: Chaitra P B <chaitra.basappa@broadcom.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1810781
> 
> Presently driver is using combined reply queue feature when MSI-x vectors >
> 8 for both SAS3 and SAS3.5 controllers.  But as per MPI-spec,
> 
> 1. For SAS3 controllers, driver should use combined reply queue when HBA
> supports more than 8 MSI-x vectors.
> 
> 2. For SAS3.5 controllers, driver should use combined reply queue when HBA
> supports more than 16 MSI-x vectors.
> 
> Modified driver code to use combined reply queue for SAS3 controllers when
> HBA supports > 8 MSI-x vectors and for SAS3.5 controllers when HBA supports
> > 16 MSI-x vectors.
> 
> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> (cherry picked from commit 2b48be65685a23f4ffc7a06858992bc31e98e198)
> Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 15 ++++++++-------
>  drivers/scsi/mpt3sas/mpt3sas_base.h |  1 +
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 3c8c17c0b547..7efc7f00c907 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2924,10 +2924,9 @@ mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
>  	_base_free_irq(ioc);
>  	_base_disable_msix(ioc);
>  
> -	if (ioc->combined_reply_queue) {
> -		kfree(ioc->replyPostRegisterIndex);
> -		ioc->replyPostRegisterIndex = NULL;
> -	}
> +	kfree(ioc->replyPostRegisterIndex);
> +	ioc->replyPostRegisterIndex = NULL;
> +
>  
>  	if (ioc->chip_phys) {
>  		iounmap(ioc->chip);
> @@ -3034,7 +3033,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>  	/* Use the Combined reply queue feature only for SAS3 C0 & higher
>  	 * revision HBAs and also only when reply queue count is greater than 8
>  	 */
> -	if (ioc->combined_reply_queue && ioc->reply_queue_count > 8) {
> +	if (ioc->combined_reply_queue) {
>  		/* Determine the Supplemental Reply Post Host Index Registers
>  		 * Addresse. Supplemental Reply Post Host Index Registers
>  		 * starts at offset MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET and
> @@ -3058,8 +3057,7 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
>  			     MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
>  			     (i * MPT3_SUP_REPLY_POST_HOST_INDEX_REG_OFFSET));
>  		}
> -	} else
> -		ioc->combined_reply_queue = 0;
> +	}
>  
>  	if (ioc->is_warpdrive) {
>  		ioc->reply_post_host_index[0] = (resource_size_t __iomem *)
> @@ -5682,6 +5680,9 @@ _base_get_ioc_facts(struct MPT3SAS_ADAPTER *ioc)
>  	facts->WhoInit = mpi_reply.WhoInit;
>  	facts->NumberOfPorts = mpi_reply.NumberOfPorts;
>  	facts->MaxMSIxVectors = mpi_reply.MaxMSIxVectors;
> +	if (ioc->msix_enable && (facts->MaxMSIxVectors <=
> +	    MAX_COMBINED_MSIX_VECTORS(ioc->is_gen35_ioc)))
> +		ioc->combined_reply_queue = 0;
>  	facts->RequestCredit = le16_to_cpu(mpi_reply.RequestCredit);
>  	facts->MaxReplyDescriptorPostQueueDepth =
>  	    le16_to_cpu(mpi_reply.MaxReplyDescriptorPostQueueDepth);
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
> index f02974c0be4a..e3095fb0f77a 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -323,6 +323,7 @@
>   * There are twelve Supplemental Reply Post Host Index Registers
>   * and each register is at offset 0x10 bytes from the previous one.
>   */
> +#define MAX_COMBINED_MSIX_VECTORS(gen35) ((gen35 == 1) ? 16 : 8)
>  #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_COUNT_G3	12
>  #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_COUNT_G35	16
>  #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_OFFSET	(0x10)

Acked-by: Khalid Elmously <khalid.elmously@canonical.com>

Patch
diff mbox series

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 3c8c17c0b547..7efc7f00c907 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2924,10 +2924,9 @@  mpt3sas_base_unmap_resources(struct MPT3SAS_ADAPTER *ioc)
 	_base_free_irq(ioc);
 	_base_disable_msix(ioc);
 
-	if (ioc->combined_reply_queue) {
-		kfree(ioc->replyPostRegisterIndex);
-		ioc->replyPostRegisterIndex = NULL;
-	}
+	kfree(ioc->replyPostRegisterIndex);
+	ioc->replyPostRegisterIndex = NULL;
+
 
 	if (ioc->chip_phys) {
 		iounmap(ioc->chip);
@@ -3034,7 +3033,7 @@  mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
 	/* Use the Combined reply queue feature only for SAS3 C0 & higher
 	 * revision HBAs and also only when reply queue count is greater than 8
 	 */
-	if (ioc->combined_reply_queue && ioc->reply_queue_count > 8) {
+	if (ioc->combined_reply_queue) {
 		/* Determine the Supplemental Reply Post Host Index Registers
 		 * Addresse. Supplemental Reply Post Host Index Registers
 		 * starts at offset MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET and
@@ -3058,8 +3057,7 @@  mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
 			     MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
 			     (i * MPT3_SUP_REPLY_POST_HOST_INDEX_REG_OFFSET));
 		}
-	} else
-		ioc->combined_reply_queue = 0;
+	}
 
 	if (ioc->is_warpdrive) {
 		ioc->reply_post_host_index[0] = (resource_size_t __iomem *)
@@ -5682,6 +5680,9 @@  _base_get_ioc_facts(struct MPT3SAS_ADAPTER *ioc)
 	facts->WhoInit = mpi_reply.WhoInit;
 	facts->NumberOfPorts = mpi_reply.NumberOfPorts;
 	facts->MaxMSIxVectors = mpi_reply.MaxMSIxVectors;
+	if (ioc->msix_enable && (facts->MaxMSIxVectors <=
+	    MAX_COMBINED_MSIX_VECTORS(ioc->is_gen35_ioc)))
+		ioc->combined_reply_queue = 0;
 	facts->RequestCredit = le16_to_cpu(mpi_reply.RequestCredit);
 	facts->MaxReplyDescriptorPostQueueDepth =
 	    le16_to_cpu(mpi_reply.MaxReplyDescriptorPostQueueDepth);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index f02974c0be4a..e3095fb0f77a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -323,6 +323,7 @@ 
  * There are twelve Supplemental Reply Post Host Index Registers
  * and each register is at offset 0x10 bytes from the previous one.
  */
+#define MAX_COMBINED_MSIX_VECTORS(gen35) ((gen35 == 1) ? 16 : 8)
 #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_COUNT_G3	12
 #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_COUNT_G35	16
 #define MPT3_SUP_REPLY_POST_HOST_INDEX_REG_OFFSET	(0x10)