Re: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVD drive connected to mptspi driver

Re: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVD drive connected to mptspi driver

[Andrew Morton]

(Switching to email - please use reply-to-all)

On Thu, 3 May 2007 15:26:41 -0700
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=8426
> 
>            Summary: massive slowdown on SCSI CD/DVD drive connected to
>                     mptspi driver
>     Kernel Version: 2.6.21
>             Status: NEW
>           Severity: high
>              Owner: scsi_drivers-other@kernel-bugs.osdl.org
>          Submitter: doug.chapman@hp.com
> 
> 
> Most recent kernel where this bug did *NOT* occur:
> 2.6.20
> 
> Distribution:
> fedora 7 - test 4
> seen with fedora kernel and stock upstream 2.6.21 kernel as well
> 
> Hardware Environment:
> HP Integrity Superdome
> NEC  DVD_RW ND-3540A
> LSI Fusion MPT SPI SCSI adapter
> 
> also on:
> HP Integrity rx8640
> NEC DVD+RW ND-2100AD
> 
> 
> Software Environment: fedora 7 test 4
> 
> Problem Description:
> Reading CD/DVD is approx 150times slower than usual.  Simple test of:
> time cat kernel-2.6.20-1.3088.fc7.ia64.rpm > /dev/null
> 
> took approx 10 minutes on 2.6.21, took 2-4 seconds on 2.6.20 (same system, same
> disk, repeated multiple times to ensure it wasn't bad hardware)
> 
> 
> Steps to reproduce:
> mount a DVD
> read a large file from the DVD
> 
> 
> Appears to have been caused by this git commit:

Thanks heaps for doing the bisection - it really helps.

> commit 5a9c47b1344b514758d5d7f193c672850390cc36
> Author: Eric Moore <eric.moore@lsi.com>
> Date:   Mon Jan 29 09:43:17 2007 -0700
> 
>     [SCSI] fusion - move SPI API over to mptspi.c
> 
>     Move some functions that only apply to the mptspi module over from mptscsih.
>     Signed-off-by: Eric Moore <Eric.Moore@lsi.com>
>     Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
> 
> 
> I backed out this patch against the current HEAD and DVD drive performance is
> back to normal.  I have not yet looked into this to determine exactly what in
> this patch caused the issue.

That's a bit surprising - all the patch allegedly does is move stuff
around.  Here's my version of a backout patch against 2.6.21 - can anyone
spot any bloopers in it?


 drivers/message/fusion/mptscsih.c |  284 ++++++++++++++++++++++++++++
 drivers/message/fusion/mptspi.c   |  268 --------------------------
 2 files changed, 285 insertions(+), 267 deletions(-)

diff -puN drivers/message/fusion/mptscsih.c~revert-fusion-move-spi-api-over-to-mptspic drivers/message/fusion/mptscsih.c
--- a/drivers/message/fusion/mptscsih.c~revert-fusion-move-spi-api-over-to-mptspic
+++ a/drivers/message/fusion/mptscsih.c
@@ -98,6 +98,9 @@ static int	mptscsih_IssueTaskMgmt(MPT_SC
 int		mptscsih_ioc_reset(MPT_ADAPTER *ioc, int post_reset);
 int		mptscsih_event_process(MPT_ADAPTER *ioc, EventNotificationReply_t *pEvReply);
 
+static void	mptscsih_initTarget(MPT_SCSI_HOST *hd, VirtTarget *vtarget, struct scsi_device *sdev);
+static void	mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtTarget *vtarget, struct scsi_device *sdev);
+static int	mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id);
 int		mptscsih_scandv_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *r);
 static int	mptscsih_do_cmd(MPT_SCSI_HOST *hd, INTERNAL_CMD *iocmd);
 static void	mptscsih_synchronize_cache(MPT_SCSI_HOST *hd, VirtDevice *vdevice);
@@ -2410,6 +2413,7 @@ mptscsih_slave_configure(struct scsi_dev
 	}
 
 	vdevice->configured_lun = 1;
