diff mbox series

megaraid_sas: enable blk-mq for fusion

Message ID 20190111080524.96424-1-hare@suse.de (mailing list archive)
State Deferred
Headers show
Series megaraid_sas: enable blk-mq for fusion | expand

Commit Message

Hannes Reinecke Jan. 11, 2019, 8:05 a.m. UTC 
Fusion adapters can steer completions to individual queues, so
we can enable blk-mq for those adapters.
And in doing so we can rely on the interrupt affinity from the
block layer and drop the hand-crafted 'reply_map' construct.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/megaraid/megaraid_sas.h        |  1 -
 drivers/scsi/megaraid/megaraid_sas_base.c   | 34 ++---------------------------
 drivers/scsi/megaraid/megaraid_sas_fusion.c |  8 +++----
 3 files changed, 6 insertions(+), 37 deletions(-)

Comments

Kashyap Desai Jan. 11, 2019, 9:41 a.m. UTC | #1 
> Fusion adapters can steer completions to individual queues, so we can
enable
> blk-mq for those adapters.
> And in doing so we can rely on the interrupt affinity from the block
layer and
> drop the hand-crafted 'reply_map' construct.

Hannes, I understand the intend of this patch as we discussed couple of
time on same topic in past. We really don't need such interface in MR/IT
HBA because we have single submission queue (h/w registers.).
Mimicing multiple h/w submission queue really does not help any
performance. Relevant discussion at below -

https://marc.info/?l=linux-scsi&m=153078454611933&w=2

We can hit max h/w and storage performance limit using nr_hw_queues = 1.
We could not find any reason to really setup more than one hw queue.

