diff mbox series

[1/4] scsi: megaraid_sas: use private counter for tracking inflight per-LUN commands

Message ID 20191118103117.978-2-ming.lei@redhat.com (mailing list archive)
State New, archived
Headers show
Series scis: don't apply per-LUN queue depth for SSD | expand

Commit Message

Ming Lei Nov. 18, 2019, 10:31 a.m. UTC
Prepare for bypassing sdev->device_busy for improving performance on
fast SCSI storage device, so sdev->device_busy can't be relied
any more.

megaraid_sas may need one such counter for balancing load among LUNs in
some specific setting, so add one private counter for this purpose.

Cc: Sathya Prakash <sathya.prakash@broadcom.com>
Cc: Chaitra P B <chaitra.basappa@broadcom.com>
Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Sumit Saxena <sumit.saxena@broadcom.com>
Cc: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
Cc: Ewan D. Milne <emilne@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>,
Cc: Hannes Reinecke <hare@suse.de>
Cc: Bart Van Assche <bart.vanassche@wdc.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/scsi/megaraid/megaraid_sas.h        |  1 +
 drivers/scsi/megaraid/megaraid_sas_base.c   | 15 +++++++++++++--
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 13 +++++++++----
 3 files changed, 23 insertions(+), 6 deletions(-)

Comments

Hannes Reinecke Nov. 20, 2019, 9:54 a.m. UTC | #1
On 11/18/19 11:31 AM, Ming Lei wrote:
> Prepare for bypassing sdev->device_busy for improving performance on
> fast SCSI storage device, so sdev->device_busy can't be relied
> any more.
> 
> megaraid_sas may need one such counter for balancing load among LUNs in
> some specific setting, so add one private counter for this purpose.
> 
> Cc: Sathya Prakash <sathya.prakash@broadcom.com>
> Cc: Chaitra P B <chaitra.basappa@broadcom.com>
> Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
> Cc: Kashyap Desai <kashyap.desai@broadcom.com>
> Cc: Sumit Saxena <sumit.saxena@broadcom.com>
> Cc: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
> Cc: Ewan D. Milne <emilne@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>,
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Bart Van Assche <bart.vanassche@wdc.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas.h        |  1 +
>  drivers/scsi/megaraid/megaraid_sas_base.c   | 15 +++++++++++++--
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 13 +++++++++----
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
Kashyap Desai Nov. 26, 2019, 3:12 a.m. UTC | #2
> >  drivers/scsi/megaraid/megaraid_sas.h        |  1 +
> >  drivers/scsi/megaraid/megaraid_sas_base.c   | 15 +++++++++++++--
> >  drivers/scsi/megaraid/megaraid_sas_fusion.c | 13 +++++++++----
> >  3 files changed, 23 insertions(+), 6 deletions(-)
> >
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Ming - Sorry for delay. I will update this Patch. We prefer driver to
avoid counter for per sdev if possible. We are currently testing driver
using below changes.

inline unsigned long sdev_nr_inflight_request(struct request_queue *q) {
    struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[0]

    return atomic_read(&hctx->nr_active);
}

>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke		      Teamlead Storage & Networking
> hare@suse.de			                  +49 911 74053 688
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB
> 247165 (AG München), GF: Felix Imendörffer
Ming Lei Nov. 26, 2019, 3:37 a.m. UTC | #3
On Tue, Nov 26, 2019 at 08:42:59AM +0530, Kashyap Desai wrote:
> > >  drivers/scsi/megaraid/megaraid_sas.h        |  1 +
> > >  drivers/scsi/megaraid/megaraid_sas_base.c   | 15 +++++++++++++--
> > >  drivers/scsi/megaraid/megaraid_sas_fusion.c | 13 +++++++++----
> > >  3 files changed, 23 insertions(+), 6 deletions(-)
> > >
> > Reviewed-by: Hannes Reinecke <hare@suse.de>
> 
> Ming - Sorry for delay. I will update this Patch. We prefer driver to
> avoid counter for per sdev if possible. We are currently testing driver
> using below changes.
> 
> inline unsigned long sdev_nr_inflight_request(struct request_queue *q) {
>     struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[0]
> 
>     return atomic_read(&hctx->nr_active);
> }