+	mptscsih_initTarget(hd, vtarget, sdev);
 	mptscsih_change_queue_depth(sdev, MPT_SCSI_CMD_PER_DEV_HIGH);
 
 	dsprintk((MYIOC_s_INFO_FMT
@@ -2673,6 +2677,286 @@ mptscsih_event_process(MPT_ADAPTER *ioc,
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /*
+ *	mptscsih_initTarget - Target, LUN alloc/free functionality.
+ *	@hd: Pointer to MPT_SCSI_HOST structure
+ *	@vtarget: per target private data
+ *	@sdev: SCSI device
+ *
+ *	NOTE: It's only SAFE to call this routine if data points to
+ *	sane & valid STANDARD INQUIRY data!
+ *
+ *	Allocate and initialize memory for this target.
+ *	Save inquiry data.
+ *
+ */
+static void
+mptscsih_initTarget(MPT_SCSI_HOST *hd, VirtTarget *vtarget,
+		    struct scsi_device *sdev)
+{
+	dinitprintk((MYIOC_s_INFO_FMT "initTarget channel=%d id=%d lun=%d hd=%p\n",
+		hd->ioc->name, vtarget->channel, vtarget->id,
+		sdev->lun, hd));
+
+	/* Is LUN supported? If so, upper 2 bits will be 0
+	* in first byte of inquiry data.
+	*/
+	if (sdev->inq_periph_qual != 0)
+		return;
+
+	if (vtarget == NULL)
+		return;
+
+	vtarget->type = sdev->type;
+
+	if (hd->ioc->bus_type != SPI)
+		return;
+
+	if ((sdev->type == TYPE_PROCESSOR) && (hd->ioc->spi_data.Saf_Te)) {
+		/* Treat all Processors as SAF-TE if
+		 * command line option is set */
+		vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
+		mptscsih_writeIOCPage4(hd, vtarget->channel, vtarget->id);
+	}else if ((sdev->type == TYPE_PROCESSOR) &&
+		!(vtarget->tflags & MPT_TARGET_FLAGS_SAF_TE_ISSUED )) {
+		if (sdev->inquiry_len > 49 ) {
+			if (sdev->inquiry[44] == 'S' &&
+			    sdev->inquiry[45] == 'A' &&
+			    sdev->inquiry[46] == 'F' &&
+			    sdev->inquiry[47] == '-' &&
+			    sdev->inquiry[48] == 'T' &&
+			    sdev->inquiry[49] == 'E' ) {
+				vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
+				mptscsih_writeIOCPage4(hd, vtarget->channel, vtarget->id);
+			}
+		}
+	}
+	mptscsih_setTargetNegoParms(hd, vtarget, sdev);
+}
+
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*
+ *  Update the target negotiation parameters based on the
+ *  the Inquiry data, adapter capabilities, and NVRAM settings.
+ *
+ */
+static void
+mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtTarget *target,
+			    struct scsi_device *sdev)
+{
+	SpiCfgData *pspi_data = &hd->ioc->spi_data;
+	int  id = (int) target->id;
+	int  nvram;
+	u8 width = MPT_NARROW;
+	u8 factor = MPT_ASYNC;
+	u8 offset = 0;
+	u8 nfactor;
+	u8 noQas = 1;
+
+	target->negoFlags = pspi_data->noQas;
+
+	/* noQas == 0 => device supports QAS. */
+
+	if (sdev->scsi_level < SCSI_2) {
+		width = 0;
+		factor = MPT_ULTRA2;
+		offset = pspi_data->maxSyncOffset;
+		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
+	} else {
+		if (scsi_device_wide(sdev)) {
+			width = 1;
+		}
+
+		if (scsi_device_sync(sdev)) {
+			factor = pspi_data->minSyncFactor;
+			if (!scsi_device_dt(sdev))
+					factor = MPT_ULTRA2;
+			else {
+				if (!scsi_device_ius(sdev) &&
+				    !scsi_device_qas(sdev))
+					factor = MPT_ULTRA160;
+				else {
+					factor = MPT_ULTRA320;
+					if (scsi_device_qas(sdev)) {
+						ddvtprintk((KERN_INFO "Enabling QAS due to byte56=%02x on id=%d!\n", scsi_device_qas(sdev), id));
+						noQas = 0;
+					}
+					if (sdev->type == TYPE_TAPE &&
+					    scsi_device_ius(sdev))
+						target->negoFlags |= MPT_TAPE_NEGO_IDP;
+				}
+			}
+			offset = pspi_data->maxSyncOffset;
+
+			/* If RAID, never disable QAS
+			 * else if non RAID, do not disable
+			 *   QAS if bit 1 is set
+			 * bit 1 QAS support, non-raid only
+			 * bit 0 IU support
+			 */
+			if (target->raidVolume == 1) {
+				noQas = 0;
+			}
+		} else {
+			factor = MPT_ASYNC;
+			offset = 0;
+		}
+	}
+
+	if (!sdev->tagged_supported) {
+		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
+	}
+
+	/* Update tflags based on NVRAM settings. (SCSI only)
+	 */
+	if (pspi_data->nvram && (pspi_data->nvram[id] != MPT_HOST_NVRAM_INVALID)) {
+		nvram = pspi_data->nvram[id];
+		nfactor = (nvram & MPT_NVRAM_SYNC_MASK) >> 8;
+
+		if (width)
+			width = nvram & MPT_NVRAM_WIDE_DISABLE ? 0 : 1;
+
+		if (offset > 0) {
+			/* Ensure factor is set to the
+			 * maximum of: adapter, nvram, inquiry
+			 */
+			if (nfactor) {
+				if (nfactor < pspi_data->minSyncFactor )
+					nfactor = pspi_data->minSyncFactor;
+
+				factor = max(factor, nfactor);
+				if (factor == MPT_ASYNC)
+					offset = 0;
+			} else {
+				offset = 0;
+				factor = MPT_ASYNC;
+		}
+		} else {
+			factor = MPT_ASYNC;
+		}
+	}
+
+	/* Make sure data is consistent
+	 */
+	if ((!width) && (factor < MPT_ULTRA2)) {
+		factor = MPT_ULTRA2;
+	}
+
+	/* Save the data to the target structure.
+	 */
+	target->minSyncFactor = factor;
+	target->maxOffset = offset;
+	target->maxWidth = width;
+
+	target->tflags |= MPT_TARGET_FLAGS_VALID_NEGO;
+
+	/* Disable unused features.
+	 */
+	if (!width)
+		target->negoFlags |= MPT_TARGET_NO_NEGO_WIDE;
+
+	if (!offset)
+		target->negoFlags |= MPT_TARGET_NO_NEGO_SYNC;
+
+	if ( factor > MPT_ULTRA320 )
+		noQas = 0;
+
+	if (noQas && (pspi_data->noQas == 0)) {
+		pspi_data->noQas |= MPT_TARGET_NO_NEGO_QAS;
+		target->negoFlags |= MPT_TARGET_NO_NEGO_QAS;
+
+		/* Disable QAS in a mixed configuration case
+		 */
+
+		ddvtprintk((KERN_INFO "Disabling QAS due to noQas=%02x on id=%d!\n", noQas, id));
+	}
+}
+
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*
+ *  SCSI Config Page functionality ...
+ */
+
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*	mptscsih_writeIOCPage4  - write IOC Page 4
+ *	@hd: Pointer to a SCSI Host Structure
+ *	@channel: write IOC Page4 for this Bus
+ *	@id: write IOC Page4 for this ID
+ *
+ *	Return: -EAGAIN if unable to obtain a Message Frame
+ *		or 0 if success.
+ *
+ *	Remark: We do not wait for a return, write pages sequentially.
+ */
+static int
+mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id)
+{
+	MPT_ADAPTER		*ioc = hd->ioc;
+	Config_t		*pReq;
+	IOCPage4_t		*IOCPage4Ptr;
+	MPT_FRAME_HDR		*mf;
+	dma_addr_t		 dataDma;
+	u16			 req_idx;
+	u32			 frameOffset;
+	u32			 flagsLength;
+	int			 ii;
+
+	/* Get a MF for this command.
+	 */
+	if ((mf = mpt_get_msg_frame(ioc->DoneCtx, ioc)) == NULL) {
+		dfailprintk((MYIOC_s_WARN_FMT "writeIOCPage4 : no msg frames!\n",
+					ioc->name));
+		return -EAGAIN;
+	}
+
+	/* Set the request and the data pointers.
+	 * Place data at end of MF.
+	 */
+	pReq = (Config_t *)mf;
+
+	req_idx = le16_to_cpu(mf->u.frame.hwhdr.msgctxu.fld.req_idx);
+	frameOffset = ioc->req_sz - sizeof(IOCPage4_t);
+
+	/* Complete the request frame (same for all requests).
+	 */
+	pReq->Action = MPI_CONFIG_ACTION_PAGE_WRITE_CURRENT;
+	pReq->Reserved = 0;
+	pReq->ChainOffset = 0;
+	pReq->Function = MPI_FUNCTION_CONFIG;
+	pReq->ExtPageLength = 0;
+	pReq->ExtPageType = 0;
+	pReq->MsgFlags = 0;
+	for (ii=0; ii < 8; ii++) {
+		pReq->Reserved2[ii] = 0;
+	}
+
+	IOCPage4Ptr = ioc->spi_data.pIocPg4;
+	dataDma = ioc->spi_data.IocPg4_dma;
+	ii = IOCPage4Ptr->ActiveSEP++;
+	IOCPage4Ptr->SEP[ii].SEPTargetID = id;
+	IOCPage4Ptr->SEP[ii].SEPBus = channel;
+	pReq->Header = IOCPage4Ptr->Header;
+	pReq->PageAddress = cpu_to_le32(id | (channel << 8 ));
+
+	/* Add a SGE to the config request.
+	 */
+	flagsLength = MPT_SGE_FLAGS_SSIMPLE_WRITE |
+		(IOCPage4Ptr->Header.PageLength + ii) * 4;
+
+	mpt_add_sge((char *)&pReq->PageBufferSGE, flagsLength, dataDma);
+
+	dinitprintk((MYIOC_s_INFO_FMT
+		"writeIOCPage4: MaxSEP=%d ActiveSEP=%d channel=%d id=%d \n",
+			ioc->name, IOCPage4Ptr->MaxSEP, IOCPage4Ptr->ActiveSEP, channel, id));
+
+	mpt_put_msg_frame(ioc->DoneCtx, ioc, mf);
+
+	return 0;
+}
+
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*
  *  Bus Scan and Domain Validation functionality ...
  */
 
diff -puN drivers/message/fusion/mptspi.c~revert-fusion-move-spi-api-over-to-mptspic drivers/message/fusion/mptspi.c
--- a/drivers/message/fusion/mptspi.c~revert-fusion-move-spi-api-over-to-mptspic
+++ a/drivers/message/fusion/mptspi.c
@@ -95,269 +95,6 @@ static int	mptspiDoneCtx = -1;
 static int	mptspiTaskCtx = -1;
 static int	mptspiInternalCtx = -1; /* Used only for internal commands */
 
-/**
- * 	mptspi_setTargetNegoParms  - Update the target negotiation
- *	parameters based on the the Inquiry data, adapter capabilities,
- *	and NVRAM settings
- *
- *	@hd: Pointer to a SCSI Host Structure
- *	@vtarget: per target private data
- *	@sdev: SCSI device
- *
- **/
-static void
-mptspi_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtTarget *target,
-			    struct scsi_device *sdev)
-{
-	SpiCfgData *pspi_data = &hd->ioc->spi_data;
-	int  id = (int) target->id;
-	int  nvram;
-	u8 width = MPT_NARROW;
-	u8 factor = MPT_ASYNC;
-	u8 offset = 0;
-	u8 nfactor;
-	u8 noQas = 1;
-
-	target->negoFlags = pspi_data->noQas;
-
-	if (sdev->scsi_level < SCSI_2) {
-		width = 0;
-		factor = MPT_ULTRA2;
-		offset = pspi_data->maxSyncOffset;
-		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
-	} else {
-		if (scsi_device_wide(sdev))
-			width = 1;
-
-		if (scsi_device_sync(sdev)) {
-			factor = pspi_data->minSyncFactor;
-			if (!scsi_device_dt(sdev))
-					factor = MPT_ULTRA2;
-			else {
-				if (!scsi_device_ius(sdev) &&
-				    !scsi_device_qas(sdev))
-					factor = MPT_ULTRA160;
-				else {
-					factor = MPT_ULTRA320;
-					if (scsi_device_qas(sdev)) {
-						ddvprintk((KERN_INFO "Enabling QAS due to byte56=%02x on id=%d!\n", scsi_device_qas(sdev), id));
-						noQas = 0;
-					}
-					if (sdev->type == TYPE_TAPE &&
-					    scsi_device_ius(sdev))
-						target->negoFlags |= MPT_TAPE_NEGO_IDP;
-				}
-			}
-			offset = pspi_data->maxSyncOffset;
-
-			/* If RAID, never disable QAS
-			 * else if non RAID, do not disable
-			 *   QAS if bit 1 is set
-			 * bit 1 QAS support, non-raid only
-			 * bit 0 IU support
-			 */
-			if (target->raidVolume == 1)
-				noQas = 0;
-		} else {
-			factor = MPT_ASYNC;
-			offset = 0;
-		}
-	}
-
-	if (!sdev->tagged_supported)
-		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
-
-	/* Update tflags based on NVRAM settings. (SCSI only)
-	 */
-	if (pspi_data->nvram && (pspi_data->nvram[id] != MPT_HOST_NVRAM_INVALID)) {
-		nvram = pspi_data->nvram[id];
-		nfactor = (nvram & MPT_NVRAM_SYNC_MASK) >> 8;
-
-		if (width)
-			width = nvram & MPT_NVRAM_WIDE_DISABLE ? 0 : 1;
-
-		if (offset > 0) {
-			/* Ensure factor is set to the
-			 * maximum of: adapter, nvram, inquiry
-			 */
-			if (nfactor) {
-				if (nfactor < pspi_data->minSyncFactor )
-					nfactor = pspi_data->minSyncFactor;
-
-				factor = max(factor, nfactor);
-				if (factor == MPT_ASYNC)
-					offset = 0;
-			} else {
-				offset = 0;
-				factor = MPT_ASYNC;
-		}
-		} else {
-			factor = MPT_ASYNC;
-		}
-	}
-
-	/* Make sure data is consistent
-	 */
-	if ((!width) && (factor < MPT_ULTRA2))
-		factor = MPT_ULTRA2;
-
-	/* Save the data to the target structure.
-	 */
-	target->minSyncFactor = factor;
-	target->maxOffset = offset;
-	target->maxWidth = width;
-
-	target->tflags |= MPT_TARGET_FLAGS_VALID_NEGO;
-
-	/* Disable unused features.
-	 */
-	if (!width)
-		target->negoFlags |= MPT_TARGET_NO_NEGO_WIDE;
-
-	if (!offset)
-		target->negoFlags |= MPT_TARGET_NO_NEGO_SYNC;
-
-	if ( factor > MPT_ULTRA320 )
-		noQas = 0;
-
-	if (noQas && (pspi_data->noQas == 0)) {
-		pspi_data->noQas |= MPT_TARGET_NO_NEGO_QAS;
-		target->negoFlags |= MPT_TARGET_NO_NEGO_QAS;
-
-		/* Disable QAS in a mixed configuration case
-		 */
-
-		ddvprintk((KERN_INFO "Disabling QAS due to noQas=%02x on id=%d!\n", noQas, id));
-	}
-}
-
-/**
- * 	mptspi_writeIOCPage4  - write IOC Page 4
- *	@hd: Pointer to a SCSI Host Structure
- *	@channel:
- *	@id: write IOC Page4 for this ID & Bus
- *
- *	Return: -EAGAIN if unable to obtain a Message Frame
- *		or 0 if success.
- *
- *	Remark: We do not wait for a return, write pages sequentially.
- **/
-static int
-mptspi_writeIOCPage4(MPT_SCSI_HOST *hd, u8 channel , u8 id)
-{
-	MPT_ADAPTER		*ioc = hd->ioc;
-	Config_t		*pReq;
-	IOCPage4_t		*IOCPage4Ptr;
-	MPT_FRAME_HDR		*mf;
-	dma_addr_t		 dataDma;
-	u16			 req_idx;
-	u32			 frameOffset;
-	u32			 flagsLength;
-	int			 ii;
-
-	/* Get a MF for this command.
-	 */
-	if ((mf = mpt_get_msg_frame(ioc->DoneCtx, ioc)) == NULL) {
-		dfailprintk((MYIOC_s_WARN_FMT "writeIOCPage4 : no msg frames!\n",
-					ioc->name));
-		return -EAGAIN;
-	}
-
-	/* Set the request and the data pointers.
-	 * Place data at end of MF.
-	 */
-	pReq = (Config_t *)mf;
-
-	req_idx = le16_to_cpu(mf->u.frame.hwhdr.msgctxu.fld.req_idx);
-	frameOffset = ioc->req_sz - sizeof(IOCPage4_t);
-
-	/* Complete the request frame (same for all requests).
-	 */
-	pReq->Action = MPI_CONFIG_ACTION_PAGE_WRITE_CURRENT;
-	pReq->Reserved = 0;
-	pReq->ChainOffset = 0;
-	pReq->Function = MPI_FUNCTION_CONFIG;
-	pReq->ExtPageLength = 0;
-	pReq->ExtPageType = 0;
-	pReq->MsgFlags = 0;
-	for (ii=0; ii < 8; ii++) {
-		pReq->Reserved2[ii] = 0;
-	}
-
-	IOCPage4Ptr = ioc->spi_data.pIocPg4;
-	dataDma = ioc->spi_data.IocPg4_dma;
-	ii = IOCPage4Ptr->ActiveSEP++;
-	IOCPage4Ptr->SEP[ii].SEPTargetID = id;
-	IOCPage4Ptr->SEP[ii].SEPBus = channel;
-	pReq->Header = IOCPage4Ptr->Header;
-	pReq->PageAddress = cpu_to_le32(id | (channel << 8 ));
-
-	/* Add a SGE to the config request.
-	 */
-	flagsLength = MPT_SGE_FLAGS_SSIMPLE_WRITE |
-		(IOCPage4Ptr->Header.PageLength + ii) * 4;
-
-	mpt_add_sge((char *)&pReq->PageBufferSGE, flagsLength, dataDma);
-
-	ddvprintk((MYIOC_s_INFO_FMT
-		"writeIOCPage4: MaxSEP=%d ActiveSEP=%d id=%d bus=%d\n",
-			ioc->name, IOCPage4Ptr->MaxSEP, IOCPage4Ptr->ActiveSEP, id, channel));
-
-	mpt_put_msg_frame(ioc->DoneCtx, ioc, mf);
-
-	return 0;
-}
-
-/**
- *	mptspi_initTarget - Target, LUN alloc/free functionality.
- *	@hd: Pointer to MPT_SCSI_HOST structure
- *	@vtarget: per target private data
- *	@sdev: SCSI device
- *
- *	NOTE: It's only SAFE to call this routine if data points to
- *	sane & valid STANDARD INQUIRY data!
- *
- *	Allocate and initialize memory for this target.
- *	Save inquiry data.
- *
- **/
-static void
-mptspi_initTarget(MPT_SCSI_HOST *hd, VirtTarget *vtarget,
-		    struct scsi_device *sdev)
-{
-
-	/* Is LUN supported? If so, upper 2 bits will be 0
-	* in first byte of inquiry data.
-	*/
-	if (sdev->inq_periph_qual != 0)
-		return;
-
-	if (vtarget == NULL)
-		return;
-
-	vtarget->type = sdev->type;
-
-	if ((sdev->type == TYPE_PROCESSOR) && (hd->ioc->spi_data.Saf_Te)) {
-		/* Treat all Processors as SAF-TE if
-		 * command line option is set */
-		vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
-		mptspi_writeIOCPage4(hd, vtarget->channel, vtarget->id);
-	}else if ((sdev->type == TYPE_PROCESSOR) &&
-		!(vtarget->tflags & MPT_TARGET_FLAGS_SAF_TE_ISSUED )) {
-		if (sdev->inquiry_len > 49 ) {
-			if (sdev->inquiry[44] == 'S' &&
-			    sdev->inquiry[45] == 'A' &&
-			    sdev->inquiry[46] == 'F' &&
-			    sdev->inquiry[47] == '-' &&
-			    sdev->inquiry[48] == 'T' &&
-			    sdev->inquiry[49] == 'E' ) {
-				vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
-				mptspi_writeIOCPage4(hd, vtarget->channel, vtarget->id);
-			}
-		}
-	}
-	mptspi_setTargetNegoParms(hd, vtarget, sdev);
-}
 
 /**
  *	mptspi_is_raid - Determines whether target is belonging to volume
@@ -723,16 +460,13 @@ static int mptspi_slave_alloc(struct scs
 
 static int mptspi_slave_configure(struct scsi_device *sdev)
 {
+	int ret = mptscsih_slave_configure(sdev);
 	struct _MPT_SCSI_HOST *hd =
 		(struct _MPT_SCSI_HOST *)sdev->host->hostdata;
-	VirtTarget *vtarget = scsi_target(sdev)->hostdata;
-	int ret = mptscsih_slave_configure(sdev);
 
 	if (ret)
 		return ret;
 
-	mptspi_initTarget(hd, vtarget, sdev);
-
 	ddvprintk((MYIOC_s_INFO_FMT "id=%d min_period=0x%02x"
 		" max_offset=0x%02x max_width=%d\n", hd->ioc->name,
 		sdev->id, spi_min_period(scsi_target(sdev)),
_

Re: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVD drive connected to mptspi driver

[Doug Chapman]

On Thu, 2007-05-03 at 17:20 -0700, Andrew Morton wrote:
> (Switching to email - please use reply-to-all)
> 
> On Thu, 3 May 2007 15:26:41 -0700
> bugme-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=8426
> > 
> >            Summary: massive slowdown on SCSI CD/DVD drive connected to
> >                     mptspi driver
> >   
... snip ...
> That's a bit surprising - all the patch allegedly does is move stuff
> around.  Here's my version of a backout patch against 2.6.21 - can anyone
> spot any bloopers in it?
> 

I thought the same thing at first and didn't suspect this patch until I
tried it.  Looks like it does a little more than move stuff around.  It
adds mptspi_initTarget.  I will try backing out just that bit tomorrow
to see if that fixes the issue.

- Doug

Re: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVD drive connected to mptspi driver

[Doug Chapman]

On Thu, 2007-05-03 at 17:20 -0700, Andrew Morton wrote:
> (Switching to email - please use reply-to-all)
> 
> On Thu, 3 May 2007 15:26:41 -0700
> bugme-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=8426
> > 
> >            Summary: massive slowdown on SCSI CD/DVD drive connected to
> >                     mptspi driver
> >     Kernel Version: 2.6.21
> >             Status: NEW
> >           Severity: high
> >              Owner: scsi_drivers-other@kernel-bugs.osdl.org
> >          Submitter: doug.chapman@hp.com
> > 
> > 
> > Most recent kernel where this bug did *NOT* occur:
> > 2.6.20
> > 
> > Distribution:
> > fedora 7 - test 4
> > seen with fedora kernel and stock upstream 2.6.21 kernel as well
> > 
> > Hardware Environment:
> > HP Integrity Superdome
> > NEC  DVD_RW ND-3540A
> > LSI Fusion MPT SPI SCSI adapter
> > 
> > also on:
> > HP Integrity rx8640
> > NEC DVD+RW ND-2100AD
> > 
> > 
> > Software Environment: fedora 7 test 4
> > 
> > Problem Description:
> > Reading CD/DVD is approx 150times slower than usual.  Simple test of:
> > time cat kernel-2.6.20-1.3088.fc7.ia64.rpm > /dev/null
> > 
> > took approx 10 minutes on 2.6.21, took 2-4 seconds on 2.6.20 (same system, same
> > disk, repeated multiple times to ensure it wasn't bad hardware)
> > 
> > 
> > Steps to reproduce:
> > mount a DVD
> > read a large file from the DVD
> > 
> > 
> > Appears to have been caused by this git commit:
> 
> Thanks heaps for doing the bisection - it really helps.
> 
> > commit 5a9c47b1344b514758d5d7f193c672850390cc36
> > Author: Eric Moore <eric.moore@lsi.com>
> > Date:   Mon Jan 29 09:43:17 2007 -0700
> > 
> >     [SCSI] fusion - move SPI API over to mptspi.c
> > 
> >     Move some functions that only apply to the mptspi module over from mptscsih.
> >     Signed-off-by: Eric Moore <Eric.Moore@lsi.com>
> >     Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
> > 
> > 
> > I backed out this patch against the current HEAD and DVD drive performance is
> > back to normal.  I have not yet looked into this to determine exactly what in
> > this patch caused the issue.
> 
> That's a bit surprising - all the patch allegedly does is move stuff
> around.  Here's my version of a backout patch against 2.6.21 - can anyone
> spot any bloopers in it?
> 
> 
>  drivers/message/fusion/mptscsih.c |  284 ++++++++++++++++++++++++++++
>  drivers/message/fusion/mptspi.c   |  268 --------------------------
>  2 files changed, 285 insertions(+), 267 deletions(-)
> 
> diff -puN drivers/message/fusion/mptscsih.c~revert-fusion-move-spi-api-over-to-mptspic drivers/message/fusion/mptscsih.c
> --- a/drivers/message/fusion/mptscsih.c~revert-fusion-move-spi-api-over-to-mptspic
> +++ a/drivers/message/fusion/mptscsih.c
> @@ -98,6 +98,9 @@ static int	mptscsih_IssueTaskMgmt(MPT_SC
>  int		mptscsih_ioc_reset(MPT_ADAPTER *ioc, int post_reset);
>  int		mptscsih_event_process(MPT_ADAPTER *ioc, EventNotificationReply_t *pEvReply);
>  
> +static void	mptscsih_initTarget(MPT_SCSI_HOST *hd, VirtTarget *vtarget, struct scsi_device *sdev);
> +static void	mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtTarget *vtarget, struct scsi_device *sdev);
> +static int	mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id);
>  int		mptscsih_scandv_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *r);
>  static int	mptscsih_do_cmd(MPT_SCSI_HOST *hd, INTERNAL_CMD *iocmd);
>  static void	mptscsih_synchronize_cache(MPT_SCSI_HOST *hd, VirtDevice *vdevice);
> @@ -2410,6 +2413,7 @@ mptscsih_slave_configure(struct scsi_dev
>  	}
>  
>  	vdevice->configured_lun = 1;
> +	mptscsih_initTarget(hd, vtarget, sdev);
>  	mptscsih_change_queue_depth(sdev, MPT_SCSI_CMD_PER_DEV_HIGH);
>  
>  	dsprintk((MYIOC_s_INFO_FMT
> @@ -2673,6 +2677,286 @@ mptscsih_event_process(MPT_ADAPTER *ioc,
>  
>  /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
>  /*
> + *	mptscsih_initTarget - Target, LUN alloc/free functionality.
> + *	@hd: Pointer to MPT_SCSI_HOST structure
> + *	@vtarget: per target private data
> + *	@sdev: SCSI device
> + *
> + *	NOTE: It's only SAFE to call this routine if data points to
> + *	sane & valid STANDARD INQUIRY data!
> + *
> + *	Allocate and initialize memory for this target.
> + *	Save inquiry data.
> + *
> + */
> +static void
> +mptscsih_initTarget(MPT_SCSI_HOST *hd, VirtTarget *vtarget,
> +		    struct scsi_device *sdev)
> +{
> +	dinitprintk((MYIOC_s_INFO_FMT "initTarget channel=%d id=%d lun=%d hd=%p\n",
> +		hd->ioc->name, vtarget->channel, vtarget->id,
> +		sdev->lun, hd));
> +
> +	/* Is LUN supported? If so, upper 2 bits will be 0
> +	* in first byte of inquiry data.
> +	*/
> +	if (sdev->inq_periph_qual != 0)
> +		return;
> +
> +	if (vtarget == NULL)
> +		return;
> +
> +	vtarget->type = sdev->type;
> +
> +	if (hd->ioc->bus_type != SPI)
> +		return;
> +
> +	if ((sdev->type == TYPE_PROCESSOR) && (hd->ioc->spi_data.Saf_Te)) {
> +		/* Treat all Processors as SAF-TE if
> +		 * command line option is set */
> +		vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
> +		mptscsih_writeIOCPage4(hd, vtarget->channel, vtarget->id);
> +	}else if ((sdev->type == TYPE_PROCESSOR) &&
> +		!(vtarget->tflags & MPT_TARGET_FLAGS_SAF_TE_ISSUED )) {
> +		if (sdev->inquiry_len > 49 ) {
> +			if (sdev->inquiry[44] == 'S' &&
> +			    sdev->inquiry[45] == 'A' &&
> +			    sdev->inquiry[46] == 'F' &&
> +			    sdev->inquiry[47] == '-' &&
> +			    sdev->inquiry[48] == 'T' &&
> +			    sdev->inquiry[49] == 'E' ) {
> +				vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
> +				mptscsih_writeIOCPage4(hd, vtarget->channel, vtarget->id);
> +			}
> +		}
> +	}
> +	mptscsih_setTargetNegoParms(hd, vtarget, sdev);
> +}
> +
> +/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> +/*
> + *  Update the target negotiation parameters based on the
> + *  the Inquiry data, adapter capabilities, and NVRAM settings.
> + *
> + */
> +static void
> +mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtTarget *target,
> +			    struct scsi_device *sdev)
> +{
> +	SpiCfgData *pspi_data = &hd->ioc->spi_data;
> +	int  id = (int) target->id;
> +	int  nvram;
> +	u8 width = MPT_NARROW;
> +	u8 factor = MPT_ASYNC;
> +	u8 offset = 0;
> +	u8 nfactor;
> +	u8 noQas = 1;
> +
> +	target->negoFlags = pspi_data->noQas;
> +
> +	/* noQas == 0 => device supports QAS. */
> +
> +	if (sdev->scsi_level < SCSI_2) {
> +		width = 0;
> +		factor = MPT_ULTRA2;
> +		offset = pspi_data->maxSyncOffset;
> +		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
> +	} else {
> +		if (scsi_device_wide(sdev)) {
> +			width = 1;
> +		}
> +
> +		if (scsi_device_sync(sdev)) {
> +			factor = pspi_data->minSyncFactor;
> +			if (!scsi_device_dt(sdev))
> +					factor = MPT_ULTRA2;
> +			else {
> +				if (!scsi_device_ius(sdev) &&
> +				    !scsi_device_qas(sdev))
> +					factor = MPT_ULTRA160;
> +				else {
> +					factor = MPT_ULTRA320;
> +					if (scsi_device_qas(sdev)) {
> +						ddvtprintk((KERN_INFO "Enabling QAS due to byte56=%02x on id=%d!\n", scsi_device_qas(sdev), id));
> +						noQas = 0;
> +					}
> +					if (sdev->type == TYPE_TAPE &&
> +					    scsi_device_ius(sdev))
> +						target->negoFlags |= MPT_TAPE_NEGO_IDP;
> +				}
> +			}
> +			offset = pspi_data->maxSyncOffset;
> +
> +			/* If RAID, never disable QAS
> +			 * else if non RAID, do not disable
> +			 *   QAS if bit 1 is set
> +			 * bit 1 QAS support, non-raid only
> +			 * bit 0 IU support
> +			 */
> +			if (target->raidVolume == 1) {
> +				noQas = 0;
> +			}
> +		} else {
> +			factor = MPT_ASYNC;
> +			offset = 0;
> +		}
> +	}
> +
> +	if (!sdev->tagged_supported) {
> +		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
> +	}
> +
> +	/* Update tflags based on NVRAM settings. (SCSI only)
> +	 */
> +	if (pspi_data->nvram && (pspi_data->nvram[id] != MPT_HOST_NVRAM_INVALID)) {
> +		nvram = pspi_data->nvram[id];
> +		nfactor = (nvram & MPT_NVRAM_SYNC_MASK) >> 8;
> +
> +		if (width)
> +			width = nvram & MPT_NVRAM_WIDE_DISABLE ? 0 : 1;
> +
> +		if (offset > 0) {
> +			/* Ensure factor is set to the
> +			 * maximum of: adapter, nvram, inquiry
> +			 */
> +			if (nfactor) {
> +				if (nfactor < pspi_data->minSyncFactor )
> +					nfactor = pspi_data->minSyncFactor;
> +
> +				factor = max(factor, nfactor);
> +				if (factor == MPT_ASYNC)
> +					offset = 0;
> +			} else {
> +				offset = 0;
> +				factor = MPT_ASYNC;
> +		}
> +		} else {
> +			factor = MPT_ASYNC;
> +		}
> +	}
> +
> +	/* Make sure data is consistent
> +	 */
> +	if ((!width) && (factor < MPT_ULTRA2)) {
> +		factor = MPT_ULTRA2;
> +	}
> +
> +	/* Save the data to the target structure.
> +	 */
> +	target->minSyncFactor = factor;
> +	target->maxOffset = offset;
> +	target->maxWidth = width;
> +
> +	target->tflags |= MPT_TARGET_FLAGS_VALID_NEGO;
> +
> +	/* Disable unused features.
> +	 */
> +	if (!width)
> +		target->negoFlags |= MPT_TARGET_NO_NEGO_WIDE;
> +
> +	if (!offset)
> +		target->negoFlags |= MPT_TARGET_NO_NEGO_SYNC;
> +
> +	if ( factor > MPT_ULTRA320 )
> +		noQas = 0;
> +
> +	if (noQas && (pspi_data->noQas == 0)) {
> +		pspi_data->noQas |= MPT_TARGET_NO_NEGO_QAS;
> +		target->negoFlags |= MPT_TARGET_NO_NEGO_QAS;
> +
> +		/* Disable QAS in a mixed configuration case
> +		 */
> +
> +		ddvtprintk((KERN_INFO "Disabling QAS due to noQas=%02x on id=%d!\n", noQas, id));
> +	}
> +}
> +
> +/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> +
> +/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> +/*
> + *  SCSI Config Page functionality ...
> + */
> +
> +/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> +/*	mptscsih_writeIOCPage4  - write IOC Page 4
> + *	@hd: Pointer to a SCSI Host Structure
> + *	@channel: write IOC Page4 for this Bus
> + *	@id: write IOC Page4 for this ID
> + *
> + *	Return: -EAGAIN if unable to obtain a Message Frame
> + *		or 0 if success.
> + *
> + *	Remark: We do not wait for a return, write pages sequentially.
> + */
> +static int
> +mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id)
> +{
> +	MPT_ADAPTER		*ioc = hd->ioc;
> +	Config_t		*pReq;
> +	IOCPage4_t		*IOCPage4Ptr;
> +	MPT_FRAME_HDR		*mf;
> +	dma_addr_t		 dataDma;
> +	u16			 req_idx;
> +	u32			 frameOffset;
> +	u32			 flagsLength;
> +	int			 ii;
> +
> +	/* Get a MF for this command.
> +	 */
> +	if ((mf = mpt_get_msg_frame(ioc->DoneCtx, ioc)) == NULL) {
> +		dfailprintk((MYIOC_s_WARN_FMT "writeIOCPage4 : no msg frames!\n",
> +					ioc->name));
> +		return -EAGAIN;
> +	}
> +
> +	/* Set the request and the data pointers.
> +	 * Place data at end of MF.
> +	 */
> +	pReq = (Config_t *)mf;
> +
> +	req_idx = le16_to_cpu(mf->u.frame.hwhdr.msgctxu.fld.req_idx);
> +	frameOffset = ioc->req_sz - sizeof(IOCPage4_t);
> +
> +	/* Complete the request frame (same for all requests).
> +	 */
> +	pReq->Action = MPI_CONFIG_ACTION_PAGE_WRITE_CURRENT;
> +	pReq->Reserved = 0;
> +	pReq->ChainOffset = 0;
> +	pReq->Function = MPI_FUNCTION_CONFIG;
> +	pReq->ExtPageLength = 0;
> +	pReq->ExtPageType = 0;
> +	pReq->MsgFlags = 0;
> +	for (ii=0; ii < 8; ii++) {
> +		pReq->Reserved2[ii] = 0;
> +	}
> +
> +	IOCPage4Ptr = ioc->spi_data.pIocPg4;
> +	dataDma = ioc->spi_data.IocPg4_dma;
> +	ii = IOCPage4Ptr->ActiveSEP++;
> +	IOCPage4Ptr->SEP[ii].SEPTargetID = id;
> +	IOCPage4Ptr->SEP[ii].SEPBus = channel;
> +	pReq->Header = IOCPage4Ptr->Header;
> +	pReq->PageAddress = cpu_to_le32(id | (channel << 8 ));
> +
> +	/* Add a SGE to the config request.
> +	 */
> +	flagsLength = MPT_SGE_FLAGS_SSIMPLE_WRITE |
> +		(IOCPage4Ptr->Header.PageLength + ii) * 4;
> +
> +	mpt_add_sge((char *)&pReq->PageBufferSGE, flagsLength, dataDma);
> +
> +	dinitprintk((MYIOC_s_INFO_FMT
> +		"writeIOCPage4: MaxSEP=%d ActiveSEP=%d channel=%d id=%d \n",
> +			ioc->name, IOCPage4Ptr->MaxSEP, IOCPage4Ptr->ActiveSEP, channel, id));
> +
> +	mpt_put_msg_frame(ioc->DoneCtx, ioc, mf);
> +
> +	return 0;
> +}
> +
> +/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
> +/*
>   *  Bus Scan and Domain Validation functionality ...
>   */
>  
> diff -puN drivers/message/fusion/mptspi.c~revert-fusion-move-spi-api-over-to-mptspic drivers/message/fusion/mptspi.c
> --- a/drivers/message/fusion/mptspi.c~revert-fusion-move-spi-api-over-to-mptspic
> +++ a/drivers/message/fusion/mptspi.c
> @@ -95,269 +95,6 @@ static int	mptspiDoneCtx = -1;
>  static int	mptspiTaskCtx = -1;
>  static int	mptspiInternalCtx = -1; /* Used only for internal commands */
>  
> -/**
> - * 	mptspi_setTargetNegoParms  - Update the target negotiation
> - *	parameters based on the the Inquiry data, adapter capabilities,
> - *	and NVRAM settings
> - *
> - *	@hd: Pointer to a SCSI Host Structure
> - *	@vtarget: per target private data
> - *	@sdev: SCSI device
> - *
> - **/
> -static void
> -mptspi_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtTarget *target,
> -			    struct scsi_device *sdev)
> -{
> -	SpiCfgData *pspi_data = &hd->ioc->spi_data;
> -	int  id = (int) target->id;
> -	int  nvram;
> -	u8 width = MPT_NARROW;
> -	u8 factor = MPT_ASYNC;
> -	u8 offset = 0;
> -	u8 nfactor;
> -	u8 noQas = 1;
> -
> -	target->negoFlags = pspi_data->noQas;
> -
> -	if (sdev->scsi_level < SCSI_2) {
> -		width = 0;
> -		factor = MPT_ULTRA2;
> -		offset = pspi_data->maxSyncOffset;
> -		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
> -	} else {
> -		if (scsi_device_wide(sdev))
> -			width = 1;
> -
> -		if (scsi_device_sync(sdev)) {
> -			factor = pspi_data->minSyncFactor;
> -			if (!scsi_device_dt(sdev))
> -					factor = MPT_ULTRA2;
> -			else {
> -				if (!scsi_device_ius(sdev) &&
> -				    !scsi_device_qas(sdev))
> -					factor = MPT_ULTRA160;
> -				else {
> -					factor = MPT_ULTRA320;
> -					if (scsi_device_qas(sdev)) {
> -						ddvprintk((KERN_INFO "Enabling QAS due to byte56=%02x on id=%d!\n", scsi_device_qas(sdev), id));
> -						noQas = 0;
> -					}
> -					if (sdev->type == TYPE_TAPE &&
> -					    scsi_device_ius(sdev))
> -						target->negoFlags |= MPT_TAPE_NEGO_IDP;
> -				}
> -			}
> -			offset = pspi_data->maxSyncOffset;
> -
> -			/* If RAID, never disable QAS
> -			 * else if non RAID, do not disable
> -			 *   QAS if bit 1 is set
> -			 * bit 1 QAS support, non-raid only
> -			 * bit 0 IU support
> -			 */
> -			if (target->raidVolume == 1)
> -				noQas = 0;
> -		} else {
> -			factor = MPT_ASYNC;
> -			offset = 0;
> -		}
> -	}
> -
> -	if (!sdev->tagged_supported)
> -		target->tflags &= ~MPT_TARGET_FLAGS_Q_YES;
> -
> -	/* Update tflags based on NVRAM settings. (SCSI only)
> -	 */
> -	if (pspi_data->nvram && (pspi_data->nvram[id] != MPT_HOST_NVRAM_INVALID)) {
> -		nvram = pspi_data->nvram[id];
> -		nfactor = (nvram & MPT_NVRAM_SYNC_MASK) >> 8;
> -
> -		if (width)
> -			width = nvram & MPT_NVRAM_WIDE_DISABLE ? 0 : 1;
> -
> -		if (offset > 0) {
> -			/* Ensure factor is set to the
> -			 * maximum of: adapter, nvram, inquiry
> -			 */
> -			if (nfactor) {
> -				if (nfactor < pspi_data->minSyncFactor )
> -					nfactor = pspi_data->minSyncFactor;
> -
> -				factor = max(factor, nfactor);
> -				if (factor == MPT_ASYNC)
> -					offset = 0;
> -			} else {
> -				offset = 0;
> -				factor = MPT_ASYNC;
> -		}
> -		} else {
> -			factor = MPT_ASYNC;
> -		}
> -	}
> -
> -	/* Make sure data is consistent
> -	 */
> -	if ((!width) && (factor < MPT_ULTRA2))
> -		factor = MPT_ULTRA2;
> -
> -	/* Save the data to the target structure.
> -	 */
> -	target->minSyncFactor = factor;
> -	target->maxOffset = offset;
> -	target->maxWidth = width;
> -
> -	target->tflags |= MPT_TARGET_FLAGS_VALID_NEGO;
> -
> -	/* Disable unused features.
> -	 */
> -	if (!width)
> -		target->negoFlags |= MPT_TARGET_NO_NEGO_WIDE;
> -
> -	if (!offset)
> -		target->negoFlags |= MPT_TARGET_NO_NEGO_SYNC;
> -
> -	if ( factor > MPT_ULTRA320 )
> -		noQas = 0;
> -
> -	if (noQas && (pspi_data->noQas == 0)) {
> -		pspi_data->noQas |= MPT_TARGET_NO_NEGO_QAS;
> -		target->negoFlags |= MPT_TARGET_NO_NEGO_QAS;
> -
> -		/* Disable QAS in a mixed configuration case
> -		 */
> -
> -		ddvprintk((KERN_INFO "Disabling QAS due to noQas=%02x on id=%d!\n", noQas, id));
> -	}
> -}
> -
> -/**
> - * 	mptspi_writeIOCPage4  - write IOC Page 4
> - *	@hd: Pointer to a SCSI Host Structure
> - *	@channel:
> - *	@id: write IOC Page4 for this ID & Bus
> - *
> - *	Return: -EAGAIN if unable to obtain a Message Frame
> - *		or 0 if success.
> - *
> - *	Remark: We do not wait for a return, write pages sequentially.
> - **/
> -static int
> -mptspi_writeIOCPage4(MPT_SCSI_HOST *hd, u8 channel , u8 id)
> -{
> -	MPT_ADAPTER		*ioc = hd->ioc;
> -	Config_t		*pReq;
> -	IOCPage4_t		*IOCPage4Ptr;
> -	MPT_FRAME_HDR		*mf;
> -	dma_addr_t		 dataDma;
> -	u16			 req_idx;
> -	u32			 frameOffset;
> -	u32			 flagsLength;
> -	int			 ii;
> -
> -	/* Get a MF for this command.
> -	 */
> -	if ((mf = mpt_get_msg_frame(ioc->DoneCtx, ioc)) == NULL) {
> -		dfailprintk((MYIOC_s_WARN_FMT "writeIOCPage4 : no msg frames!\n",
> -					ioc->name));
> -		return -EAGAIN;
> -	}
> -
> -	/* Set the request and the data pointers.
> -	 * Place data at end of MF.
> -	 */
> -	pReq = (Config_t *)mf;
> -
> -	req_idx = le16_to_cpu(mf->u.frame.hwhdr.msgctxu.fld.req_idx);
> -	frameOffset = ioc->req_sz - sizeof(IOCPage4_t);
> -
> -	/* Complete the request frame (same for all requests).
> -	 */
> -	pReq->Action = MPI_CONFIG_ACTION_PAGE_WRITE_CURRENT;
> -	pReq->Reserved = 0;
> -	pReq->ChainOffset = 0;
> -	pReq->Function = MPI_FUNCTION_CONFIG;
> -	pReq->ExtPageLength = 0;
> -	pReq->ExtPageType = 0;
> -	pReq->MsgFlags = 0;
> -	for (ii=0; ii < 8; ii++) {
> -		pReq->Reserved2[ii] = 0;
> -	}
> -
> -	IOCPage4Ptr = ioc->spi_data.pIocPg4;
> -	dataDma = ioc->spi_data.IocPg4_dma;
> -	ii = IOCPage4Ptr->ActiveSEP++;
> -	IOCPage4Ptr->SEP[ii].SEPTargetID = id;
> -	IOCPage4Ptr->SEP[ii].SEPBus = channel;
> -	pReq->Header = IOCPage4Ptr->Header;
> -	pReq->PageAddress = cpu_to_le32(id | (channel << 8 ));
> -
> -	/* Add a SGE to the config request.
> -	 */
> -	flagsLength = MPT_SGE_FLAGS_SSIMPLE_WRITE |
> -		(IOCPage4Ptr->Header.PageLength + ii) * 4;
> -
> -	mpt_add_sge((char *)&pReq->PageBufferSGE, flagsLength, dataDma);
> -
> -	ddvprintk((MYIOC_s_INFO_FMT
> -		"writeIOCPage4: MaxSEP=%d ActiveSEP=%d id=%d bus=%d\n",
> -			ioc->name, IOCPage4Ptr->MaxSEP, IOCPage4Ptr->ActiveSEP, id, channel));
> -
> -	mpt_put_msg_frame(ioc->DoneCtx, ioc, mf);
> -
> -	return 0;
> -}
> -
> -/**
> - *	mptspi_initTarget - Target, LUN alloc/free functionality.
> - *	@hd: Pointer to MPT_SCSI_HOST structure
> - *	@vtarget: per target private data
> - *	@sdev: SCSI device
> - *
> - *	NOTE: It's only SAFE to call this routine if data points to
> - *	sane & valid STANDARD INQUIRY data!
> - *
> - *	Allocate and initialize memory for this target.
> - *	Save inquiry data.
> - *
> - **/
> -static void
> -mptspi_initTarget(MPT_SCSI_HOST *hd, VirtTarget *vtarget,
> -		    struct scsi_device *sdev)
> -{
> -
> -	/* Is LUN supported? If so, upper 2 bits will be 0
> -	* in first byte of inquiry data.
> -	*/
> -	if (sdev->inq_periph_qual != 0)
> -		return;
> -
> -	if (vtarget == NULL)
> -		return;
> -
> -	vtarget->type = sdev->type;
> -
> -	if ((sdev->type == TYPE_PROCESSOR) && (hd->ioc->spi_data.Saf_Te)) {
> -		/* Treat all Processors as SAF-TE if
> -		 * command line option is set */
> -		vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
> -		mptspi_writeIOCPage4(hd, vtarget->channel, vtarget->id);
> -	}else if ((sdev->type == TYPE_PROCESSOR) &&
> -		!(vtarget->tflags & MPT_TARGET_FLAGS_SAF_TE_ISSUED )) {
> -		if (sdev->inquiry_len > 49 ) {
> -			if (sdev->inquiry[44] == 'S' &&
> -			    sdev->inquiry[45] == 'A' &&
> -			    sdev->inquiry[46] == 'F' &&
> -			    sdev->inquiry[47] == '-' &&
> -			    sdev->inquiry[48] == 'T' &&
> -			    sdev->inquiry[49] == 'E' ) {
> -				vtarget->tflags |= MPT_TARGET_FLAGS_SAF_TE_ISSUED;
> -				mptspi_writeIOCPage4(hd, vtarget->channel, vtarget->id);
> -			}
> -		}
> -	}
> -	mptspi_setTargetNegoParms(hd, vtarget, sdev);
> -}
>  
>  /**
>   *	mptspi_is_raid - Determines whether target is belonging to volume
> @@ -723,16 +460,13 @@ static int mptspi_slave_alloc(struct scs
>  
>  static int mptspi_slave_configure(struct scsi_device *sdev)
>  {
> +	int ret = mptscsih_slave_configure(sdev);
>  	struct _MPT_SCSI_HOST *hd =
>  		(struct _MPT_SCSI_HOST *)sdev->host->hostdata;
> -	VirtTarget *vtarget = scsi_target(sdev)->hostdata;
> -	int ret = mptscsih_slave_configure(sdev);
>  
>  	if (ret)
>  		return ret;
>  
> -	mptspi_initTarget(hd, vtarget, sdev);
> -
>  	ddvprintk((MYIOC_s_INFO_FMT "id=%d min_period=0x%02x"
>  		" max_offset=0x%02x max_width=%d\n", hd->ioc->name,
>  		sdev->id, spi_min_period(scsi_target(sdev)),
> _
> 

ACK, tested this on my system where I originally found the problem and
all is well with this.

Ignore my earlier comment about the original patch adding the new
function mptspi_initTarget.  After looking at what is going on I realize
that it didn't add this, it was just renamed from mptscsih_initTarget.

- Doug

RE: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVDdrive connected to mptspi driver

[Moore, Eric]

On Thursday, May 03, 2007 9:50 PM,  Doug Chapman wrote:
> 
> ACK, tested this on my system where I originally found the problem and
> all is well with this.
> 
> Ignore my earlier comment about the original patch adding the new
> function mptspi_initTarget.  After looking at what is going 
> on I realize
> that it didn't add this, it was just renamed from mptscsih_initTarget.
> 

Are you still having issues?   I'm not clear with the above ACK email.

AFAIK, that patch your refering to which I submitted is only moving
code, not actually changing any functionality.   If your having a
problem with speed, then its most likely a domain validation problem.
In this driver, the domain validation is done from the spi transport
layer.    When you load the driver, there should be some messages
displayed along with the inquiry info during device scan, that would
provide the negotiation rates. Search your /var/log/messages or dmesg.
You can also look in the SysFS, and all the info is there as well.   If
your device is host_W:Channel_X:Target_Y:Lun_Z, then you would look in
/sys/class/spi_transport_targetW:X:Y:Z/ , in this folder will be period.
The period is found below at the end of the each line in nano seconds
units.

        factor:0x08   Ultra320 (160 Mega-transfers / second) (6.25 ns)
        factor:0x09   Ultra160 ( 80 Mega-transfers / second) (12.5 ns)
        factor:0x0A   Ultra2   ( 40 Mega-transfers / second) (25 ns)
        factor:0x0B   Ultra2   ( 40 Mega-transfers / second) (30.3 ns)
        factor:0x0C   Ultra    ( 20 Mega-transfers / second) (50 ns)
        factor:0x19   FAST     ( 10 Mega-transfers / second)
        factor:0x32   SCSI     (  5 Mega-transfers / second)
        factor:0xFF   5 Mega-trasfers/second and asynchronous


Also, in the mpt fusion, I have some debug you could enable, which will
dump all the negotiation parameters as they are written and read from
via the driver.  The spi transport layer calls these entry points when
it wants to change the negotiation parameter for each test it runs.  In
the mpt fusion driver Makefile, you need to uncomment the line
MPT_DEBUG_DV.   When you do that, then mptspi_print_read_nego and
mptspi_print_write_nego would be called.

I would like to point out that around the same time I supplied that mpt
fusion patch, there were changes in scsi_transport_spi.c, that would
effect negotitaion with regards to the starting min sync rate value.
This file is in /usr/src/linux/drivers/scsi.  You could diff between
your kernels to see the changes.

Eric Moore 
LSI

RE: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVDdrive connected to mptspi driver

[Doug Chapman]

On Fri, 2007-05-04 at 14:34 -0600, Moore, Eric wrote:
> On Thursday, May 03, 2007 9:50 PM,  Doug Chapman wrote:
> > 
> > ACK, tested this on my system where I originally found the problem and
> > all is well with this.
> > 
> > Ignore my earlier comment about the original patch adding the new
> > function mptspi_initTarget.  After looking at what is going 
> > on I realize
> > that it didn't add this, it was just renamed from mptscsih_initTarget.
> > 
> 
> Are you still having issues?   I'm not clear with the above ACK email.

I was ACKing Andrew's patch as it fixes the issue for me.  Without the
backup patch it is still broken even in the latest git tree.  (Linus's
tree).

> 
> AFAIK, that patch your refering to which I submitted is only moving
> code, not actually changing any functionality.   If your having a
> problem with speed, then its most likely a domain validation problem.

I agree it looks that way.  In fact it took me longer to narrow this
down because I didn't suspect that patch.  But, I tested this multiple
times backing out just that specific patch and it _does_ make the
difference.  It is rather dramatic, takes about 10 minutes to read a
kernel.rpm file from a DVD (takes 2 to 4 seconds normally).

> In this driver, the domain validation is done from the spi transport
> layer.    When you load the driver, there should be some messages
> displayed along with the inquiry info during device scan, that would
> provide the negotiation rates. Search your /var/log/messages or dmesg.
> You can also look in the SysFS, and all the info is there as well.   If
> your device is host_W:Channel_X:Target_Y:Lun_Z, then you would look in
> /sys/class/spi_transport_targetW:X:Y:Z/ , in this folder will be period.
> The period is found below at the end of the each line in nano seconds
> units.
> 
>         factor:0x08   Ultra320 (160 Mega-transfers / second) (6.25 ns)
>         factor:0x09   Ultra160 ( 80 Mega-transfers / second) (12.5 ns)
>         factor:0x0A   Ultra2   ( 40 Mega-transfers / second) (25 ns)
>         factor:0x0B   Ultra2   ( 40 Mega-transfers / second) (30.3 ns)
>         factor:0x0C   Ultra    ( 20 Mega-transfers / second) (50 ns)
>         factor:0x19   FAST     ( 10 Mega-transfers / second)
>         factor:0x32   SCSI     (  5 Mega-transfers / second)
>         factor:0xFF   5 Mega-trasfers/second and asynchronous
> 

/sys/class/spi_transport/target5:0:2/period is 50 with our without the
patch in question.

> 
> Also, in the mpt fusion, I have some debug you could enable, which will
> dump all the negotiation parameters as they are written and read from
> via the driver.  The spi transport layer calls these entry points when
> it wants to change the negotiation parameter for each test it runs.  In
> the mpt fusion driver Makefile, you need to uncomment the line
> MPT_DEBUG_DV.   When you do that, then mptspi_print_read_nego and
> mptspi_print_write_nego would be called.

Perhaps I can look into this monday.

> 
> I would like to point out that around the same time I supplied that mpt
> fusion patch, there were changes in scsi_transport_spi.c, that would
> effect negotitaion with regards to the starting min sync rate value.
> This file is in /usr/src/linux/drivers/scsi.  You could diff between
> your kernels to see the changes.

I am applying/removing _only_ your patch and the problem goes away with
just removing it.

- Doug

RE: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVDdrive connected to mptspi driver

[James Bottomley]

On Fri, 2007-05-04 at 17:56 -0400, Doug Chapman wrote:
> I am applying/removing _only_ your patch and the problem goes away with
> just removing it.

What id and channel is this DVD drive on?  There is a potential bug in
the move:

-mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id)
+mptspi_writeIOCPage4(MPT_SCSI_HOST *hd, u8 channel , u8 id)

Note int became u8, but later we have:


	pReq->PageAddress = cpu_to_le32(id | (channel << 8 ));

channel << 8 is always zero as a u8 entity (unless something promotes
the arithmetic beyond u8).  So if the DVD is on a non-zero channel, we
might have our cause.

James

RE: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVDdrive connected to mptspi driver

[Doug Chapman]

On Fri, 2007-05-04 at 17:58 -0500, James Bottomley wrote:
> On Fri, 2007-05-04 at 17:56 -0400, Doug Chapman wrote:
> > I am applying/removing _only_ your patch and the problem goes away with
> > just removing it.
> 
> What id and channel is this DVD drive on?  There is a potential bug in
> the move:
> 
> -mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id)
> +mptspi_writeIOCPage4(MPT_SCSI_HOST *hd, u8 channel , u8 id)
> 

I manually changed channel and id back to "int" and it does not seem to
have any affect.

It is looking like the issue is related to the order in how things are
called.  That is the only real change in functionality.  Perhaps there
is something that used to get initialized before mpt*_initTarget that is
now called after?  I don't see anything obvious but I am unfamiliar with
the code and have not had the time to study it all that much.

- Doug

RE: [Bugme-new] [Bug 8426] New: massive slowdown on SCSI CD/DVDdrive connected to mptspi driver

[Doug Chapman]

Found the problem.  Sending a patch to linux-scsi under a [PATCH]
subject line.

- Doug

On Fri, 2007-05-04 at 17:58 -0500, James Bottomley wrote:
> On Fri, 2007-05-04 at 17:56 -0400, Doug Chapman wrote:
> > I am applying/removing _only_ your patch and the problem goes away with
> > just removing it.
> 
> What id and channel is this DVD drive on?  There is a potential bug in
> the move:
> 
> -mptscsih_writeIOCPage4(MPT_SCSI_HOST *hd, int channel, int id)
> +mptspi_writeIOCPage4(MPT_SCSI_HOST *hd, u8 channel , u8 id)
> 
> Note int became u8, but later we have:
> 
> 
> 	pReq->PageAddress = cpu_to_le32(id | (channel << 8 ));
> 
> channel << 8 is always zero as a u8 entity (unless something promotes
> the arithmetic beyond u8).  So if the DVD is on a non-zero channel, we
> might have our cause.
> 
> James
> 
>