Kashyap
Ming Lei Jan. 11, 2019, 9:45 a.m. UTC | #2 
On Fri, Jan 11, 2019 at 09:05:24AM +0100, Hannes Reinecke wrote:
> Fusion adapters can steer completions to individual queues, so
> we can enable blk-mq for those adapters.
> And in doing so we can rely on the interrupt affinity from the
> block layer and drop the hand-crafted 'reply_map' construct.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas.h        |  1 -
>  drivers/scsi/megaraid/megaraid_sas_base.c   | 34 ++---------------------------
>  drivers/scsi/megaraid/megaraid_sas_fusion.c |  8 +++----
>  3 files changed, 6 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
> index 16536c41f0c5..28ffcd5fcca2 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -2166,7 +2166,6 @@ enum MR_PD_TYPE {
>  
>  struct megasas_instance {
>  
> -	unsigned int *reply_map;
>  	__le32 *producer;
>  	dma_addr_t producer_h;
>  	__le32 *consumer;
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index f7bdd783360a..d2612e6358ec 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -5299,26 +5299,6 @@ megasas_setup_jbod_map(struct megasas_instance *instance)
>  		instance->use_seqnum_jbod_fp = false;
>  }
>  
> -static void megasas_setup_reply_map(struct megasas_instance *instance)
> -{
> -	const struct cpumask *mask;
> -	unsigned int queue, cpu;
> -
> -	for (queue = 0; queue < instance->msix_vectors; queue++) {
> -		mask = pci_irq_get_affinity(instance->pdev, queue);
> -		if (!mask)
> -			goto fallback;
> -
> -		for_each_cpu(cpu, mask)
> -			instance->reply_map[cpu] = queue;
> -	}
> -	return;
> -
> -fallback:
> -	for_each_possible_cpu(cpu)
> -		instance->reply_map[cpu] = cpu % instance->msix_vectors;
> -}
> -
>  /**
>   * megasas_init_fw -	Initializes the FW
>   * @instance:		Adapter soft state
> @@ -5524,8 +5504,6 @@ static int megasas_init_fw(struct megasas_instance *instance)
>  			goto fail_init_adapter;
>  	}
>  
> -	megasas_setup_reply_map(instance);
> -
>  	dev_info(&instance->pdev->dev,
>  		"firmware supports msix\t: (%d)", fw_msix_count);
>  	dev_info(&instance->pdev->dev,
> @@ -6164,6 +6142,8 @@ static int megasas_io_attach(struct megasas_instance *instance)
>  	host->max_id = MEGASAS_MAX_DEV_PER_CHANNEL;
>  	host->max_lun = MEGASAS_MAX_LUN;
>  	host->max_cmd_len = 16;
> +	if (instance->adapter_type != MFI_SERIES && instance->msix_vectors > 0)
> +		host->nr_hw_queues = instance->msix_vectors;
>  
>  	/*
>  	 * Notify the mid-layer about the new controller
> @@ -6331,11 +6311,6 @@ static inline int megasas_alloc_mfi_ctrl_mem(struct megasas_instance *instance)
>   */
>  static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
>  {
> -	instance->reply_map = kcalloc(nr_cpu_ids, sizeof(unsigned int),
> -				      GFP_KERNEL);
> -	if (!instance->reply_map)
> -		return -ENOMEM;
> -
>  	switch (instance->adapter_type) {
>  	case MFI_SERIES:
>  		if (megasas_alloc_mfi_ctrl_mem(instance))
> @@ -6352,8 +6327,6 @@ static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
>  
>  	return 0;
>   fail:
> -	kfree(instance->reply_map);
> -	instance->reply_map = NULL;
>  	return -ENOMEM;
>  }
>  
> @@ -6366,7 +6339,6 @@ static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
>   */
>  static inline void megasas_free_ctrl_mem(struct megasas_instance *instance)
>  {
> -	kfree(instance->reply_map);
>  	if (instance->adapter_type == MFI_SERIES) {
>  		if (instance->producer)
>  			dma_free_coherent(&instance->pdev->dev, sizeof(u32),
> @@ -6988,8 +6960,6 @@ megasas_resume(struct pci_dev *pdev)
>  	if (rval < 0)
>  		goto fail_reenable_msix;
>  
> -	megasas_setup_reply_map(instance);
> -
>  	if (instance->adapter_type != MFI_SERIES) {
>  		megasas_reset_reply_desc(instance);
>  		if (megasas_ioc_init_fusion(instance)) {
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index 211c17c33aa0..dd89fa4dd328 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -2629,6 +2629,7 @@ megasas_build_ldio_fusion(struct megasas_instance *instance,
>  	struct MR_PRIV_DEVICE *mrdev_priv;
>  	struct RAID_CONTEXT *rctx;
>  	struct RAID_CONTEXT_G35 *rctx_g35;
> +	u32 tag = blk_mq_unique_tag(scp->request);
>  
>  	device_id = MEGASAS_DEV_INDEX(scp);
>  
> @@ -2724,8 +2725,7 @@ megasas_build_ldio_fusion(struct megasas_instance *instance,
>  			fp_possible = (io_info.fpOkForIo > 0) ? true : false;
>  	}
>  
> -	cmd->request_desc->SCSIIO.MSIxIndex =
> -		instance->reply_map[raw_smp_processor_id()];
> +	cmd->request_desc->SCSIIO.MSIxIndex = blk_mq_unique_tag_to_hwq(tag);
>  
>  	if (instance->adapter_type >= VENTURA_SERIES) {
>  		/* FP for Optimal raid level 1.
> @@ -2971,6 +2971,7 @@ megasas_build_syspd_fusion(struct megasas_instance *instance,
>  	u16 pd_index = 0;
>  	u16 os_timeout_value;
>  	u16 timeout_limit;
> +	u32 tag = blk_mq_unique_tag(scmd->request);
>  	struct MR_DRV_RAID_MAP_ALL *local_map_ptr;
>  	struct RAID_CONTEXT	*pRAID_Context;
>  	struct MR_PD_CFG_SEQ_NUM_SYNC *pd_sync;
> @@ -3038,8 +3039,7 @@ megasas_build_syspd_fusion(struct megasas_instance *instance,
>  
>  	cmd->request_desc->SCSIIO.DevHandle = io_request->DevHandle;
>  
> -	cmd->request_desc->SCSIIO.MSIxIndex =
> -		instance->reply_map[raw_smp_processor_id()];
> +	cmd->request_desc->SCSIIO.MSIxIndex = blk_mq_unique_tag_to_hwq(tag);
>  
>  	if (!fp_possible) {
>  		/* system pd firmware path */
> -- 
> 2.16.4
> 

The hand-crafted 'reply_map' is just for fixing legacy io path, and now
we don't need it any more, so:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

BTW, HPSA needs the cleanup too, could you do it too?

Thanks,
Ming
Lee Duncan Jan. 11, 2019, 6:59 p.m. UTC | #3 
On 1/11/19 12:05 AM, Hannes Reinecke wrote:
> Fusion adapters can steer completions to individual queues, so
> we can enable blk-mq for those adapters.
> And in doing so we can rely on the interrupt affinity from the
> block layer and drop the hand-crafted 'reply_map' construct.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas.h        |  1 -
>  drivers/scsi/megaraid/megaraid_sas_base.c   | 34 ++---------------------------
>  drivers/scsi/megaraid/megaraid_sas_fusion.c |  8 +++----
>  3 files changed, 6 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
> index 16536c41f0c5..28ffcd5fcca2 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -2166,7 +2166,6 @@ enum MR_PD_TYPE {
>  
>  struct megasas_instance {
>  
> -	unsigned int *reply_map;
>  	__le32 *producer;
>  	dma_addr_t producer_h;
>  	__le32 *consumer;
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index f7bdd783360a..d2612e6358ec 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -5299,26 +5299,6 @@ megasas_setup_jbod_map(struct megasas_instance *instance)
>  		instance->use_seqnum_jbod_fp = false;
>  }
>  
> -static void megasas_setup_reply_map(struct megasas_instance *instance)
> -{
> -	const struct cpumask *mask;
> -	unsigned int queue, cpu;
> -
> -	for (queue = 0; queue < instance->msix_vectors; queue++) {
> -		mask = pci_irq_get_affinity(instance->pdev, queue);
> -		if (!mask)
> -			goto fallback;
> -
> -		for_each_cpu(cpu, mask)
> -			instance->reply_map[cpu] = queue;
> -	}
> -	return;
> -
> -fallback:
> -	for_each_possible_cpu(cpu)
> -		instance->reply_map[cpu] = cpu % instance->msix_vectors;
> -}
> -
>  /**
>   * megasas_init_fw -	Initializes the FW
>   * @instance:		Adapter soft state
> @@ -5524,8 +5504,6 @@ static int megasas_init_fw(struct megasas_instance *instance)
>  			goto fail_init_adapter;
>  	}
>  
> -	megasas_setup_reply_map(instance);
> -
>  	dev_info(&instance->pdev->dev,
>  		"firmware supports msix\t: (%d)", fw_msix_count);
>  	dev_info(&instance->pdev->dev,
> @@ -6164,6 +6142,8 @@ static int megasas_io_attach(struct megasas_instance *instance)
>  	host->max_id = MEGASAS_MAX_DEV_PER_CHANNEL;
>  	host->max_lun = MEGASAS_MAX_LUN;
>  	host->max_cmd_len = 16;
> +	if (instance->adapter_type != MFI_SERIES && instance->msix_vectors > 0)
> +		host->nr_hw_queues = instance->msix_vectors;
>  
>  	/*
>  	 * Notify the mid-layer about the new controller
> @@ -6331,11 +6311,6 @@ static inline int megasas_alloc_mfi_ctrl_mem(struct megasas_instance *instance)
>   */
>  static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
>  {
> -	instance->reply_map = kcalloc(nr_cpu_ids, sizeof(unsigned int),
> -				      GFP_KERNEL);
> -	if (!instance->reply_map)
> -		return -ENOMEM;
> -
>  	switch (instance->adapter_type) {
>  	case MFI_SERIES:
>  		if (megasas_alloc_mfi_ctrl_mem(instance))
> @@ -6352,8 +6327,6 @@ static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
>  
>  	return 0;
>   fail:
> -	kfree(instance->reply_map);
> -	instance->reply_map = NULL;
>  	return -ENOMEM;
>  }
>  
> @@ -6366,7 +6339,6 @@ static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
>   */
>  static inline void megasas_free_ctrl_mem(struct megasas_instance *instance)
>  {
> -	kfree(instance->reply_map);
>  	if (instance->adapter_type == MFI_SERIES) {
>  		if (instance->producer)
>  			dma_free_coherent(&instance->pdev->dev, sizeof(u32),
> @@ -6988,8 +6960,6 @@ megasas_resume(struct pci_dev *pdev)
>  	if (rval < 0)
>  		goto fail_reenable_msix;
>  
> -	megasas_setup_reply_map(instance);
> -
>  	if (instance->adapter_type != MFI_SERIES) {
>  		megasas_reset_reply_desc(instance);
>  		if (megasas_ioc_init_fusion(instance)) {
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index 211c17c33aa0..dd89fa4dd328 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -2629,6 +2629,7 @@ megasas_build_ldio_fusion(struct megasas_instance *instance,
>  	struct MR_PRIV_DEVICE *mrdev_priv;
>  	struct RAID_CONTEXT *rctx;
>  	struct RAID_CONTEXT_G35 *rctx_g35;
> +	u32 tag = blk_mq_unique_tag(scp->request);
>  
>  	device_id = MEGASAS_DEV_INDEX(scp);
>  
> @@ -2724,8 +2725,7 @@ megasas_build_ldio_fusion(struct megasas_instance *instance,
>  			fp_possible = (io_info.fpOkForIo > 0) ? true : false;
>  	}
>  
> -	cmd->request_desc->SCSIIO.MSIxIndex =
> -		instance->reply_map[raw_smp_processor_id()];
> +	cmd->request_desc->SCSIIO.MSIxIndex = blk_mq_unique_tag_to_hwq(tag);
>  
>  	if (instance->adapter_type >= VENTURA_SERIES) {
>  		/* FP for Optimal raid level 1.
> @@ -2971,6 +2971,7 @@ megasas_build_syspd_fusion(struct megasas_instance *instance,
>  	u16 pd_index = 0;
>  	u16 os_timeout_value;
>  	u16 timeout_limit;
> +	u32 tag = blk_mq_unique_tag(scmd->request);
>  	struct MR_DRV_RAID_MAP_ALL *local_map_ptr;
>  	struct RAID_CONTEXT	*pRAID_Context;
>  	struct MR_PD_CFG_SEQ_NUM_SYNC *pd_sync;
> @@ -3038,8 +3039,7 @@ megasas_build_syspd_fusion(struct megasas_instance *instance,
>  
>  	cmd->request_desc->SCSIIO.DevHandle = io_request->DevHandle;
>  
> -	cmd->request_desc->SCSIIO.MSIxIndex =
> -		instance->reply_map[raw_smp_processor_id()];
> +	cmd->request_desc->SCSIIO.MSIxIndex = blk_mq_unique_tag_to_hwq(tag);
>  
>  	if (!fp_possible) {
>  		/* system pd firmware path */
> 

Does what it says it'll do ...

Reviewed-by: Lee Duncan <lduncan@suse.com>
diff mbox series

Patch

diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
index 16536c41f0c5..28ffcd5fcca2 100644
--- a/drivers/scsi/megaraid/megaraid_sas.h
+++ b/drivers/scsi/megaraid/megaraid_sas.h
@@ -2166,7 +2166,6 @@  enum MR_PD_TYPE {
 
 struct megasas_instance {
 
-	unsigned int *reply_map;
 	__le32 *producer;
 	dma_addr_t producer_h;
 	__le32 *consumer;
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index f7bdd783360a..d2612e6358ec 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5299,26 +5299,6 @@  megasas_setup_jbod_map(struct megasas_instance *instance)
 		instance->use_seqnum_jbod_fp = false;
 }
 
-static void megasas_setup_reply_map(struct megasas_instance *instance)
-{
-	const struct cpumask *mask;
-	unsigned int queue, cpu;
-
-	for (queue = 0; queue < instance->msix_vectors; queue++) {
-		mask = pci_irq_get_affinity(instance->pdev, queue);
-		if (!mask)
-			goto fallback;
-
-		for_each_cpu(cpu, mask)
-			instance->reply_map[cpu] = queue;
-	}
-	return;
-
-fallback:
-	for_each_possible_cpu(cpu)
-		instance->reply_map[cpu] = cpu % instance->msix_vectors;
-}
-
 /**
  * megasas_init_fw -	Initializes the FW
  * @instance:		Adapter soft state
@@ -5524,8 +5504,6 @@  static int megasas_init_fw(struct megasas_instance *instance)
 			goto fail_init_adapter;
 	}
 
-	megasas_setup_reply_map(instance);
-
 	dev_info(&instance->pdev->dev,
 		"firmware supports msix\t: (%d)", fw_msix_count);
 	dev_info(&instance->pdev->dev,
@@ -6164,6 +6142,8 @@  static int megasas_io_attach(struct megasas_instance *instance)
 	host->max_id = MEGASAS_MAX_DEV_PER_CHANNEL;
 	host->max_lun = MEGASAS_MAX_LUN;
 	host->max_cmd_len = 16;
+	if (instance->adapter_type != MFI_SERIES && instance->msix_vectors > 0)
+		host->nr_hw_queues = instance->msix_vectors;
 
 	/*
 	 * Notify the mid-layer about the new controller
@@ -6331,11 +6311,6 @@  static inline int megasas_alloc_mfi_ctrl_mem(struct megasas_instance *instance)
  */
 static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
 {
-	instance->reply_map = kcalloc(nr_cpu_ids, sizeof(unsigned int),
-				      GFP_KERNEL);
-	if (!instance->reply_map)
-		return -ENOMEM;
-
 	switch (instance->adapter_type) {
 	case MFI_SERIES:
 		if (megasas_alloc_mfi_ctrl_mem(instance))
@@ -6352,8 +6327,6 @@  static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
 
 	return 0;
  fail:
-	kfree(instance->reply_map);
-	instance->reply_map = NULL;
 	return -ENOMEM;
 }
 
@@ -6366,7 +6339,6 @@  static int megasas_alloc_ctrl_mem(struct megasas_instance *instance)
  */
 static inline void megasas_free_ctrl_mem(struct megasas_instance *instance)
 {
-	kfree(instance->reply_map);
 	if (instance->adapter_type == MFI_SERIES) {
 		if (instance->producer)
 			dma_free_coherent(&instance->pdev->dev, sizeof(u32),
@@ -6988,8 +6960,6 @@  megasas_resume(struct pci_dev *pdev)
 	if (rval < 0)
 		goto fail_reenable_msix;
 
-	megasas_setup_reply_map(instance);
-
 	if (instance->adapter_type != MFI_SERIES) {
 		megasas_reset_reply_desc(instance);
 		if (megasas_ioc_init_fusion(instance)) {
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 211c17c33aa0..dd89fa4dd328 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -2629,6 +2629,7 @@  megasas_build_ldio_fusion(struct megasas_instance *instance,
 	struct MR_PRIV_DEVICE *mrdev_priv;
 	struct RAID_CONTEXT *rctx;
 	struct RAID_CONTEXT_G35 *rctx_g35;
+	u32 tag = blk_mq_unique_tag(scp->request);
 
 	device_id = MEGASAS_DEV_INDEX(scp);
 
@@ -2724,8 +2725,7 @@  megasas_build_ldio_fusion(struct megasas_instance *instance,
 			fp_possible = (io_info.fpOkForIo > 0) ? true : false;
 	}
 
-	cmd->request_desc->SCSIIO.MSIxIndex =
-		instance->reply_map[raw_smp_processor_id()];
+	cmd->request_desc->SCSIIO.MSIxIndex = blk_mq_unique_tag_to_hwq(tag);
 
 	if (instance->adapter_type >= VENTURA_SERIES) {
 		/* FP for Optimal raid level 1.
@@ -2971,6 +2971,7 @@  megasas_build_syspd_fusion(struct megasas_instance *instance,
 	u16 pd_index = 0;
 	u16 os_timeout_value;
 	u16 timeout_limit;
+	u32 tag = blk_mq_unique_tag(scmd->request);
 	struct MR_DRV_RAID_MAP_ALL *local_map_ptr;
 	struct RAID_CONTEXT	*pRAID_Context;
 	struct MR_PD_CFG_SEQ_NUM_SYNC *pd_sync;
@@ -3038,8 +3039,7 @@  megasas_build_syspd_fusion(struct megasas_instance *instance,
 
 	cmd->request_desc->SCSIIO.DevHandle = io_request->DevHandle;
 
-	cmd->request_desc->SCSIIO.MSIxIndex =
-		instance->reply_map[raw_smp_processor_id()];
+	cmd->request_desc->SCSIIO.MSIxIndex = blk_mq_unique_tag_to_hwq(tag);
 
 	if (!fp_possible) {
 		/* system pd firmware path */