OK, I am fine with this way, given it is just used for balancing irq load.


Thanks,
Ming
Kashyap Desai Dec. 5, 2019, 10:32 a.m. UTC | #4
> > Ming - Sorry for delay. I will update this Patch. We prefer driver to
> > avoid counter for per sdev if possible. We are currently testing
> > driver using below changes.
> >
> > inline unsigned long sdev_nr_inflight_request(struct request_queue *q)
{
> >     struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[0]
> >
> >     return atomic_read(&hctx->nr_active); }
>
> OK, I am fine with this way, given it is just used for balancing irq
load.

We have removed sdev->device_busy check from megaraid_sas and mpt3sas
drive.  I am able to get required performance 3.0M IOPs on Aero controller
using <hctx->nr_active>
Since this is not urgent, we will wait for some testing done by Broadcom.
We will post this specific change as Driver update.

>
>
> Thanks,
> Ming
diff mbox series

Patch

diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
index a6e788c02ff4..f9562c02705b 100644
--- a/drivers/scsi/megaraid/megaraid_sas.h
+++ b/drivers/scsi/megaraid/megaraid_sas.h
@@ -2025,6 +2025,7 @@  struct MR_PRIV_DEVICE {
 	u8 interface_type;
 	u8 task_abort_tmo;
 	u8 target_reset_tmo;
+	atomic_t active_cmds;
 };
 struct megasas_cmd;
 
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 42cf38c1ea99..3afead3bc96e 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -1759,6 +1759,7 @@  megasas_queue_command(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
 {
 	struct megasas_instance *instance;
 	struct MR_PRIV_DEVICE *mr_device_priv_data;
+	int ret;
 
 	instance = (struct megasas_instance *)
 	    scmd->device->host->hostdata;
@@ -1821,7 +1822,11 @@  megasas_queue_command(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
 		goto out_done;
 	}
 
-	return instance->instancet->build_and_issue_cmd(instance, scmd);
+	atomic_inc(&mr_device_priv_data->active_cmds);
+	ret = instance->instancet->build_and_issue_cmd(instance, scmd);
+	if (ret)
+		atomic_dec(&mr_device_priv_data->active_cmds);
+	return ret;
 
  out_done:
 	scmd->scsi_done(scmd);
@@ -2100,6 +2105,7 @@  static int megasas_slave_alloc(struct scsi_device *sdev)
 
 	atomic_set(&mr_device_priv_data->r1_ldio_hint,
 		   instance->r1_ldio_hint_default);
+	atomic_set(&mr_device_priv_data->active_cmds, 0);
 	return 0;
 }
 
@@ -3475,12 +3481,15 @@  megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 	unsigned long flags;
 	struct fusion_context *fusion = instance->ctrl_context;
 	u32 opcode, status;
+	struct MR_PRIV_DEVICE *mr_device_priv_data = NULL;
 
 	/* flag for the retry reset */
 	cmd->retry_for_fw_reset = 0;
 
-	if (cmd->scmd)
+	if (cmd->scmd) {
 		cmd->scmd->SCp.ptr = NULL;
+		mr_device_priv_data = cmd->scmd->device->hostdata;
+	}
 
 	switch (hdr->cmd) {
 	case MFI_CMD_INVALID:
@@ -3519,6 +3528,7 @@  megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 		if (exception) {
 
 			atomic_dec(&instance->fw_outstanding);
+			atomic_dec(&mr_device_priv_data->active_cmds);
 
 			scsi_dma_unmap(cmd->scmd);
 			cmd->scmd->scsi_done(cmd->scmd);
@@ -3567,6 +3577,7 @@  megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 		}
 
 		atomic_dec(&instance->fw_outstanding);
+		atomic_dec(&mr_device_priv_data->active_cmds);
 
 		scsi_dma_unmap(cmd->scmd);
 		cmd->scmd->scsi_done(cmd->scmd);
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index e301458bcbae..10ed3bc3b643 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -2728,7 +2728,7 @@  megasas_build_ldio_fusion(struct megasas_instance *instance,
 	u8 *raidLUN;
 	unsigned long spinlock_flags;
 	struct MR_LD_RAID *raid = NULL;
-	struct MR_PRIV_DEVICE *mrdev_priv;
+	struct MR_PRIV_DEVICE *mrdev_priv = scp->device->hostdata;
 	struct RAID_CONTEXT *rctx;
 	struct RAID_CONTEXT_G35 *rctx_g35;
 
@@ -2826,7 +2826,7 @@  megasas_build_ldio_fusion(struct megasas_instance *instance,
 	}
 
 	if ((instance->perf_mode == MR_BALANCED_PERF_MODE) &&
-		atomic_read(&scp->device->device_busy) >
+		atomic_read(&mrdev_priv->active_cmds) >
 		(io_info.data_arms * MR_DEVICE_HIGH_IOPS_DEPTH))
 		cmd->request_desc->SCSIIO.MSIxIndex =
 			mega_mod64((atomic64_add_return(1, &instance->high_iops_outstanding) /
@@ -2849,7 +2849,6 @@  megasas_build_ldio_fusion(struct megasas_instance *instance,
 		 * with the SLD bit asserted.
 		 */
 		if (io_info.r1_alt_dev_handle != MR_DEVHANDLE_INVALID) {
-			mrdev_priv = scp->device->hostdata;
 
 			if (atomic_inc_return(&instance->fw_outstanding) >
 				(instance->host->can_queue)) {
@@ -3159,7 +3158,7 @@  megasas_build_syspd_fusion(struct megasas_instance *instance,
 	cmd->request_desc->SCSIIO.DevHandle = io_request->DevHandle;
 
 	if ((instance->perf_mode == MR_BALANCED_PERF_MODE) &&
-		atomic_read(&scmd->device->device_busy) > MR_DEVICE_HIGH_IOPS_DEPTH)
+		atomic_read(&mr_device_priv_data->active_cmds) > MR_DEVICE_HIGH_IOPS_DEPTH)
 		cmd->request_desc->SCSIIO.MSIxIndex =
 			mega_mod64((atomic64_add_return(1, &instance->high_iops_outstanding) /
 				MR_HIGH_IOPS_BATCH_COUNT), instance->low_latency_index_start);
@@ -3550,6 +3549,7 @@  complete_cmd_fusion(struct megasas_instance *instance, u32 MSIxIndex,
 
 	while (d_val.u.low != cpu_to_le32(UINT_MAX) &&
 	       d_val.u.high != cpu_to_le32(UINT_MAX)) {
+		struct MR_PRIV_DEVICE *mr_device_priv_data = NULL;
 
 		smid = le16_to_cpu(reply_desc->SMID);
 		cmd_fusion = fusion->cmd_list[smid - 1];
@@ -3585,6 +3585,8 @@  complete_cmd_fusion(struct megasas_instance *instance, u32 MSIxIndex,
 			}
 			/* Fall through - and complete IO */
 		case MEGASAS_MPI2_FUNCTION_LD_IO_REQUEST: /* LD-IO Path */
+			mr_device_priv_data = scmd_local->device->hostdata;
+			atomic_dec(&mr_device_priv_data->active_cmds);
 			atomic_dec(&instance->fw_outstanding);
 			if (cmd_fusion->r1_alt_dev_handle == MR_DEVHANDLE_INVALID) {
 				map_cmd_status(fusion, scmd_local, status,
@@ -4865,6 +4867,7 @@  int megasas_reset_fusion(struct Scsi_Host *shost, int reason)
 
 		/* Now return commands back to the OS */
 		for (i = 0 ; i < instance->max_scsi_cmds; i++) {
+			struct MR_PRIV_DEVICE *mr_device_priv_data = NULL;
 			cmd_fusion = fusion->cmd_list[i];
 			/*check for extra commands issued by driver*/
 			if (instance->adapter_type >= VENTURA_SERIES) {
@@ -4893,6 +4896,8 @@  int megasas_reset_fusion(struct Scsi_Host *shost, int reason)
 				megasas_return_cmd_fusion(instance, cmd_fusion);
 				scsi_dma_unmap(scmd_local);
 				scmd_local->scsi_done(scmd_local);
+				mr_device_priv_data = scmd_local->device->hostdata;
+				atomic_dec(&mr_device_priv_data->active_cmds);
 			}
 		}