Skip to contentSkip to conversation

Performance improvements for LSI SCSI cards

PATCH In review Matthew Wilcox 7 participants 18 messages Mar 27, 2014 – Apr 7, 2014

Subsystems (2)
SCSI LSI Logic MPT fusion drivers (FC/SAS/SPI)
lore
Share & tools
lore

Matthew Wilcox

author
#0 lore raw

cover Performance improvements for LSI SCSI cards

The host lock is a serious scalability problem on 2-socket and larger systems which are doing a lot of I/O. Before removing the temporary usgae of DEF_SCSI_QCMD, we need to remove all uses of serial_number.

An unrelated performance issue is that reusing the most recent driver-specific data structure to track the I/O instead of the least recently used keeps the cache-hot lines in use, which is a nice performance improvement. It's already present in the mpt3sas driver, it just didn't make it into the fusion or mpt2sas drivers yet.

Matthew Wilcox (7):
mpt3sas: Remove uses of serial_number
mpt3sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Remove uses of serial_number
mpt2sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Add free smids to the head, not tail of list
fusion: Add free msg frames to the head, not tail of list
fusion: Remove use of DEF_SCSI_QCMD

 drivers/message/fusion/mptbase.c     |  2 +-
 drivers/message/fusion/mptfc.c       | 12 +++++-------
 drivers/message/fusion/mptsas.c      | 10 ++++------
 drivers/message/fusion/mptscsih.c    |  8 +++-----
 drivers/message/fusion/mptscsih.h    |  2 +-
 drivers/message/fusion/mptspi.c      | 12 +++++-------
 drivers/scsi/mpt2sas/mpt2sas_base.c  |  8 ++++----
 drivers/scsi/mpt2sas/mpt2sas_base.h  |  2 +-
 drivers/scsi/mpt2sas/mpt2sas_ctl.c   |  2 +-
 drivers/scsi/mpt2sas/mpt2sas_scsih.c | 24 +++++++++---------------
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 24 +++++++++---------------
 13 files changed, 45 insertions(+), 65 deletions(-)
Signature · 1 line
1.9.0
author
#1 lore raw

PATCH 1/7 mpt3sas: Remove uses of serial_number

The mpt3sas_scsih_issue_tm() function does not use the 'serial_number' argument passed to it. Removing it removes the last vestiges of the scsi_cmnd's serial_number field from this driver.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ++++++---------
 3 files changed, 8 insertions(+), 11 deletions(-)
drivers/scsi/mpt3sas/mpt3sas_base.h +1 -1
index 0ebf5d9..9b90a6f 100644
@@ -993,7 +993,7 @@ void mpt3sas_scsih_reset_handler(struct MPT3SAS_ADAPTER *ioc, int reset_phase);
 
 int mpt3sas_scsih_issue_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
 	uint channel, uint id, uint lun, u8 type, u16 smid_task,
-	ulong timeout, unsigned long serial_number,  enum mutex_type m_type);
+	ulong timeout, enum mutex_type m_type);
 void mpt3sas_scsih_set_tm_flag(struct MPT3SAS_ADAPTER *ioc, u16 handle);
 void mpt3sas_scsih_clear_tm_flag(struct MPT3SAS_ADAPTER *ioc, u16 handle);
 void mpt3sas_expander_remove(struct MPT3SAS_ADAPTER *ioc, u64 sas_address);
drivers/scsi/mpt3sas/mpt3sas_ctl.c +1 -1
index 9b89de1..ba9cbe5 100644
@@ -980,7 +980,7 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 			mpt3sas_scsih_issue_tm(ioc,
 			    le16_to_cpu(mpi_request->FunctionDependent1), 0, 0,
 			    0, MPI2_SCSITASKMGMT_TASKTYPE_TARGET_RESET, 0, 30,
-			    0, TM_MUTEX_ON);
+			    TM_MUTEX_ON);
 		} else
 			mpt3sas_base_hard_reset_handler(ioc, CAN_SLEEP,
 			    FORCE_BIG_HAMMER);
drivers/scsi/mpt3sas/mpt3sas_scsih.c +6 -9
index a961fe1..952f6e0 100644
@@ -2029,7 +2029,6 @@ mpt3sas_scsih_clear_tm_flag(struct MPT3SAS_ADAPTER *ioc, u16 handle)
  * @type: MPI2_SCSITASKMGMT_TASKTYPE__XXX (defined in mpi2_init.h)
  * @smid_task: smid assigned to the task
  * @timeout: timeout in seconds
- * @serial_number: the serial_number from scmd
  * @m_type: TM_MUTEX_ON or TM_MUTEX_OFF
  * Context: user
  *

@@ -2042,7 +2041,7 @@ mpt3sas_scsih_clear_tm_flag(struct MPT3SAS_ADAPTER *ioc, u16 handle)
 int
 mpt3sas_scsih_issue_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle, uint channel,
 	uint id, uint lun, u8 type, u16 smid_task, ulong timeout,
-	unsigned long serial_number, enum mutex_type m_type)
+	enum mutex_type m_type)
 {
 	Mpi2SCSITaskManagementRequest_t *mpi_request;
 	Mpi2SCSITaskManagementReply_t *mpi_reply;

@@ -2293,8 +2292,7 @@ _scsih_abort(struct scsi_cmnd *scmd)
 	handle = sas_device_priv_data->sas_target->handle;
 	r = mpt3sas_scsih_issue_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, scmd->device->lun,
-	    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30,
-	    scmd->serial_number, TM_MUTEX_ON);
+	    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30, TM_MUTEX_ON);
 
  out:
 	sdev_printk(KERN_INFO, scmd->device, "task abort: %s scmd(%p)\n",

@@ -2353,8 +2351,7 @@ _scsih_dev_reset(struct scsi_cmnd *scmd)
 
 	r = mpt3sas_scsih_issue_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, scmd->device->lun,
-	    MPI2_SCSITASKMGMT_TASKTYPE_LOGICAL_UNIT_RESET, 0, 30, 0,
-	    TM_MUTEX_ON);
+	    MPI2_SCSITASKMGMT_TASKTYPE_LOGICAL_UNIT_RESET, 0, 30, TM_MUTEX_ON);
 
  out:
 	sdev_printk(KERN_INFO, scmd->device, "device reset: %s scmd(%p)\n",

@@ -2414,7 +2411,7 @@ _scsih_target_reset(struct scsi_cmnd *scmd)
 
 	r = mpt3sas_scsih_issue_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, 0, MPI2_SCSITASKMGMT_TASKTYPE_TARGET_RESET, 0,
-	    30, 0, TM_MUTEX_ON);
+	    30, TM_MUTEX_ON);
 
  out:
 	starget_printk(KERN_INFO, starget, "target reset: %s scmd(%p)\n",

@@ -5425,7 +5422,7 @@ _scsih_sas_broadcast_primitive_event(struct MPT3SAS_ADAPTER *ioc,
 
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 		r = mpt3sas_scsih_issue_tm(ioc, handle, 0, 0, lun,
-		    MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK, smid, 30, 0,
+		    MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK, smid, 30,
 		    TM_MUTEX_OFF);
 		if (r == FAILED) {
 			sdev_printk(KERN_WARNING, sdev,

@@ -5467,7 +5464,7 @@ _scsih_sas_broadcast_primitive_event(struct MPT3SAS_ADAPTER *ioc,
 
 		r = mpt3sas_scsih_issue_tm(ioc, handle, sdev->channel, sdev->id,
 		    sdev->lun, MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30,
-		    scmd->serial_number, TM_MUTEX_OFF);
+		    TM_MUTEX_OFF);
 		if (r == FAILED) {
 			sdev_printk(KERN_WARNING, sdev,
 			    "mpt3sas_scsih_issue_tm: ABORT_TASK: FAILED : "
Signature · 1 line
1.9.0
author
#3 lore raw

PATCH 2/7 mpt3sas: Remove use of DEF_SCSI_QCMD

In reply to Matthew Wilcox

Removing the host_lock from the I/O submission path gives a huge scalability improvement.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/scsi/mpt3sas/mpt3sas_scsih.c +3 -6
index 952f6e0..18e713d 100644
@@ -3515,7 +3515,7 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 ioc_status)
 
 
 /**
- * _scsih_qcmd_lck - main scsi request entry point
+ * _scsih_qcmd - main scsi request entry point
  * @scmd: pointer to scsi command object
  * @done: function pointer to be invoked on completion
  *

@@ -3526,9 +3526,9 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 ioc_status)
  * SCSI_MLQUEUE_HOST_BUSY if the entire host queue is full
  */
 static int
-_scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
+_scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
 {
-	struct MPT3SAS_ADAPTER *ioc = shost_priv(scmd->device->host);
+	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
 	struct MPT3SAS_DEVICE *sas_device_priv_data;
 	struct MPT3SAS_TARGET *sas_target_priv_data;
 	Mpi2SCSIIORequest_t *mpi_request;

@@ -3541,7 +3541,6 @@ _scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
 		scsi_print_command(scmd);
 #endif
 
-	scmd->scsi_done = done;
 	sas_device_priv_data = scmd->device->hostdata;
 	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
 		scmd->result = DID_NO_CONNECT << 16;

@@ -3656,8 +3655,6 @@ _scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
  out:
 	return SCSI_MLQUEUE_HOST_BUSY;
 }
-static DEF_SCSI_QCMD(_scsih_qcmd)
-
 
 /**
  * _scsih_normalize_sense - normalize descriptor and fixed format sense data
Signature · 1 line
1.9.0
#4 lore raw

Re: [PATCH 2/7] mpt3sas: Remove use of DEF_SCSI_QCMD

Quoted 2 lines · Matthew Wilcox

Removing the host_lock from the I/O submission path gives a huge scalability improvement.

This looks reasonable to me, but did you do a full audit that all state is properly synchronized in the functions called from _scsih_qcmd_lck?

If so that should be stated in the patch description.

#5 lore raw

Re: [PATCH 2/7] mpt3sas: Remove use of DEF_SCSI_QCMD

Quoted 8 lines · Christoph Hellwig
Quoted 2 lines · Matthew Wilcox

Removing the host_lock from the I/O submission path gives a huge scalability improvement.

This looks reasonable to me, but did you do a full audit that all state is properly synchronized in the functions called from _scsih_qcmd_lck?

If so that should be stated in the patch description.

I did one for mpt2sas back in July 2011 when I first submitted this patch.
http://marc.info/?l=linux-scsi&m=130980763617589&w=2

I must confess to having not done so for this iteration. The equivalent to this patch has been in RHEL since 6.2, so I hope that gives us some amount of confidence.

author
#6 lore raw

PATCH 3/7 mpt2sas: Remove uses of serial_number

In reply to Matthew Wilcox

The mpt2sas_scsih_issue_tm() function does not use the 'serial_number' argument passed to it. Removing it removes the last vestiges of the scsi_cmnd's serial_number field from this driver.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/scsi/mpt2sas/mpt2sas_base.h  |  2 +-
 drivers/scsi/mpt2sas/mpt2sas_ctl.c   |  2 +-
 drivers/scsi/mpt2sas/mpt2sas_scsih.c | 15 ++++++---------
 3 files changed, 8 insertions(+), 11 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_base.h +1 -1
index 1f2ac3a..fd3b998 100644
@@ -1065,7 +1065,7 @@ void mpt2sas_scsih_event_callback(struct MPT2SAS_ADAPTER *ioc, u8 msix_index,
     u32 reply);
 int mpt2sas_scsih_issue_tm(struct MPT2SAS_ADAPTER *ioc, u16 handle,
 	uint channel, uint id, uint lun, u8 type, u16 smid_task,
-	ulong timeout, unsigned long serial_number, enum mutex_type m_type);
+	ulong timeout, enum mutex_type m_type);
 void mpt2sas_scsih_set_tm_flag(struct MPT2SAS_ADAPTER *ioc, u16 handle);
 void mpt2sas_scsih_clear_tm_flag(struct MPT2SAS_ADAPTER *ioc, u16 handle);
 void mpt2sas_expander_remove(struct MPT2SAS_ADAPTER *ioc, u64 sas_address);
drivers/scsi/mpt2sas/mpt2sas_ctl.c +1 -1
index b7f887c..62df8f9 100644
@@ -987,7 +987,7 @@ _ctl_do_mpt_command(struct MPT2SAS_ADAPTER *ioc, struct mpt2_ioctl_command karg,
 			mpt2sas_scsih_issue_tm(ioc,
 			    le16_to_cpu(mpi_request->FunctionDependent1), 0, 0,
 			    0, MPI2_SCSITASKMGMT_TASKTYPE_TARGET_RESET, 0, 10,
-			    0, TM_MUTEX_ON);
+			    TM_MUTEX_ON);
 			ioc->tm_cmds.status = MPT2_CMD_NOT_USED;
 		} else
 			mpt2sas_base_hard_reset_handler(ioc, CAN_SLEEP,
drivers/scsi/mpt2sas/mpt2sas_scsih.c +6 -9
index 7f0af4f..33586a3 100644
@@ -2368,7 +2368,6 @@ mpt2sas_scsih_clear_tm_flag(struct MPT2SAS_ADAPTER *ioc, u16 handle)
  * @type: MPI2_SCSITASKMGMT_TASKTYPE__XXX (defined in mpi2_init.h)
  * @smid_task: smid assigned to the task
  * @timeout: timeout in seconds
- * @serial_number: the serial_number from scmd
  * @m_type: TM_MUTEX_ON or TM_MUTEX_OFF
  * Context: user
  *

@@ -2381,7 +2380,7 @@ mpt2sas_scsih_clear_tm_flag(struct MPT2SAS_ADAPTER *ioc, u16 handle)
 int
 mpt2sas_scsih_issue_tm(struct MPT2SAS_ADAPTER *ioc, u16 handle, uint channel,
     uint id, uint lun, u8 type, u16 smid_task, ulong timeout,
-	unsigned long serial_number, enum mutex_type m_type)
+	enum mutex_type m_type)
 {
 	Mpi2SCSITaskManagementRequest_t *mpi_request;
 	Mpi2SCSITaskManagementReply_t *mpi_reply;

@@ -2634,8 +2633,7 @@ _scsih_abort(struct scsi_cmnd *scmd)
 	handle = sas_device_priv_data->sas_target->handle;
 	r = mpt2sas_scsih_issue_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, scmd->device->lun,
-	    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30,
-	    scmd->serial_number, TM_MUTEX_ON);
+	    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30, TM_MUTEX_ON);
 
  out:
 	sdev_printk(KERN_INFO, scmd->device, "task abort: %s scmd(%p)\n",

@@ -2696,8 +2694,7 @@ _scsih_dev_reset(struct scsi_cmnd *scmd)
 
 	r = mpt2sas_scsih_issue_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, scmd->device->lun,
-	    MPI2_SCSITASKMGMT_TASKTYPE_LOGICAL_UNIT_RESET, 0, 30, 0,
-	    TM_MUTEX_ON);
+	    MPI2_SCSITASKMGMT_TASKTYPE_LOGICAL_UNIT_RESET, 0, 30, TM_MUTEX_ON);
 
  out:
 	sdev_printk(KERN_INFO, scmd->device, "device reset: %s scmd(%p)\n",

@@ -2757,7 +2754,7 @@ _scsih_target_reset(struct scsi_cmnd *scmd)
 
 	r = mpt2sas_scsih_issue_tm(ioc, handle, scmd->device->channel,
 	    scmd->device->id, 0, MPI2_SCSITASKMGMT_TASKTYPE_TARGET_RESET, 0,
-	    30, 0, TM_MUTEX_ON);
+	    30, TM_MUTEX_ON);
 
  out:
 	starget_printk(KERN_INFO, starget, "target reset: %s scmd(%p)\n",

@@ -5880,7 +5877,7 @@ broadcast_aen_retry:
 
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 		r = mpt2sas_scsih_issue_tm(ioc, handle, 0, 0, lun,
-		    MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK, smid, 30, 0,
+		    MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK, smid, 30,
 		    TM_MUTEX_OFF);
 		if (r == FAILED) {
 			sdev_printk(KERN_WARNING, sdev,

@@ -5922,7 +5919,7 @@ broadcast_aen_retry:
 
 		r = mpt2sas_scsih_issue_tm(ioc, handle, sdev->channel, sdev->id,
 		    sdev->lun, MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK, smid, 30,
-		    scmd->serial_number, TM_MUTEX_OFF);
+		    TM_MUTEX_OFF);
 		if (r == FAILED) {
 			sdev_printk(KERN_WARNING, sdev,
 			    "mpt2sas_scsih_issue_tm: ABORT_TASK: FAILED : "
Signature · 1 line
1.9.0
author
#8 lore raw

PATCH 4/7 mpt2sas: Remove use of DEF_SCSI_QCMD

In reply to Matthew Wilcox

Removing the host_lock from the I/O submission path gives a huge scalability improvement.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/scsi/mpt2sas/mpt2sas_scsih.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_scsih.c +3 -6
index 33586a3..7351843 100644
@@ -3950,9 +3950,9 @@ _scsih_setup_direct_io(struct MPT2SAS_ADAPTER *ioc, struct scsi_cmnd *scmd,
  * SCSI_MLQUEUE_HOST_BUSY if the entire host queue is full
  */
 static int
-_scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
+_scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
 {
-	struct MPT2SAS_ADAPTER *ioc = shost_priv(scmd->device->host);
+	struct MPT2SAS_ADAPTER *ioc = shost_priv(shost);
 	struct MPT2SAS_DEVICE *sas_device_priv_data;
 	struct MPT2SAS_TARGET *sas_target_priv_data;
 	struct _raid_device *raid_device;

@@ -3960,7 +3960,6 @@ _scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
 	u32 mpi_control;
 	u16 smid;
 
-	scmd->scsi_done = done;
 	sas_device_priv_data = scmd->device->hostdata;
 	if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
 		scmd->result = DID_NO_CONNECT << 16;

@@ -4036,7 +4035,7 @@ _scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
 	    MPT_TARGET_FLAGS_RAID_COMPONENT)
 		mpi_request->Function = MPI2_FUNCTION_RAID_SCSI_IO_PASSTHROUGH;
 	else
-		mpi_request->Function = MPI2_FUNCTION_SCSI_IO_REQUEST;
+	mpi_request->Function = MPI2_FUNCTION_SCSI_IO_REQUEST;
 	mpi_request->DevHandle =
 	    cpu_to_le16(sas_device_priv_data->sas_target->handle);
 	mpi_request->DataLength = cpu_to_le32(scsi_bufflen(scmd));

@@ -4080,8 +4079,6 @@ _scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
 	return SCSI_MLQUEUE_HOST_BUSY;
 }
 
-static DEF_SCSI_QCMD(_scsih_qcmd)
-
 /**
  * _scsih_normalize_sense - normalize descriptor and fixed format sense data
  * @sense_buffer: sense data returned by target
Signature · 1 line
1.9.0
author
#9 lore raw

PATCH 5/7 mpt2sas: Add free smids to the head, not tail of list

In reply to Matthew Wilcox

Reusing a smid quickly means it's still cache-hot. This yields a small but noticable performance improvement in a well-known database benchmark. This improvement is already present in the mpt3sas driver.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_base.c +4 -4
index bde63f7..8b88118 100644
@@ -1739,14 +1739,14 @@ mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
 			list_for_each_entry_safe(chain_req, next,
 			    &ioc->scsi_lookup[i].chain_list, tracker_list) {
 				list_del_init(&chain_req->tracker_list);
-				list_add_tail(&chain_req->tracker_list,
+				list_add(&chain_req->tracker_list,
 				    &ioc->free_chain_list);
 			}
 		}
 		ioc->scsi_lookup[i].cb_idx = 0xFF;
 		ioc->scsi_lookup[i].scmd = NULL;
 		ioc->scsi_lookup[i].direct_io = 0;
-		list_add_tail(&ioc->scsi_lookup[i].tracker_list,
+		list_add(&ioc->scsi_lookup[i].tracker_list,
 		    &ioc->free_list);
 		spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 

@@ -1764,13 +1764,13 @@ mpt2sas_base_free_smid(struct MPT2SAS_ADAPTER *ioc, u16 smid)
 		/* hi-priority */
 		i = smid - ioc->hi_priority_smid;
 		ioc->hpr_lookup[i].cb_idx = 0xFF;
-		list_add_tail(&ioc->hpr_lookup[i].tracker_list,
+		list_add(&ioc->hpr_lookup[i].tracker_list,
 		    &ioc->hpr_free_list);
 	} else if (smid <= ioc->hba_queue_depth) {
 		/* internal queue */
 		i = smid - ioc->internal_smid;
 		ioc->internal_lookup[i].cb_idx = 0xFF;
-		list_add_tail(&ioc->internal_lookup[i].tracker_list,
+		list_add(&ioc->internal_lookup[i].tracker_list,
 		    &ioc->internal_free_list);
 	}
 	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
Signature · 1 line
1.9.0
author
#11 lore raw

PATCH 6/7 fusion: Add free msg frames to the head, not tail of list

In reply to Matthew Wilcox

Reusing a msg frame quickly means it's still cache-hot. This yields a small but noticable performance improvement in a well-known database benchmark. This improvement is already present in the mpt3sas driver.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/message/fusion/mptbase.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/message/fusion/mptbase.c +1 -1
index 570b18a..ebc0af7 100644
@@ -1037,7 +1037,7 @@ mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf)
 		goto out;
 	/* signature to know if this mf is freed */
 	mf->u.frame.linkage.arg1 = cpu_to_le32(0xdeadbeaf);
-	list_add_tail(&mf->u.frame.linkage.list, &ioc->FreeQ);
+	list_add(&mf->u.frame.linkage.list, &ioc->FreeQ);
 #ifdef MFCNT
 	ioc->mfcnt--;
 #endif
Signature · 1 line
1.9.0
author
#13 lore raw

PATCH 7/7 fusion: Remove use of DEF_SCSI_QCMD

In reply to Matthew Wilcox

Removing the host_lock from the I/O submission path gives a huge scalability improvement.

Signed-off-by Matthew Wilcox <[email protected]>
---
 drivers/message/fusion/mptfc.c    | 12 +++++-------
 drivers/message/fusion/mptsas.c   | 10 ++++------
 drivers/message/fusion/mptscsih.c |  8 +++-----
 drivers/message/fusion/mptscsih.h |  2 +-
 drivers/message/fusion/mptspi.c   | 12 +++++-------
 5 files changed, 18 insertions(+), 26 deletions(-)
drivers/message/fusion/mptfc.c +5 -7
index fd75108..02a3eef 100644
@@ -649,7 +649,7 @@ mptfc_slave_alloc(struct scsi_device *sdev)
 }
 
 static int
-mptfc_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
+mptfc_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *SCpnt)
 {
 	struct mptfc_rport_info	*ri;
 	struct fc_rport	*rport = starget_to_rport(scsi_target(SCpnt->device));

@@ -658,14 +658,14 @@ mptfc_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 
 	if (!vdevice || !vdevice->vtarget) {
 		SCpnt->result = DID_NO_CONNECT << 16;
-		done(SCpnt);
+		SCpnt->scsi_done(SCpnt);
 		return 0;
 	}
 
 	err = fc_remote_port_chkready(rport);
 	if (unlikely(err)) {
 		SCpnt->result = err;
-		done(SCpnt);
+		SCpnt->scsi_done(SCpnt);
 		return 0;
 	}
 

@@ -673,15 +673,13 @@ mptfc_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 	ri = *((struct mptfc_rport_info **)rport->dd_data);
 	if (unlikely(!ri)) {
 		SCpnt->result = DID_IMM_RETRY << 16;
-		done(SCpnt);
+		SCpnt->scsi_done(SCpnt);
 		return 0;
 	}
 
-	return mptscsih_qcmd(SCpnt,done);
+	return mptscsih_qcmd(SCpnt);
 }
 
-static DEF_SCSI_QCMD(mptfc_qcmd)
-
 /*
  *	mptfc_display_port_link_speed - displaying link speed
  *	@ioc: Pointer to MPT_ADAPTER structure
drivers/message/fusion/mptsas.c +4 -6
index 00d339c..711fcb5 100644
@@ -1896,7 +1896,7 @@ mptsas_slave_alloc(struct scsi_device *sdev)
 }
 
 static int
-mptsas_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
+mptsas_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
 	MPT_ADAPTER	*ioc;

@@ -1904,11 +1904,11 @@ mptsas_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 
 	if (!vdevice || !vdevice->vtarget || vdevice->vtarget->deleted) {
 		SCpnt->result = DID_NO_CONNECT << 16;
-		done(SCpnt);
+		SCpnt->scsi_done(SCpnt);
 		return 0;
 	}
 
-	hd = shost_priv(SCpnt->device->host);
+	hd = shost_priv(shost);
 	ioc = hd->ioc;
 
 	if (ioc->sas_discovery_quiesce_io)

@@ -1917,11 +1917,9 @@ mptsas_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 	if (ioc->debug_level & MPT_DEBUG_SCSI)
 		scsi_print_command(SCpnt);
 
-	return mptscsih_qcmd(SCpnt,done);
+	return mptscsih_qcmd(SCpnt);
 }
 
-static DEF_SCSI_QCMD(mptsas_qcmd)
-
 /**
  *	mptsas_mptsas_eh_timed_out - resets the scsi_cmnd timeout
  *		if the device under question is currently in the
drivers/message/fusion/mptscsih.c +3 -5
index 727819c..2a1c6f2 100644
@@ -1304,7 +1304,6 @@ int mptscsih_show_info(struct seq_file *m, struct Scsi_Host *host)
 /**
  *	mptscsih_qcmd - Primary Fusion MPT SCSI initiator IO start routine.
  *	@SCpnt: Pointer to scsi_cmnd structure
- *	@done: Pointer SCSI mid-layer IO completion function
  *
  *	(linux scsi_host_template.queuecommand routine)
  *	This is the primary SCSI IO start routine.  Create a MPI SCSIIORequest

@@ -1313,7 +1312,7 @@ int mptscsih_show_info(struct seq_file *m, struct Scsi_Host *host)
  *	Returns 0. (rtn value discarded by linux scsi mid-layer)
  */
 int
-mptscsih_qcmd(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
+mptscsih_qcmd(struct scsi_cmnd *SCpnt)
 {
 	MPT_SCSI_HOST		*hd;
 	MPT_FRAME_HDR		*mf;

@@ -1329,10 +1328,9 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 
 	hd = shost_priv(SCpnt->device->host);
 	ioc = hd->ioc;
-	SCpnt->scsi_done = done;
 
-	dmfprintk(ioc, printk(MYIOC_s_DEBUG_FMT "qcmd: SCpnt=%p, done()=%p\n",
-		ioc->name, SCpnt, done));
+	dmfprintk(ioc, printk(MYIOC_s_DEBUG_FMT "qcmd: SCpnt=%p\n",
+		ioc->name, SCpnt));
 
 	if (ioc->taskmgmt_quiesce_io)
 		return SCSI_MLQUEUE_HOST_BUSY;
drivers/message/fusion/mptscsih.h +1 -1
index 83f5031..99e3390 100644
@@ -113,7 +113,7 @@ extern int mptscsih_resume(struct pci_dev *pdev);
 #endif
 extern int mptscsih_show_info(struct seq_file *, struct Scsi_Host *);
 extern const char * mptscsih_info(struct Scsi_Host *SChost);
-extern int mptscsih_qcmd(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *));
+extern int mptscsih_qcmd(struct scsi_cmnd *SCpnt);
 extern int mptscsih_IssueTaskMgmt(MPT_SCSI_HOST *hd, u8 type, u8 channel,
 	u8 id, int lun, int ctx2abort, ulong timeout);
 extern void mptscsih_slave_destroy(struct scsi_device *device);
drivers/message/fusion/mptspi.c +5 -7
index 5653e50..49d1133 100644
@@ -780,33 +780,31 @@ static int mptspi_slave_configure(struct scsi_device *sdev)
 }
 
 static int
-mptspi_qcmd_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
+mptspi_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *SCpnt)
 {
-	struct _MPT_SCSI_HOST *hd = shost_priv(SCpnt->device->host);
+	struct _MPT_SCSI_HOST *hd = shost_priv(shost);
 	VirtDevice	*vdevice = SCpnt->device->hostdata;
 	MPT_ADAPTER *ioc = hd->ioc;
 
 	if (!vdevice || !vdevice->vtarget) {
 		SCpnt->result = DID_NO_CONNECT << 16;
-		done(SCpnt);
+		SCpnt->scsi_done(SCpnt);
 		return 0;
 	}
 
 	if (SCpnt->device->channel == 1 &&
 		mptscsih_is_phys_disk(ioc, 0, SCpnt->device->id) == 0) {
 		SCpnt->result = DID_NO_CONNECT << 16;
-		done(SCpnt);
+		SCpnt->scsi_done(SCpnt);
 		return 0;
 	}
 
 	if (spi_dv_pending(scsi_target(SCpnt->device)))
 		ddvprintk(ioc, scsi_print_command(SCpnt));
 
-	return mptscsih_qcmd(SCpnt,done);
+	return mptscsih_qcmd(SCpnt);
 }
 
-static DEF_SCSI_QCMD(mptspi_qcmd)
-
 static void mptspi_slave_destroy(struct scsi_device *sdev)
 {
 	struct scsi_target *starget = scsi_target(sdev);
Signature · 1 line
1.9.0
Reviewed-by on the v1 series
#14 lore raw
In reply to Matthew Wilcox

Hi Matthew,

Quoted 10 lines · Matthew Wilcox

The host lock is a serious scalability problem on 2-socket and larger systems which are doing a lot of I/O. Before removing the temporary usgae of DEF_SCSI_QCMD, we need to remove all uses of serial_number.

An unrelated performance issue is that reusing the most recent driver-specific data structure to track the I/O instead of the least recently used keeps the cache-hot lines in use, which is a nice performance improvement. It's already present in the mpt3sas driver, it just didn't make it into the fusion or mpt2sas drivers yet.

Quoted 10 lines

Matthew Wilcox (7):
mpt3sas: Remove uses of serial_number
mpt3sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Remove uses of serial_number
mpt2sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Add free smids to the head, not tail of list
fusion: Add free msg frames to the head, not tail of list
fusion: Remove use of DEF_SCSI_QCMD

+1 to this long overdue series to enable host_lock-less mode with mpt*sas + fusion. (CC'ing LSI folks + jejb)

Reviewed-by Nicholas Bellinger <[email protected]>
#15 lore raw

CC few more LSI driver developers.

Quoted 37 lines

-----Original Message-----
From: Nicholas A. Bellinger [mailto:[email protected]]
Sent: Monday, March 31, 2014 11:25 AM
To: Matthew Wilcox
Cc: [email protected]; [email protected]; Desai, Kashyap; Saxena, Sumit; James Bottomley
Subject: Re: [PATCH 0/7] Performance improvements for LSI SCSI cards

Hi Matthew,

Quoted 10 lines · Matthew Wilcox

The host lock is a serious scalability problem on 2-socket and larger systems which are doing a lot of I/O. Before removing the temporary usgae of DEF_SCSI_QCMD, we need to remove all uses of serial_number.

An unrelated performance issue is that reusing the most recent driver-specific data structure to track the I/O instead of the least recently used keeps the cache-hot lines in use, which is a nice performance improvement. It's already present in the mpt3sas driver, it just didn't make it into the fusion or mpt2sas drivers yet.

Quoted 10 lines

Matthew Wilcox (7):
mpt3sas: Remove uses of serial_number
mpt3sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Remove uses of serial_number
mpt2sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Add free smids to the head, not tail of list
fusion: Add free msg frames to the head, not tail of list
fusion: Remove use of DEF_SCSI_QCMD

+1 to this long overdue series to enable host_lock-less mode with mpt*sas + fusion. (CC'ing LSI folks + jejb)

Reviewed-by Nicholas Bellinger <[email protected]>
#17 lore raw
In reply to Matthew Wilcox

Hi James,

This patch set seem to be fine. Please consider this patch set as Acked-by: "Sreekanth Reddy" <[email protected]>.

Regards,
Sreekanth

Quoted 49 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Matthew Wilcox
Sent: Friday, March 28, 2014 2:10 AM
To: [email protected]
Cc: Matthew Wilcox; [email protected]
Subject: [PATCH 0/7] Performance improvements for LSI SCSI cards

The host lock is a serious scalability problem on 2-socket and larger systems which are doing a lot of I/O. Before removing the temporary usgae of DEF_SCSI_QCMD, we need to remove all uses of serial_number.

An unrelated performance issue is that reusing the most recent driver-specific data structure to track the I/O instead of the least recently used keeps the cache-hot lines in use, which is a nice performance improvement. It's already present in the mpt3sas driver, it just didn't make it into the fusion or mpt2sas drivers yet.

Matthew Wilcox (7):
mpt3sas: Remove uses of serial_number
mpt3sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Remove uses of serial_number
mpt2sas: Remove use of DEF_SCSI_QCMD
mpt2sas: Add free smids to the head, not tail of list
fusion: Add free msg frames to the head, not tail of list
fusion: Remove use of DEF_SCSI_QCMD

drivers/message/fusion/mptbase.c     |  2 +-
drivers/message/fusion/mptfc.c       | 12 +++++-------
drivers/message/fusion/mptsas.c      | 10 ++++------
drivers/message/fusion/mptscsih.c    |  8 +++-----
drivers/message/fusion/mptscsih.h    |  2 +-
drivers/message/fusion/mptspi.c      | 12 +++++-------
drivers/scsi/mpt2sas/mpt2sas_base.c  |  8 ++++----
drivers/scsi/mpt2sas/mpt2sas_base.h  |  2 +-
drivers/scsi/mpt2sas/mpt2sas_ctl.c   |  2 +-
drivers/scsi/mpt2sas/mpt2sas_scsih.c | 24 +++++++++---------------
drivers/scsi/mpt3sas/mpt3sas_base.h  |  2 +-
drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  2 +-
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 24 +++++++++---------------
13 files changed, 45 insertions(+), 65 deletions(-)

--
1.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html