linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [PATCH] fix dma mapping leak in fusion
@ 2004-08-30 22:29 Moore, Eric Dean
  2004-08-31 12:56 ` Luben Tuikov
  0 siblings, 1 reply; 18+ messages in thread
From: Moore, Eric Dean @ 2004-08-30 22:29 UTC (permalink / raw)
  To: Christoph Hellwig, Moore, Eric Dean; +Cc: Masao Fukuchi, linux-scsi

On Saturday, August 28, 2004 1:03 PM, Christoph Hellwig wrote:

> 
> On Mon, Aug 23, 2004 at 11:56:50AM -0400, Moore, Eric Dean wrote:
> > Christoph - What is the criteria for the mid-layer
> > to offline a device? 
> 
> When eh_abort didn't succeed for all outstanding commands and 
> the various
> reset methods failed aswell.

Look in mptscsih_abort.  If abort task management request failed,
it will immediately complete the command to mid-layer, returning
FAILED.  If we succeeded issuing the command, we return SUCCESS, and
the command would be completed later by mptscsih_taskmgmt_complete.
If mptscsih_taskmgmt_complete fails to be called, then timer would
expire, calling mptscsih_taskmgmt_timeout.  This will eventually endup
reseting card, and flusing the outstanding commands from 
mptscsih_flush_running_commands.

> 
> > The dmesg dump in previous email indicates that mid-layer 
> > issued several aborts to the LLD, then mpt driver is returning 
> > SUCCESS. However at some point the midlayer offlines the device,
> > however commands are still in the LLD, and completed 
> sometime later after
> > the mptscsih_taskmgmt_timeout is called, thus hitting the
> > oops(because request_buffer=NULL), as we have removed the 
> > scsi_device_online check per your request.
> 
> I'm looking over fusion code with your latest patch applied now.  What
> worries me is mptscsih_flush_running_cmds where's you're erroring out
> commands possibly from withing EH methods (?, I have a hard time
> following the code from mptscsih to mptbase and back), but not under
> EH control.
> 
> Btw, when looking over this it seems to me you could kill
> mptscsih_search_running_cmds - the scsi core makes sure you'll never
> have outstanding commands when it calls ->slave_destory.
> 

I don't recommend removing mptscsih_search_running_cmds. 
This is cleaning up the scsi-look table, chain buffer, and msg frames.

  

^ permalink raw reply	[flat|nested] 18+ messages in thread
* RE: [PATCH] fix dma mapping leak in fusion
@ 2004-08-31 14:42 Moore, Eric Dean
  0 siblings, 0 replies; 18+ messages in thread
From: Moore, Eric Dean @ 2004-08-31 14:42 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: Christoph Hellwig, Masao Fukuchi, linux-scsi

On Tuesday, August 31, 2004 6:56 AM, Luben Tuikov wrote
> 
> > Look in mptscsih_abort.  If abort task management request failed,
> > it will immediately complete the command to mid-layer, returning
> > FAILED.  If we succeeded issuing the command, we return SUCCESS, and
> > the command would be completed later by mptscsih_taskmgmt_complete.
> > If mptscsih_taskmgmt_complete fails to be called, then timer would
> > expire, calling mptscsih_taskmgmt_timeout.  This will 
> eventually endup
> > reseting card, and flusing the outstanding commands from
> > mptscsih_flush_running_commands.
> 
> It doesn't make much sense for ABORT TASK to return FAILED.  The
> reasons,
> 	"In either case, the SCSI target device shall guarantee
> 	that no further responses from the task are sent to the
> 	SCSI initiator port.", SAM3r13, 7.2.

For this represented path in the driver, we just tried
to issue the ABORT TASK, and for some reason the HBA failed to accept
the ABORT TASK,  thus the driver immediately completes the SCSI command,
and returns FAILED to mid layer.   I didn't think it would be appropiate
to tell the midlayer we succeeded in issuing the abort, when it actually
failed.

> 
> That is, unless the delivery subsystem wants to let know the
> application client about the _delivery_ status of the TMF,
> we're better off returning SUCCESS.
> 
> So, in either case, we "_kill_" the task (slot) in the driver
> queue.  If the service response did fail and we couldn't send
> the TMF, and the aborted task did return status, we'd not
> find it in our queues and thus know that it is/was a cancelled
> task and therefore _not_ report status to SCSI Core,
> which is _not_ expecting status for that command.  This is
> a more graceful solution, rather than resetting the host,
> when ABORT TASK delivery fails, and thus zapping other
> initiator ports' tasks, etc.
> 

The reset is issued only after the timeout of the TM timer has expired.
The reset is to get the HBA back into known state, and terminate
outstanding commands.  It wouldn't be good idea to start releasing dma to 
commands when we don't know what state they are at down in the firmware.


> Either way, I cannot imagine _recovering_ TMFs.
> 
> BTW, it is my conviction that _most_ (and maybe all) TMFs should
> be HOQ attribute tasks.  (This is one "feature" which would
> distinguish between different, say, iSCSI vendors, for example.
> "Did it hang?" "No, it recovered quickly.", etc.)
> 
> 		Luben
>  
> >  >
> >  > > The dmesg dump in previous email indicates that mid-layer
> >  > > issued several aborts to the LLD, then mpt driver is returning
> >  > > SUCCESS. However at some point the midlayer offlines 
> the device,
> >  > > however commands are still in the LLD, and completed
> >  > sometime later after
> >  > > the mptscsih_taskmgmt_timeout is called, thus hitting the
> >  > > oops(because request_buffer=NULL), as we have removed the
> >  > > scsi_device_online check per your request.
> >  >
> >  > I'm looking over fusion code with your latest patch 
> applied now.  What
> >  > worries me is mptscsih_flush_running_cmds where's you're 
> erroring out
> >  > commands possibly from withing EH methods (?, I have a hard time
> >  > following the code from mptscsih to mptbase and back), 
> but not under
> >  > EH control.
> >  >
> >  > Btw, when looking over this it seems to me you could kill
> >  > mptscsih_search_running_cmds - the scsi core makes sure 
> you'll never
> >  > have outstanding commands when it calls ->slave_destory.
> >  >
> > 
> > I don't recommend removing mptscsih_search_running_cmds.
> > This is cleaning up the scsi-look table, chain buffer, and 
> msg frames.
> > 
> >  
> > -
> > To unsubscribe from this list: send the line "unsubscribe 
> linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* RE: [PATCH] fix dma mapping leak in fusion
@ 2004-08-23 15:56 Moore, Eric Dean
  2004-08-28 19:02 ` Christoph Hellwig
  0 siblings, 1 reply; 18+ messages in thread
From: Moore, Eric Dean @ 2004-08-23 15:56 UTC (permalink / raw)
  To: Christoph Hellwig, Masao Fukuchi; +Cc: linux-scsi

[-- Attachment #1: Type: text/plain, Size: 5055 bytes --]

Christoph - What is the criteria for the mid-layer
to offline a device? 

The fix in the 3.01.16 patch(see attached) was to return SUCCESS 
in the eh_abort handler if the AbortTaskManagement
request is successfully issued, or immediately unmask dma if
this failed, returning FAILED to mid-layer.

The dmesg dump in previous email indicates that mid-layer 
issued several aborts to the LLD, then mpt driver is returning 
SUCCESS. However at some point the midlayer offlines the device,
however commands are still in the LLD, and completed sometime later after
the mptscsih_taskmgmt_timeout is called, thus hitting the
oops(because request_buffer=NULL), as we have removed the 
scsi_device_online check per your request.

Eric


On Sunday, August 22, 2004 7:42 PM, Masao Fukuchi wrote:
> 
> 
> Hi Eric,
> 
> I took a message according to your request.
> Please look at the message and comment me.
> 
> Masao Fukuchi
> 
> Moore, Eric Dean wrote:
> >Would it be possible for you to set up remote system 
> >to send kernel messages over serial cable?
> >Is so, pls recompile the driver with MPT_DEBUG_TM defined in
> >the mpt device driver Makefile; then send me log file.
> >
> >Thanks,
> >Eric
> >
> >On Thursday, August 19, 2004 9:05 PM, Masao Fukuchi wrote:
> >> 
> >> 
> >> Eric,
> >> 
> >> I applyed your patch and tested it, but the patch didn't work well.
> >> I didn't met Oops. 
> >> But after read command finishing with fail, keyboard and 
> mouse click 
> >> didn't work.
> >> And also I couldn't login from other server.
> >> 
> >> The message was almost same as Fusion MPT driver 3.01.15.
> >> 
> >> Masao Fukuchi
> >> 
> >> Moore, Eric Dean wrote:
> >> >Here is a patch to apply against 3.01.15. It will have a small
> >> >fix in mptscsih_abort which should work with Christoph's 
> suggestion
> >> >of removing the "is the device is offline" check in 
> >> >mptscsih_flush_running_cmds.  Pls test and let me know if 
> you still 
> >> >hit the oops.
> >> >
> >> >Eric
> >> >
> >> >
> >> >On Thursday, August 19, 2004 7:15 AM, Masao Fukuchi wrote:
> >> >> 
> >> >> I tried fusion MPT driver 3.01.15 and I didn't met Oops.
> >> >> (Eric fixed Oops problem by Fusion MPT driver 3.01.04)
> >> >> 
> >> >> Masao Fukuchi
> >> >> 
> >> >> Message:
> >> >>  18:19:01 kernel: mptscsih: ioc3: >> Attempting task abort! 
> >> >> (sc=e00000007c205080)
> >> >>  18:19:01 kernel: mptscsih: ioc3: >> Attempting target reset! 
> >> >> (sc=e00000007c205080)
> >> >>  18:19:01 kernel: mptscsih: ioc3: >> Attempting bus reset! 
> >> >> (sc=e00000007c205080)
> >> >>  18:19:03 kernel: mptbase: Initiating ioc3 recovery
> >> >>  18:19:28 kernel: mptscsih: ioc3: >> Attempting task abort! 
> >> >> (sc=e00000007c205080)
> >> >>  18:19:28 kernel: mptscsih: ioc3: >> Attempting host reset! 
> >> >> (sc=e00000007c205080)
> >> >>  18:19:28 kernel: mptbase: Initiating ioc3 recovery
> >> >>  18:19:52 kernel: mptscsih: ioc3: >> Attempting task abort! 
> >> >> (sc=e00000007c205080)
> >> >>  18:19:52 kernel: scsi: Device offlined - not ready after 
> >> >> error recovery: host 3 channel 0 id 1 lun 0
> >> >>  18:19:52 kernel: scsi3 (1:0): rejecting I/O to offline device
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 0
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 1
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 2
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 3
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 4
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 5
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 6
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 7
> >> >>  18:19:52 kernel: scsi3 (1:0): rejecting I/O to offline device
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 0
> >> >>  18:19:52 kernel: Buffer I/O error on device sdb, 
> logical block 1
> >> >>  18:19:54 kernel: mptbase: Initiating ioc3 recovery
> >> >> 
> >> >> 
> >> >> Christoph Hellwig wrote:
> >> >> >On Thu, Aug 19, 2004 at 12:01:11PM +0900, Masao Fukuchi wrote:
> >> >> >> Hi Christoph,
> >> >> >> 
> >> >> >> I applyed attached patch into latest fusion MPT 
> >> driver(3.01.15) and
> >> >> >> tested it, but I still met Oops.
> >> >> >> Then I also applyed your latest patch(it gets rid of the 
> >> >> fusion pendingQ
> >> >> >> in favour of using the scsi midlayer queuing), but I met 
> >> >> Oops again.
> >> >> >
> >> >> >Can you try the patch Eric posted instead?
> >> >> >
> >> >> >-
> >> >> >To unsubscribe from this list: send the line "unsubscribe 
> >> >> linux-scsi" in
> >> >> >the body of a message to majordomo@vger.kernel.org
> >> >> >More majordomo info at  
http://vger.kernel.org/majordomo-info.html
>> >> 
>> >
>> 
>-
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: mptlinux-3.01.16.patch --]
[-- Type: application/octet-stream, Size: 21876 bytes --]

diff -uarN b/message/fusion/mptbase.c a/message/fusion/mptbase.c
--- b/message/fusion/mptbase.c	2004-08-09 16:42:28.000000000 -0600
+++ a/message/fusion/mptbase.c	2004-08-18 10:59:06.000000000 -0600
@@ -3395,7 +3395,7 @@
  *	@hd: Pointer to MPT_SCSI_HOST structure
  *	@init: If set, initialize the spin lock.
  */
-int
+static int
 initChainBuffers(MPT_ADAPTER *ioc)
 {
 	u8		*mem;
diff -uarN b/message/fusion/mptbase.h a/message/fusion/mptbase.h
--- b/message/fusion/mptbase.h	2004-08-09 16:42:45.000000000 -0600
+++ a/message/fusion/mptbase.h	2004-08-18 11:20:23.000000000 -0600
@@ -83,8 +83,8 @@
 #define COPYRIGHT	"Copyright (c) 1999-2004 " MODULEAUTHOR
 #endif
 
-#define MPT_LINUX_VERSION_COMMON	"3.01.15"
-#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.01.15"
+#define MPT_LINUX_VERSION_COMMON	"3.01.16"
+#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.01.16"
 #define WHAT_MAGIC_STRING		"@" "(" "#" ")"
 
 #define show_mptmod_ver(s,ver)  \
@@ -318,17 +318,6 @@
 	struct _Q_ITEM	*tail;
 } Q_TRACKER;
 
-typedef struct _MPT_DONE_Q {
-	struct _MPT_DONE_Q	*forw;
-	struct _MPT_DONE_Q	*back;
-	void			*argp;
-} MPT_DONE_Q;
-
-typedef struct _DONE_Q_TRACKER {
-	MPT_DONE_Q	*head;
-	MPT_DONE_Q	*tail;
-} DONE_Q_TRACKER;
-
 /*
  *  Chip-specific stuff... FC929 delineates break between
  *  FC and Parallel SCSI parts. Do NOT re-order.
@@ -1028,12 +1017,7 @@
 		/* Pool of memory for holding SCpnts before doing
 		 * OS callbacks. freeQ is the free pool.
 		 */
-	u8			 *memQ;
-	DONE_Q_TRACKER		  freeQ;
-	DONE_Q_TRACKER		  doneQ;		/* Holds Linux formmatted requests */
-	DONE_Q_TRACKER		  pendingQ;		/* Holds MPI formmatted requests */
 	MPT_Q_TRACKER		  taskQ;		/* TM request Q */
-	spinlock_t		  freedoneQlock;
 	int			  taskQcnt;
 	u8			  tmPending;
 	u8			  resetPending;
diff -uarN b/message/fusion/mptctl.c a/message/fusion/mptctl.c
--- b/message/fusion/mptctl.c	2004-07-02 09:14:30.000000000 -0600
+++ a/message/fusion/mptctl.c	2004-08-18 10:59:54.000000000 -0600
@@ -2865,7 +2865,7 @@
 };
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
-int __init mptctl_init(void)
+static int __init mptctl_init(void)
 {
 	int err;
 	int where = 1;
@@ -2963,7 +2963,7 @@
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
-void mptctl_exit(void)
+static void mptctl_exit(void)
 {
 	misc_deregister(&mptctl_miscdev);
 	printk(KERN_INFO MYNAM ": Deregistered /dev/%s @ (major,minor=%d,%d)\n",
diff -uarN b/message/fusion/mptscsih.c a/message/fusion/mptscsih.c
--- b/message/fusion/mptscsih.c	2004-08-02 17:01:01.000000000 -0600
+++ a/message/fusion/mptscsih.c	2004-08-18 16:38:57.000000000 -0600
@@ -168,8 +168,6 @@
 static void	copy_sense_data(struct scsi_cmnd *sc, MPT_SCSI_HOST *hd, MPT_FRAME_HDR *mf, SCSIIOReply_t *pScsiReply);
 static int	mptscsih_tm_pending_wait(MPT_SCSI_HOST * hd);
 static u32	SCPNT_TO_LOOKUP_IDX(struct scsi_cmnd *sc);
-static MPT_FRAME_HDR *mptscsih_search_pendingQ(MPT_SCSI_HOST *hd, int scpnt_idx);
-static void	post_pendingQ_commands(MPT_SCSI_HOST *hd);
 
 static int	mptscsih_TMHandler(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 target, u8 lun, int ctx2abort, ulong timeout, int sleepFlag);
 static int	mptscsih_IssueTaskMgmt(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 target, u8 lun, int ctx2abort, ulong timeout, int sleepFlag);
@@ -178,7 +176,7 @@
 static int	mptscsih_event_process(MPT_ADAPTER *ioc, EventNotificationReply_t *pEvReply);
 
 static void	mptscsih_initTarget(MPT_SCSI_HOST *hd, int bus_id, int target_id, u8 lun, char *data, int dlen);
-void		mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56);
+static void	mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56);
 static void	mptscsih_set_dvflags(MPT_SCSI_HOST *hd, SCSIIORequest_t *pReq);
 static void	mptscsih_setDevicePage1Flags (u8 width, u8 factor, u8 offset, int *requestedPtr, int *configurationPtr, u8 flags);
 static void	mptscsih_no_negotiate(MPT_SCSI_HOST *hd, int target_id);
@@ -895,84 +893,6 @@
 	return 1;
 }
 
-/*
- * Flush all commands on the doneQ.
- * Lock Q when deleting/adding members
- * Lock io_request_lock for OS callback.
- */
-static void
-flush_doneQ(MPT_SCSI_HOST *hd)
-{
-	MPT_DONE_Q	*buffer;
-	struct scsi_cmnd	*SCpnt;
-	unsigned long	 flags;
-
-	/* Flush the doneQ.
-	 */
-	dtmprintk((KERN_INFO MYNAM ": flush_doneQ called\n"));
-	while (1) {
-		spin_lock_irqsave(&hd->freedoneQlock, flags);
-		if (Q_IS_EMPTY(&hd->doneQ)) {
-			spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			break;
-		}
-
-		buffer = hd->doneQ.head;
-		/* Delete from Q
-		 */
-		Q_DEL_ITEM(buffer);
-
-		/* Set the struct scsi_cmnd pointer
-		 */
-		SCpnt = (struct scsi_cmnd *) buffer->argp;
-		buffer->argp = NULL;
-
-		/* Add to the freeQ
-		 */
-		Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-
-		/* Do the OS callback.
-		 */
-		SCpnt->scsi_done(SCpnt);
-	}
-
-	return;
-}
-
-/*
- * Search the doneQ for a specific command. If found, delete from Q.
- * Calling function will finish processing.
- */
-static void
-search_doneQ_for_cmd(MPT_SCSI_HOST *hd, struct scsi_cmnd *SCpnt)
-{
-	unsigned long	 flags;
-	MPT_DONE_Q	*buffer;
-
-	spin_lock_irqsave(&hd->freedoneQlock, flags);
-	if (!Q_IS_EMPTY(&hd->doneQ)) {
-		buffer = hd->doneQ.head;
-		do {
-			struct scsi_cmnd *sc = (struct scsi_cmnd *) buffer->argp;
-			if (SCpnt == sc) {
-				Q_DEL_ITEM(buffer);
-				SCpnt->result = sc->result;
-
-				/* Set the struct scsi_cmnd pointer
-				 */
-				buffer->argp = NULL;
-
-				/* Add to the freeQ
-				 */
-				Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-				break;
-			}
-		} while ((buffer = buffer->forw) != (MPT_DONE_Q *) &hd->doneQ);
-	}
-	spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-	return;
-}
 
 /*
  *	mptscsih_flush_running_cmds - For each command found, search
@@ -990,10 +910,8 @@
 	MPT_ADAPTER *ioc = hd->ioc;
 	struct scsi_cmnd	*SCpnt;
 	MPT_FRAME_HDR	*mf;
-	MPT_DONE_Q	*buffer;
 	int		 ii;
 	int		 max = ioc->req_depth;
-	unsigned long	 flags;
 
 	dprintk((KERN_INFO MYNAM ": flush_ScsiLookup called\n"));
 	for (ii= 0; ii < max; ii++) {
@@ -1002,11 +920,6 @@
 			/* Command found.
 			 */
 
-			/* Search pendingQ, if found,
-			 * delete from Q.
-			 */
-			mptscsih_search_pendingQ(hd, ii);
-
 			/* Null ScsiLookup index
 			 */
 			hd->ScsiLookup[ii] = NULL;
@@ -1019,18 +932,16 @@
 			 * Do OS callback
 			 * Free driver resources (chain, msg buffers)
 			 */
-			if (scsi_device_online(SCpnt->device)) {
-				if (SCpnt->use_sg) {
-					pci_unmap_sg(ioc->pcidev,
-						(struct scatterlist *) SCpnt->request_buffer,
-						SCpnt->use_sg,
-						SCpnt->sc_data_direction);
-				} else if (SCpnt->request_bufflen) {
-					pci_unmap_single(ioc->pcidev,
-						SCpnt->SCp.dma_handle,
-						SCpnt->request_bufflen,
-						SCpnt->sc_data_direction);
-				}
+			if (SCpnt->use_sg) {
+				pci_unmap_sg(ioc->pcidev,
+					(struct scatterlist *) SCpnt->request_buffer,
+					SCpnt->use_sg,
+					SCpnt->sc_data_direction);
+			} else if (SCpnt->request_bufflen) {
+				pci_unmap_single(ioc->pcidev,
+					SCpnt->SCp.dma_handle,
+					SCpnt->request_bufflen,
+					SCpnt->sc_data_direction);
 			}
 			SCpnt->result = DID_RESET << 16;
 			SCpnt->host_scribble = NULL;
@@ -1041,32 +952,7 @@
 			/* Free Message frames */
 			mpt_free_msg_frame(ioc, mf);
 
-#if 1
-			/* Post to doneQ, do not reply until POST phase
-			 * of reset handler....prevents new commands from
-			 * being queued.
-			 */
-			spin_lock_irqsave(&hd->freedoneQlock, flags);
-			if (!Q_IS_EMPTY(&hd->freeQ)) {
-				buffer = hd->freeQ.head;
-				Q_DEL_ITEM(buffer);
-
-				/* Set the struct scsi_cmnd pointer
-				 */
-				buffer->argp = (void *)SCpnt;
-
-				/* Add to the doneQ
-				 */
-				Q_ADD_TAIL(&hd->doneQ.head, buffer, MPT_DONE_Q);
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			} else {
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-				SCpnt->scsi_done(SCpnt);
-			}
-#else
 			SCpnt->scsi_done(SCpnt);	/* Issue the command callback */
-#endif
-
 		}
 	}
 
@@ -1173,7 +1059,6 @@
 	struct Scsi_Host	*sh;
 	MPT_SCSI_HOST		*hd;
 	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
-	MPT_DONE_Q		*freedoneQ;
 	unsigned long		 flags;
 	int			 sz, ii;
 	int			 numSGE = 0;
@@ -1330,32 +1215,6 @@
 	dprintk((MYIOC_s_INFO_FMT "ScsiLookup @ %p, sz=%d\n",
 		 ioc->name, hd->ScsiLookup, sz));
 		
-	/* Allocate memory for free and doneQ's
-	 */
-	sz = sh->can_queue * sizeof(MPT_DONE_Q);
-	mem = kmalloc(sz, GFP_ATOMIC);
-	if (mem == NULL) {
-		error = -ENOMEM;
-		goto mptscsih_probe_failed;
-	}
-
-	memset(mem, 0xFF, sz);
-	hd->memQ = mem;
-
-	/* Initialize the free, done and pending Qs.
-	 */
-	Q_INIT(&hd->freeQ, MPT_DONE_Q);
-	Q_INIT(&hd->doneQ, MPT_DONE_Q);
-	Q_INIT(&hd->pendingQ, MPT_DONE_Q);
-	spin_lock_init(&hd->freedoneQlock);
-
-	mem = hd->memQ;
-	for (ii=0; ii < sh->can_queue; ii++) {
-		freedoneQ = (MPT_DONE_Q *) mem;
-		Q_ADD_TAIL(&hd->freeQ.head, freedoneQ, MPT_DONE_Q);
-		mem += sizeof(MPT_DONE_Q);
-	}
-
 	/* Initialize this Scsi_Host
 	 * internal task Q.
 	 */
@@ -1445,10 +1304,6 @@
 #ifndef MPTSCSIH_ENABLE_DOMAIN_VALIDATION
 		hd->negoNvram = MPT_SCSICFG_USE_NVRAM;
 #endif
-		if (driver_setup.dv == 0) {
-			hd->negoNvram = MPT_SCSICFG_USE_NVRAM;
-		}
-
 		ioc->spi_data.forceDv = 0;
 		for (ii=0; ii < MPT_MAX_SCSI_DEVICES; ii++) {
 			ioc->spi_data.dvStatus[ii] =
@@ -1531,7 +1386,6 @@
 	hd = (MPT_SCSI_HOST *)host->hostdata;
 	if (hd != NULL) {
 		int sz1, sz3, sztarget=0;
-		int szQ = 0;
 
 		mptscsih_shutdown(&pdev->dev);
 
@@ -1543,12 +1397,6 @@
 			hd->ScsiLookup = NULL;
 		}
 
-		if (hd->memQ != NULL) {
-			szQ = host->can_queue * sizeof(MPT_DONE_Q);
-			kfree(hd->memQ);
-			hd->memQ = NULL;
-		}
-
 		if (hd->Targets != NULL) {
 			int max, ii;
 
@@ -1760,7 +1608,7 @@
  *
  *	Returns pointer to buffer where information was written.
  */
-const char *
+static const char *
 mptscsih_info(struct Scsi_Host *SChost)
 {
 	MPT_SCSI_HOST *h;
@@ -1975,7 +1823,8 @@
  * 	hostno: scsi host number
  *	func:   if write = 1; if read = 0
  */
-int mptscsih_proc_info(struct Scsi_Host *host, char *buffer, char **start, off_t offset,
+static int 
+mptscsih_proc_info(struct Scsi_Host *host, char *buffer, char **start, off_t offset,
 			int length, int func)
 {
 	MPT_SCSI_HOST	*hd = (MPT_SCSI_HOST *)host->hostdata;
@@ -2010,15 +1859,13 @@
  *
  *	Returns 0. (rtn value discarded by linux scsi mid-layer)
  */
-int
+static int
 mptscsih_qcmd(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 {
 	MPT_SCSI_HOST		*hd;
 	MPT_FRAME_HDR		*mf;
 	SCSIIORequest_t		*pScsiReq;
 	VirtDevice		*pTarget;
-	MPT_DONE_Q		*buffer;
-	unsigned long		 flags;
 	int	 target;
 	int	 lun;
 	u32	 datalen;
@@ -2043,16 +1890,9 @@
 			(hd && hd->ioc) ? hd->ioc->name : "ioc?", SCpnt, done));
 
 	if (hd->resetPending) {
-		/* Prevent new commands from being issued
-		 * while reloading the FW. Reset timer to 60 seconds,
-		 * as the FW can take some time to come ready.
-		 * For New EH, cmds on doneQ posted to FW.
-		 */
-		did_errcode = 1;
-		mod_timer(&SCpnt->eh_timeout, jiffies + (HZ * 60));
 		dtmprintk((MYIOC_s_WARN_FMT "qcmd: SCpnt=%p timeout + 60HZ\n",
 			(hd && hd->ioc) ? hd->ioc->name : "ioc?", SCpnt));
-		goto did_error;
+		return SCSI_MLQUEUE_HOST_BUSY;
 	}
 
 	/*
@@ -2061,8 +1901,7 @@
 	if ((mf = mpt_get_msg_frame(ScsiDoneCtx, hd->ioc)) == NULL) {
 		dprintk((MYIOC_s_WARN_FMT "QueueCmd, no msg frames!!\n",
 				hd->ioc->name));
-		did_errcode = 2;
-		goto did_error;
+		return SCSI_MLQUEUE_HOST_BUSY;
 	}
 
 	pScsiReq = (SCSIIORequest_t *) mf;
@@ -2211,64 +2050,17 @@
 			dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p idx=%d\n",
 					hd->ioc->name, SCpnt, mf, my_idx));
 			DBG_DUMP_REQUEST_FRAME(mf)
-		} else {
-			ddvtprintk((MYIOC_s_INFO_FMT "Pending cmd=%p idx %d\n",
-					hd->ioc->name, SCpnt, my_idx));
-			/* Place this command on the pendingQ if possible */
-			spin_lock_irqsave(&hd->freedoneQlock, flags);
-			if (!Q_IS_EMPTY(&hd->freeQ)) {
-				buffer = hd->freeQ.head;
-				Q_DEL_ITEM(buffer);
-
-				/* Save the mf pointer
-				 */
-				buffer->argp = (void *)mf;
-
-				/* Add to the pendingQ
-				 */
-				Q_ADD_TAIL(&hd->pendingQ.head, buffer, MPT_DONE_Q);
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			} else {
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-				SCpnt->result = (DID_BUS_BUSY << 16);
-				SCpnt->scsi_done(SCpnt);
-			}
-		}
-	} else {
-		mptscsih_freeChainBuffers(hd->ioc, my_idx);
-		mpt_free_msg_frame(hd->ioc, mf);
-		did_errcode = 3;
-		goto did_error;
-	}
+		} else
+			goto fail;
+	} else
+		goto fail;
 
 	return 0;
 
-did_error:
-	dprintk((MYIOC_s_WARN_FMT "_qcmd did_errcode=%d (sc=%p)\n",
-			hd->ioc->name, did_errcode, SCpnt));
-	/* Just wish OS to issue a retry */
-	SCpnt->result = (DID_BUS_BUSY << 16);
-	spin_lock_irqsave(&hd->freedoneQlock, flags);
-	if (!Q_IS_EMPTY(&hd->freeQ)) {
-		dtmprintk((MYIOC_s_WARN_FMT "SCpnt=%p to doneQ\n",
-			(hd && hd->ioc) ? hd->ioc->name : "ioc?", SCpnt));
-		buffer = hd->freeQ.head;
-		Q_DEL_ITEM(buffer);
-
-		/* Set the scsi_cmnd pointer
-		 */
-		buffer->argp = (void *)SCpnt;
-
-		/* Add to the doneQ
-		 */
-		Q_ADD_TAIL(&hd->doneQ.head, buffer, MPT_DONE_Q);
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-	} else {
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-		SCpnt->scsi_done(SCpnt);
-	}
-
-	return 0;
+ fail:
+	mptscsih_freeChainBuffers(hd->ioc, my_idx);
+	mpt_free_msg_frame(hd->ioc, mf);
+	return SCSI_MLQUEUE_HOST_BUSY;
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -2551,10 +2343,11 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_abort(struct scsi_cmnd * SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER	*ioc;
 	MPT_FRAME_HDR	*mf;
 	u32		 ctx2abort;
 	int		 scpnt_idx;
@@ -2571,6 +2364,7 @@
 		return FAILED;
 	}
 
+	ioc = hd->ioc;
 	if (hd->resetPending)
 		return FAILED;
 
@@ -2583,11 +2377,9 @@
 	/* Find this command
 	 */
 	if ((scpnt_idx = SCPNT_TO_LOOKUP_IDX(SCpnt)) < 0) {
-		/* Cmd not found in ScsiLookup. If found in
-		 * doneQ, delete from Q. Do OS callback.
+		/* Cmd not found in ScsiLookup. 
+		 * Do OS callback.
 		 */
-		search_doneQ_for_cmd(hd, SCpnt);
-
 		SCpnt->result = DID_RESET << 16;
 		dtmprintk((KERN_WARNING MYNAM ": %s: mptscsih_abort: "
 			   "Command not in the active list! (sc=%p)\n",
@@ -2595,18 +2387,6 @@
 		return SUCCESS;
 	}
 
-	/* If this command is pended, then timeout/hang occurred
-	 * during DV. Post command and flush pending Q
-	 * and then following up with the reset request.
-	 */
-	if ((mf = mptscsih_search_pendingQ(hd, scpnt_idx)) != NULL) {
-		mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
-		post_pendingQ_commands(hd);
-		dtmprintk((KERN_WARNING MYNAM ": %s: mptscsih_abort: "
-			   "Posting pended cmd! (sc=%p)\n",
-			   hd->ioc->name, SCpnt));
-	}
-
 	/* Most important!  Set TaskMsgContext to SCpnt's MsgContext!
 	 * (the IO to be ABORT'd)
 	 *
@@ -2635,12 +2415,26 @@
 		 */
 		hd->tmPending = 0;
 		hd->tmState = TM_STATE_NONE;
-
+		
 		spin_lock_irq(host_lock);
+
+		/* Unmap the DMA buffers, if any. */
+		if (SCpnt->use_sg) {
+			pci_unmap_sg(ioc->pcidev, (struct scatterlist *) SCpnt->request_buffer,
+				    SCpnt->use_sg, SCpnt->sc_data_direction);
+		} else if (SCpnt->request_bufflen) {
+			pci_unmap_single(ioc->pcidev, SCpnt->SCp.dma_handle,
+				SCpnt->request_bufflen, SCpnt->sc_data_direction);
+		}
+		hd->ScsiLookup[scpnt_idx] = NULL;
+		SCpnt->result = DID_RESET << 16;
+		SCpnt->scsi_done(SCpnt);		/* Issue the command callback */
+		mptscsih_freeChainBuffers(ioc, scpnt_idx);
+		mpt_free_msg_frame(ioc, mf);
 		return FAILED;
 	}
 	spin_lock_irq(host_lock);
-	return FAILED;
+	return SUCCESS;
 
 }
 
@@ -2653,7 +2447,7 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_dev_reset(struct scsi_cmnd * SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
@@ -2708,7 +2502,7 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_bus_reset(struct scsi_cmnd * SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
@@ -2760,7 +2554,7 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_host_reset(struct scsi_cmnd *SCpnt)
 {
 	MPT_SCSI_HOST *  hd;
@@ -2919,7 +2713,6 @@
 			dtmprintk((MYIOC_s_WARN_FMT " TaskMgmt SUCCESS\n", ioc->name));
 
 			hd->abortSCpnt = NULL;
-			flush_doneQ(hd);
 
 		}
 	}
@@ -2937,7 +2730,7 @@
 /*
  *	This is anyones guess quite frankly.
  */
-int
+static int
 mptscsih_bios_param(struct scsi_device * sdev, struct block_device *bdev,
 		sector_t capacity, int geom[])
 {
@@ -2984,7 +2777,7 @@
  *	Return non-zero if allocation fails.
  *	Init memory once per id (not LUN).
  */
-int
+static int
 mptscsih_slave_alloc(struct scsi_device *device)
 {
 	struct Scsi_Host	*host = device->host;
@@ -3033,7 +2826,7 @@
  *	OS entry point to allow for host driver to free allocated memory
  *	Called if no device present or device being unloaded
  */
-void
+static void
 mptscsih_slave_destroy(struct scsi_device *device)
 {
 	struct Scsi_Host	*host = device->host;
@@ -3098,7 +2891,7 @@
  *	member to 1 if a device does not support Q tags.
  *	Return non-zero if fails.
  */
-int
+static int
 mptscsih_slave_configure(struct scsi_device *device)
 {
 	struct Scsi_Host	*sh = device->host;
@@ -3257,102 +3050,6 @@
 	return -1;
 }
 
-
-/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
-/* Search the pendingQ for a command with specific index.
- * If found, delete and return mf pointer
- * If not found, return NULL
- */
-static MPT_FRAME_HDR *
-mptscsih_search_pendingQ(MPT_SCSI_HOST *hd, int scpnt_idx)
-{
-	unsigned long	 flags;
-	MPT_DONE_Q	*buffer;
-	MPT_FRAME_HDR	*mf = NULL;
-	MPT_FRAME_HDR	*cmdMfPtr;
-
-	ddvtprintk((MYIOC_s_INFO_FMT ": search_pendingQ ...", hd->ioc->name));
-	cmdMfPtr = MPT_INDEX_2_MFPTR(hd->ioc, scpnt_idx);
-	spin_lock_irqsave(&hd->freedoneQlock, flags);
-	if (!Q_IS_EMPTY(&hd->pendingQ)) {
-		buffer = hd->pendingQ.head;
-		do {
-			mf = (MPT_FRAME_HDR *) buffer->argp;
-			if (mf == cmdMfPtr) {
-				Q_DEL_ITEM(buffer);
-
-				/* clear the arg pointer
-				 */
-				buffer->argp = NULL;
-
-				/* Add to the freeQ
-				 */
-				Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-				break;
-			}
-			mf = NULL;
-		} while ((buffer = buffer->forw) != (MPT_DONE_Q *) &hd->pendingQ);
-	}
-	spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-	ddvtprintk((" ...return %p\n", mf));
-	return mf;
-}
-
-/* Post all commands on the pendingQ to the FW.
- * Lock Q when deleting/adding members
- * Lock io_request_lock for OS callback.
- */
-static void
-post_pendingQ_commands(MPT_SCSI_HOST *hd)
-{
-	MPT_FRAME_HDR	*mf;
-	MPT_DONE_Q	*buffer;
-	unsigned long	 flags;
-
-	/* Flush the pendingQ.
-	 */
-	ddvtprintk((MYIOC_s_INFO_FMT ": post_pendingQ_commands\n", hd->ioc->name));
-	while (1) {
-		spin_lock_irqsave(&hd->freedoneQlock, flags);
-		if (Q_IS_EMPTY(&hd->pendingQ)) {
-			spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			break;
-		}
-
-		buffer = hd->pendingQ.head;
-		/* Delete from Q
-		 */
-		Q_DEL_ITEM(buffer);
-
-		mf = (MPT_FRAME_HDR *) buffer->argp;
-		buffer->argp = NULL;
-
-		/* Add to the freeQ
-		 */
-		Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-
-		if (!mf) {
-			/* This should never happen */
-			printk(MYIOC_s_WARN_FMT "post_pendingQ_commands: mf %p\n", hd->ioc->name, (void *) mf);
-			continue;
-		}
-
-		mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
-
-#if defined(MPT_DEBUG_DV) || defined(MPT_DEBUG_DV_TINY)
-		{
-			u16		 req_idx = le16_to_cpu(mf->u.frame.hwhdr.msgctxu.fld.req_idx);
-			struct scsi_cmnd	*sc = hd->ScsiLookup[req_idx];
-			printk(MYIOC_s_INFO_FMT "Issued SCSI cmd (sc=%p) idx=%d (mf=%p)\n",
-					hd->ioc->name, sc, req_idx, mf);
-		}
-#endif
-	}
-
-	return;
-}
-
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 static int
 mptscsih_ioc_reset(MPT_ADAPTER *ioc, int reset_phase)
@@ -3472,11 +3169,7 @@
 			hd->cmdPtr = NULL;
 		}
 
-		/* 7. Flush doneQ
-		 */
-		flush_doneQ(hd);
-
-		/* 8. Set flag to force DV and re-read IOC Page 3
+		/* 7. Set flag to force DV and re-read IOC Page 3
 		 */
 		if (hd->is_spi) {
 			ioc->spi_data.forceDv = MPT_SCSICFG_NEED_DV | MPT_SCSICFG_RELOAD_IOC_PG3;
@@ -3743,7 +3436,8 @@
  *  the Inquiry data, adapter capabilities, and NVRAM settings.
  *
  */
-void mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56)
+static void
+mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56)
 {
 	ScsiCfgData *pspi_data = &hd->ioc->spi_data;
 	int  id = (int) target->target_id;
@@ -5175,11 +4869,6 @@
 						}
 					}
 
-					/* Post OS IOs that were pended while
-					 * DV running.
-					 */
-					post_pendingQ_commands(hd);
-
 					if (hd->ioc->spi_data.noQas)
 						mptscsih_qas_check(hd, id);
 				}
@@ -5712,6 +5401,10 @@
 		}
 	}
 	ddvprintk((MYIOC_s_NOTE_FMT "DV: Basic test on id=%d completed OK.\n", ioc->name, id));
+
+	if (driver_setup.dv == 0)
+		goto target_done;
+
 	inq0 = (*pbuf1) & 0x1F;
 
 	/* Continue only for disks

^ permalink raw reply	[flat|nested] 18+ messages in thread
* RE: [PATCH] fix dma mapping leak in fusion
@ 2004-08-20 15:01 Moore, Eric Dean
  2004-08-23  1:41 ` Masao Fukuchi
  0 siblings, 1 reply; 18+ messages in thread
From: Moore, Eric Dean @ 2004-08-20 15:01 UTC (permalink / raw)
  To: Masao Fukuchi; +Cc: Christoph Hellwig, linux-scsi

Would it be possible for you to set up remote system 
to send kernel messages over serial cable?
Is so, pls recompile the driver with MPT_DEBUG_TM defined in
the mpt device driver Makefile; then send me log file.

Thanks,
Eric

On Thursday, August 19, 2004 9:05 PM, Masao Fukuchi wrote:
> 
> 
> Eric,
> 
> I applyed your patch and tested it, but the patch didn't work well.
> I didn't met Oops. 
> But after read command finishing with fail, keyboard and mouse click 
> didn't work.
> And also I couldn't login from other server.
> 
> The message was almost same as Fusion MPT driver 3.01.15.
> 
> Masao Fukuchi
> 
> Moore, Eric Dean wrote:
> >Here is a patch to apply against 3.01.15. It will have a small
> >fix in mptscsih_abort which should work with Christoph's suggestion
> >of removing the "is the device is offline" check in 
> >mptscsih_flush_running_cmds.  Pls test and let me know if you still 
> >hit the oops.
> >
> >Eric
> >
> >
> >On Thursday, August 19, 2004 7:15 AM, Masao Fukuchi wrote:
> >> 
> >> I tried fusion MPT driver 3.01.15 and I didn't met Oops.
> >> (Eric fixed Oops problem by Fusion MPT driver 3.01.04)
> >> 
> >> Masao Fukuchi
> >> 
> >> Message:
> >>  18:19:01 kernel: mptscsih: ioc3: >> Attempting task abort! 
> >> (sc=e00000007c205080)
> >>  18:19:01 kernel: mptscsih: ioc3: >> Attempting target reset! 
> >> (sc=e00000007c205080)
> >>  18:19:01 kernel: mptscsih: ioc3: >> Attempting bus reset! 
> >> (sc=e00000007c205080)
> >>  18:19:03 kernel: mptbase: Initiating ioc3 recovery
> >>  18:19:28 kernel: mptscsih: ioc3: >> Attempting task abort! 
> >> (sc=e00000007c205080)
> >>  18:19:28 kernel: mptscsih: ioc3: >> Attempting host reset! 
> >> (sc=e00000007c205080)
> >>  18:19:28 kernel: mptbase: Initiating ioc3 recovery
> >>  18:19:52 kernel: mptscsih: ioc3: >> Attempting task abort! 
> >> (sc=e00000007c205080)
> >>  18:19:52 kernel: scsi: Device offlined - not ready after 
> >> error recovery: host 3 channel 0 id 1 lun 0
> >>  18:19:52 kernel: scsi3 (1:0): rejecting I/O to offline device
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 0
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 1
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 2
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 3
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 4
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 5
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 6
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 7
> >>  18:19:52 kernel: scsi3 (1:0): rejecting I/O to offline device
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 0
> >>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 1
> >>  18:19:54 kernel: mptbase: Initiating ioc3 recovery
> >> 
> >> 
> >> Christoph Hellwig wrote:
> >> >On Thu, Aug 19, 2004 at 12:01:11PM +0900, Masao Fukuchi wrote:
> >> >> Hi Christoph,
> >> >> 
> >> >> I applyed attached patch into latest fusion MPT 
> driver(3.01.15) and
> >> >> tested it, but I still met Oops.
> >> >> Then I also applyed your latest patch(it gets rid of the 
> >> fusion pendingQ
> >> >> in favour of using the scsi midlayer queuing), but I met 
> >> Oops again.
> >> >
> >> >Can you try the patch Eric posted instead?
> >> >
> >> >-
> >> >To unsubscribe from this list: send the line "unsubscribe 
> >> linux-scsi" in
> >> >the body of a message to majordomo@vger.kernel.org
> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> 
> >
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* RE: [PATCH] fix dma mapping leak in fusion
@ 2004-08-19 14:47 Moore, Eric Dean
  2004-08-20  3:05 ` Masao Fukuchi
  0 siblings, 1 reply; 18+ messages in thread
From: Moore, Eric Dean @ 2004-08-19 14:47 UTC (permalink / raw)
  To: Masao Fukuchi, Christoph Hellwig; +Cc: linux-scsi

[-- Attachment #1: Type: text/plain, Size: 2747 bytes --]

Here is a patch to apply against 3.01.15. It will have a small
fix in mptscsih_abort which should work with Christoph's suggestion
of removing the "is the device is offline" check in 
mptscsih_flush_running_cmds.  Pls test and let me know if you still 
hit the oops.

Eric


On Thursday, August 19, 2004 7:15 AM, Masao Fukuchi wrote:
> 
> I tried fusion MPT driver 3.01.15 and I didn't met Oops.
> (Eric fixed Oops problem by Fusion MPT driver 3.01.04)
> 
> Masao Fukuchi
> 
> Message:
>  18:19:01 kernel: mptscsih: ioc3: >> Attempting task abort! 
> (sc=e00000007c205080)
>  18:19:01 kernel: mptscsih: ioc3: >> Attempting target reset! 
> (sc=e00000007c205080)
>  18:19:01 kernel: mptscsih: ioc3: >> Attempting bus reset! 
> (sc=e00000007c205080)
>  18:19:03 kernel: mptbase: Initiating ioc3 recovery
>  18:19:28 kernel: mptscsih: ioc3: >> Attempting task abort! 
> (sc=e00000007c205080)
>  18:19:28 kernel: mptscsih: ioc3: >> Attempting host reset! 
> (sc=e00000007c205080)
>  18:19:28 kernel: mptbase: Initiating ioc3 recovery
>  18:19:52 kernel: mptscsih: ioc3: >> Attempting task abort! 
> (sc=e00000007c205080)
>  18:19:52 kernel: scsi: Device offlined - not ready after 
> error recovery: host 3 channel 0 id 1 lun 0
>  18:19:52 kernel: scsi3 (1:0): rejecting I/O to offline device
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 0
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 1
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 2
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 3
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 4
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 5
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 6
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 7
>  18:19:52 kernel: scsi3 (1:0): rejecting I/O to offline device
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 0
>  18:19:52 kernel: Buffer I/O error on device sdb, logical block 1
>  18:19:54 kernel: mptbase: Initiating ioc3 recovery
> 
> 
> Christoph Hellwig wrote:
> >On Thu, Aug 19, 2004 at 12:01:11PM +0900, Masao Fukuchi wrote:
> >> Hi Christoph,
> >> 
> >> I applyed attached patch into latest fusion MPT driver(3.01.15) and
> >> tested it, but I still met Oops.
> >> Then I also applyed your latest patch(it gets rid of the 
> fusion pendingQ
> >> in favour of using the scsi midlayer queuing), but I met 
> Oops again.
> >
> >Can you try the patch Eric posted instead?
> >
> >-
> >To unsubscribe from this list: send the line "unsubscribe 
> linux-scsi" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: mptlinux-3.01.16.patch --]
[-- Type: application/octet-stream, Size: 21876 bytes --]

diff -uarN b/message/fusion/mptbase.c a/message/fusion/mptbase.c
--- b/message/fusion/mptbase.c	2004-08-09 16:42:28.000000000 -0600
+++ a/message/fusion/mptbase.c	2004-08-18 10:59:06.000000000 -0600
@@ -3395,7 +3395,7 @@
  *	@hd: Pointer to MPT_SCSI_HOST structure
  *	@init: If set, initialize the spin lock.
  */
-int
+static int
 initChainBuffers(MPT_ADAPTER *ioc)
 {
 	u8		*mem;
diff -uarN b/message/fusion/mptbase.h a/message/fusion/mptbase.h
--- b/message/fusion/mptbase.h	2004-08-09 16:42:45.000000000 -0600
+++ a/message/fusion/mptbase.h	2004-08-18 11:20:23.000000000 -0600
@@ -83,8 +83,8 @@
 #define COPYRIGHT	"Copyright (c) 1999-2004 " MODULEAUTHOR
 #endif
 
-#define MPT_LINUX_VERSION_COMMON	"3.01.15"
-#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.01.15"
+#define MPT_LINUX_VERSION_COMMON	"3.01.16"
+#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.01.16"
 #define WHAT_MAGIC_STRING		"@" "(" "#" ")"
 
 #define show_mptmod_ver(s,ver)  \
@@ -318,17 +318,6 @@
 	struct _Q_ITEM	*tail;
 } Q_TRACKER;
 
-typedef struct _MPT_DONE_Q {
-	struct _MPT_DONE_Q	*forw;
-	struct _MPT_DONE_Q	*back;
-	void			*argp;
-} MPT_DONE_Q;
-
-typedef struct _DONE_Q_TRACKER {
-	MPT_DONE_Q	*head;
-	MPT_DONE_Q	*tail;
-} DONE_Q_TRACKER;
-
 /*
  *  Chip-specific stuff... FC929 delineates break between
  *  FC and Parallel SCSI parts. Do NOT re-order.
@@ -1028,12 +1017,7 @@
 		/* Pool of memory for holding SCpnts before doing
 		 * OS callbacks. freeQ is the free pool.
 		 */
-	u8			 *memQ;
-	DONE_Q_TRACKER		  freeQ;
-	DONE_Q_TRACKER		  doneQ;		/* Holds Linux formmatted requests */
-	DONE_Q_TRACKER		  pendingQ;		/* Holds MPI formmatted requests */
 	MPT_Q_TRACKER		  taskQ;		/* TM request Q */
-	spinlock_t		  freedoneQlock;
 	int			  taskQcnt;
 	u8			  tmPending;
 	u8			  resetPending;
diff -uarN b/message/fusion/mptctl.c a/message/fusion/mptctl.c
--- b/message/fusion/mptctl.c	2004-07-02 09:14:30.000000000 -0600
+++ a/message/fusion/mptctl.c	2004-08-18 10:59:54.000000000 -0600
@@ -2865,7 +2865,7 @@
 };
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
-int __init mptctl_init(void)
+static int __init mptctl_init(void)
 {
 	int err;
 	int where = 1;
@@ -2963,7 +2963,7 @@
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
-void mptctl_exit(void)
+static void mptctl_exit(void)
 {
 	misc_deregister(&mptctl_miscdev);
 	printk(KERN_INFO MYNAM ": Deregistered /dev/%s @ (major,minor=%d,%d)\n",
diff -uarN b/message/fusion/mptscsih.c a/message/fusion/mptscsih.c
--- b/message/fusion/mptscsih.c	2004-08-02 17:01:01.000000000 -0600
+++ a/message/fusion/mptscsih.c	2004-08-18 16:38:57.000000000 -0600
@@ -168,8 +168,6 @@
 static void	copy_sense_data(struct scsi_cmnd *sc, MPT_SCSI_HOST *hd, MPT_FRAME_HDR *mf, SCSIIOReply_t *pScsiReply);
 static int	mptscsih_tm_pending_wait(MPT_SCSI_HOST * hd);
 static u32	SCPNT_TO_LOOKUP_IDX(struct scsi_cmnd *sc);
-static MPT_FRAME_HDR *mptscsih_search_pendingQ(MPT_SCSI_HOST *hd, int scpnt_idx);
-static void	post_pendingQ_commands(MPT_SCSI_HOST *hd);
 
 static int	mptscsih_TMHandler(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 target, u8 lun, int ctx2abort, ulong timeout, int sleepFlag);
 static int	mptscsih_IssueTaskMgmt(MPT_SCSI_HOST *hd, u8 type, u8 channel, u8 target, u8 lun, int ctx2abort, ulong timeout, int sleepFlag);
@@ -178,7 +176,7 @@
 static int	mptscsih_event_process(MPT_ADAPTER *ioc, EventNotificationReply_t *pEvReply);
 
 static void	mptscsih_initTarget(MPT_SCSI_HOST *hd, int bus_id, int target_id, u8 lun, char *data, int dlen);
-void		mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56);
+static void	mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56);
 static void	mptscsih_set_dvflags(MPT_SCSI_HOST *hd, SCSIIORequest_t *pReq);
 static void	mptscsih_setDevicePage1Flags (u8 width, u8 factor, u8 offset, int *requestedPtr, int *configurationPtr, u8 flags);
 static void	mptscsih_no_negotiate(MPT_SCSI_HOST *hd, int target_id);
@@ -895,84 +893,6 @@
 	return 1;
 }
 
-/*
- * Flush all commands on the doneQ.
- * Lock Q when deleting/adding members
- * Lock io_request_lock for OS callback.
- */
-static void
-flush_doneQ(MPT_SCSI_HOST *hd)
-{
-	MPT_DONE_Q	*buffer;
-	struct scsi_cmnd	*SCpnt;
-	unsigned long	 flags;
-
-	/* Flush the doneQ.
-	 */
-	dtmprintk((KERN_INFO MYNAM ": flush_doneQ called\n"));
-	while (1) {
-		spin_lock_irqsave(&hd->freedoneQlock, flags);
-		if (Q_IS_EMPTY(&hd->doneQ)) {
-			spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			break;
-		}
-
-		buffer = hd->doneQ.head;
-		/* Delete from Q
-		 */
-		Q_DEL_ITEM(buffer);
-
-		/* Set the struct scsi_cmnd pointer
-		 */
-		SCpnt = (struct scsi_cmnd *) buffer->argp;
-		buffer->argp = NULL;
-
-		/* Add to the freeQ
-		 */
-		Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-
-		/* Do the OS callback.
-		 */
-		SCpnt->scsi_done(SCpnt);
-	}
-
-	return;
-}
-
-/*
- * Search the doneQ for a specific command. If found, delete from Q.
- * Calling function will finish processing.
- */
-static void
-search_doneQ_for_cmd(MPT_SCSI_HOST *hd, struct scsi_cmnd *SCpnt)
-{
-	unsigned long	 flags;
-	MPT_DONE_Q	*buffer;
-
-	spin_lock_irqsave(&hd->freedoneQlock, flags);
-	if (!Q_IS_EMPTY(&hd->doneQ)) {
-		buffer = hd->doneQ.head;
-		do {
-			struct scsi_cmnd *sc = (struct scsi_cmnd *) buffer->argp;
-			if (SCpnt == sc) {
-				Q_DEL_ITEM(buffer);
-				SCpnt->result = sc->result;
-
-				/* Set the struct scsi_cmnd pointer
-				 */
-				buffer->argp = NULL;
-
-				/* Add to the freeQ
-				 */
-				Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-				break;
-			}
-		} while ((buffer = buffer->forw) != (MPT_DONE_Q *) &hd->doneQ);
-	}
-	spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-	return;
-}
 
 /*
  *	mptscsih_flush_running_cmds - For each command found, search
@@ -990,10 +910,8 @@
 	MPT_ADAPTER *ioc = hd->ioc;
 	struct scsi_cmnd	*SCpnt;
 	MPT_FRAME_HDR	*mf;
-	MPT_DONE_Q	*buffer;
 	int		 ii;
 	int		 max = ioc->req_depth;
-	unsigned long	 flags;
 
 	dprintk((KERN_INFO MYNAM ": flush_ScsiLookup called\n"));
 	for (ii= 0; ii < max; ii++) {
@@ -1002,11 +920,6 @@
 			/* Command found.
 			 */
 
-			/* Search pendingQ, if found,
-			 * delete from Q.
-			 */
-			mptscsih_search_pendingQ(hd, ii);
-
 			/* Null ScsiLookup index
 			 */
 			hd->ScsiLookup[ii] = NULL;
@@ -1019,18 +932,16 @@
 			 * Do OS callback
 			 * Free driver resources (chain, msg buffers)
 			 */
-			if (scsi_device_online(SCpnt->device)) {
-				if (SCpnt->use_sg) {
-					pci_unmap_sg(ioc->pcidev,
-						(struct scatterlist *) SCpnt->request_buffer,
-						SCpnt->use_sg,
-						SCpnt->sc_data_direction);
-				} else if (SCpnt->request_bufflen) {
-					pci_unmap_single(ioc->pcidev,
-						SCpnt->SCp.dma_handle,
-						SCpnt->request_bufflen,
-						SCpnt->sc_data_direction);
-				}
+			if (SCpnt->use_sg) {
+				pci_unmap_sg(ioc->pcidev,
+					(struct scatterlist *) SCpnt->request_buffer,
+					SCpnt->use_sg,
+					SCpnt->sc_data_direction);
+			} else if (SCpnt->request_bufflen) {
+				pci_unmap_single(ioc->pcidev,
+					SCpnt->SCp.dma_handle,
+					SCpnt->request_bufflen,
+					SCpnt->sc_data_direction);
 			}
 			SCpnt->result = DID_RESET << 16;
 			SCpnt->host_scribble = NULL;
@@ -1041,32 +952,7 @@
 			/* Free Message frames */
 			mpt_free_msg_frame(ioc, mf);
 
-#if 1
-			/* Post to doneQ, do not reply until POST phase
-			 * of reset handler....prevents new commands from
-			 * being queued.
-			 */
-			spin_lock_irqsave(&hd->freedoneQlock, flags);
-			if (!Q_IS_EMPTY(&hd->freeQ)) {
-				buffer = hd->freeQ.head;
-				Q_DEL_ITEM(buffer);
-
-				/* Set the struct scsi_cmnd pointer
-				 */
-				buffer->argp = (void *)SCpnt;
-
-				/* Add to the doneQ
-				 */
-				Q_ADD_TAIL(&hd->doneQ.head, buffer, MPT_DONE_Q);
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			} else {
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-				SCpnt->scsi_done(SCpnt);
-			}
-#else
 			SCpnt->scsi_done(SCpnt);	/* Issue the command callback */
-#endif
-
 		}
 	}
 
@@ -1173,7 +1059,6 @@
 	struct Scsi_Host	*sh;
 	MPT_SCSI_HOST		*hd;
 	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
-	MPT_DONE_Q		*freedoneQ;
 	unsigned long		 flags;
 	int			 sz, ii;
 	int			 numSGE = 0;
@@ -1330,32 +1215,6 @@
 	dprintk((MYIOC_s_INFO_FMT "ScsiLookup @ %p, sz=%d\n",
 		 ioc->name, hd->ScsiLookup, sz));
 		
-	/* Allocate memory for free and doneQ's
-	 */
-	sz = sh->can_queue * sizeof(MPT_DONE_Q);
-	mem = kmalloc(sz, GFP_ATOMIC);
-	if (mem == NULL) {
-		error = -ENOMEM;
-		goto mptscsih_probe_failed;
-	}
-
-	memset(mem, 0xFF, sz);
-	hd->memQ = mem;
-
-	/* Initialize the free, done and pending Qs.
-	 */
-	Q_INIT(&hd->freeQ, MPT_DONE_Q);
-	Q_INIT(&hd->doneQ, MPT_DONE_Q);
-	Q_INIT(&hd->pendingQ, MPT_DONE_Q);
-	spin_lock_init(&hd->freedoneQlock);
-
-	mem = hd->memQ;
-	for (ii=0; ii < sh->can_queue; ii++) {
-		freedoneQ = (MPT_DONE_Q *) mem;
-		Q_ADD_TAIL(&hd->freeQ.head, freedoneQ, MPT_DONE_Q);
-		mem += sizeof(MPT_DONE_Q);
-	}
-
 	/* Initialize this Scsi_Host
 	 * internal task Q.
 	 */
@@ -1445,10 +1304,6 @@
 #ifndef MPTSCSIH_ENABLE_DOMAIN_VALIDATION
 		hd->negoNvram = MPT_SCSICFG_USE_NVRAM;
 #endif
-		if (driver_setup.dv == 0) {
-			hd->negoNvram = MPT_SCSICFG_USE_NVRAM;
-		}
-
 		ioc->spi_data.forceDv = 0;
 		for (ii=0; ii < MPT_MAX_SCSI_DEVICES; ii++) {
 			ioc->spi_data.dvStatus[ii] =
@@ -1531,7 +1386,6 @@
 	hd = (MPT_SCSI_HOST *)host->hostdata;
 	if (hd != NULL) {
 		int sz1, sz3, sztarget=0;
-		int szQ = 0;
 
 		mptscsih_shutdown(&pdev->dev);
 
@@ -1543,12 +1397,6 @@
 			hd->ScsiLookup = NULL;
 		}
 
-		if (hd->memQ != NULL) {
-			szQ = host->can_queue * sizeof(MPT_DONE_Q);
-			kfree(hd->memQ);
-			hd->memQ = NULL;
-		}
-
 		if (hd->Targets != NULL) {
 			int max, ii;
 
@@ -1760,7 +1608,7 @@
  *
  *	Returns pointer to buffer where information was written.
  */
-const char *
+static const char *
 mptscsih_info(struct Scsi_Host *SChost)
 {
 	MPT_SCSI_HOST *h;
@@ -1975,7 +1823,8 @@
  * 	hostno: scsi host number
  *	func:   if write = 1; if read = 0
  */
-int mptscsih_proc_info(struct Scsi_Host *host, char *buffer, char **start, off_t offset,
+static int 
+mptscsih_proc_info(struct Scsi_Host *host, char *buffer, char **start, off_t offset,
 			int length, int func)
 {
 	MPT_SCSI_HOST	*hd = (MPT_SCSI_HOST *)host->hostdata;
@@ -2010,15 +1859,13 @@
  *
  *	Returns 0. (rtn value discarded by linux scsi mid-layer)
  */
-int
+static int
 mptscsih_qcmd(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_cmnd *))
 {
 	MPT_SCSI_HOST		*hd;
 	MPT_FRAME_HDR		*mf;
 	SCSIIORequest_t		*pScsiReq;
 	VirtDevice		*pTarget;
-	MPT_DONE_Q		*buffer;
-	unsigned long		 flags;
 	int	 target;
 	int	 lun;
 	u32	 datalen;
@@ -2043,16 +1890,9 @@
 			(hd && hd->ioc) ? hd->ioc->name : "ioc?", SCpnt, done));
 
 	if (hd->resetPending) {
-		/* Prevent new commands from being issued
-		 * while reloading the FW. Reset timer to 60 seconds,
-		 * as the FW can take some time to come ready.
-		 * For New EH, cmds on doneQ posted to FW.
-		 */
-		did_errcode = 1;
-		mod_timer(&SCpnt->eh_timeout, jiffies + (HZ * 60));
 		dtmprintk((MYIOC_s_WARN_FMT "qcmd: SCpnt=%p timeout + 60HZ\n",
 			(hd && hd->ioc) ? hd->ioc->name : "ioc?", SCpnt));
-		goto did_error;
+		return SCSI_MLQUEUE_HOST_BUSY;
 	}
 
 	/*
@@ -2061,8 +1901,7 @@
 	if ((mf = mpt_get_msg_frame(ScsiDoneCtx, hd->ioc)) == NULL) {
 		dprintk((MYIOC_s_WARN_FMT "QueueCmd, no msg frames!!\n",
 				hd->ioc->name));
-		did_errcode = 2;
-		goto did_error;
+		return SCSI_MLQUEUE_HOST_BUSY;
 	}
 
 	pScsiReq = (SCSIIORequest_t *) mf;
@@ -2211,64 +2050,17 @@
 			dmfprintk((MYIOC_s_INFO_FMT "Issued SCSI cmd (%p) mf=%p idx=%d\n",
 					hd->ioc->name, SCpnt, mf, my_idx));
 			DBG_DUMP_REQUEST_FRAME(mf)
-		} else {
-			ddvtprintk((MYIOC_s_INFO_FMT "Pending cmd=%p idx %d\n",
-					hd->ioc->name, SCpnt, my_idx));
-			/* Place this command on the pendingQ if possible */
-			spin_lock_irqsave(&hd->freedoneQlock, flags);
-			if (!Q_IS_EMPTY(&hd->freeQ)) {
-				buffer = hd->freeQ.head;
-				Q_DEL_ITEM(buffer);
-
-				/* Save the mf pointer
-				 */
-				buffer->argp = (void *)mf;
-
-				/* Add to the pendingQ
-				 */
-				Q_ADD_TAIL(&hd->pendingQ.head, buffer, MPT_DONE_Q);
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			} else {
-				spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-				SCpnt->result = (DID_BUS_BUSY << 16);
-				SCpnt->scsi_done(SCpnt);
-			}
-		}
-	} else {
-		mptscsih_freeChainBuffers(hd->ioc, my_idx);
-		mpt_free_msg_frame(hd->ioc, mf);
-		did_errcode = 3;
-		goto did_error;
-	}
+		} else
+			goto fail;
+	} else
+		goto fail;
 
 	return 0;
 
-did_error:
-	dprintk((MYIOC_s_WARN_FMT "_qcmd did_errcode=%d (sc=%p)\n",
-			hd->ioc->name, did_errcode, SCpnt));
-	/* Just wish OS to issue a retry */
-	SCpnt->result = (DID_BUS_BUSY << 16);
-	spin_lock_irqsave(&hd->freedoneQlock, flags);
-	if (!Q_IS_EMPTY(&hd->freeQ)) {
-		dtmprintk((MYIOC_s_WARN_FMT "SCpnt=%p to doneQ\n",
-			(hd && hd->ioc) ? hd->ioc->name : "ioc?", SCpnt));
-		buffer = hd->freeQ.head;
-		Q_DEL_ITEM(buffer);
-
-		/* Set the scsi_cmnd pointer
-		 */
-		buffer->argp = (void *)SCpnt;
-
-		/* Add to the doneQ
-		 */
-		Q_ADD_TAIL(&hd->doneQ.head, buffer, MPT_DONE_Q);
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-	} else {
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-		SCpnt->scsi_done(SCpnt);
-	}
-
-	return 0;
+ fail:
+	mptscsih_freeChainBuffers(hd->ioc, my_idx);
+	mpt_free_msg_frame(hd->ioc, mf);
+	return SCSI_MLQUEUE_HOST_BUSY;
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -2551,10 +2343,11 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_abort(struct scsi_cmnd * SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER	*ioc;
 	MPT_FRAME_HDR	*mf;
 	u32		 ctx2abort;
 	int		 scpnt_idx;
@@ -2571,6 +2364,7 @@
 		return FAILED;
 	}
 
+	ioc = hd->ioc;
 	if (hd->resetPending)
 		return FAILED;
 
@@ -2583,11 +2377,9 @@
 	/* Find this command
 	 */
 	if ((scpnt_idx = SCPNT_TO_LOOKUP_IDX(SCpnt)) < 0) {
-		/* Cmd not found in ScsiLookup. If found in
-		 * doneQ, delete from Q. Do OS callback.
+		/* Cmd not found in ScsiLookup. 
+		 * Do OS callback.
 		 */
-		search_doneQ_for_cmd(hd, SCpnt);
-
 		SCpnt->result = DID_RESET << 16;
 		dtmprintk((KERN_WARNING MYNAM ": %s: mptscsih_abort: "
 			   "Command not in the active list! (sc=%p)\n",
@@ -2595,18 +2387,6 @@
 		return SUCCESS;
 	}
 
-	/* If this command is pended, then timeout/hang occurred
-	 * during DV. Post command and flush pending Q
-	 * and then following up with the reset request.
-	 */
-	if ((mf = mptscsih_search_pendingQ(hd, scpnt_idx)) != NULL) {
-		mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
-		post_pendingQ_commands(hd);
-		dtmprintk((KERN_WARNING MYNAM ": %s: mptscsih_abort: "
-			   "Posting pended cmd! (sc=%p)\n",
-			   hd->ioc->name, SCpnt));
-	}
-
 	/* Most important!  Set TaskMsgContext to SCpnt's MsgContext!
 	 * (the IO to be ABORT'd)
 	 *
@@ -2635,12 +2415,26 @@
 		 */
 		hd->tmPending = 0;
 		hd->tmState = TM_STATE_NONE;
-
+		
 		spin_lock_irq(host_lock);
+
+		/* Unmap the DMA buffers, if any. */
+		if (SCpnt->use_sg) {
+			pci_unmap_sg(ioc->pcidev, (struct scatterlist *) SCpnt->request_buffer,
+				    SCpnt->use_sg, SCpnt->sc_data_direction);
+		} else if (SCpnt->request_bufflen) {
+			pci_unmap_single(ioc->pcidev, SCpnt->SCp.dma_handle,
+				SCpnt->request_bufflen, SCpnt->sc_data_direction);
+		}
+		hd->ScsiLookup[scpnt_idx] = NULL;
+		SCpnt->result = DID_RESET << 16;
+		SCpnt->scsi_done(SCpnt);		/* Issue the command callback */
+		mptscsih_freeChainBuffers(ioc, scpnt_idx);
+		mpt_free_msg_frame(ioc, mf);
 		return FAILED;
 	}
 	spin_lock_irq(host_lock);
-	return FAILED;
+	return SUCCESS;
 
 }
 
@@ -2653,7 +2447,7 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_dev_reset(struct scsi_cmnd * SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
@@ -2708,7 +2502,7 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_bus_reset(struct scsi_cmnd * SCpnt)
 {
 	MPT_SCSI_HOST	*hd;
@@ -2760,7 +2554,7 @@
  *
  *	Returns SUCCESS or FAILED.
  */
-int
+static int
 mptscsih_host_reset(struct scsi_cmnd *SCpnt)
 {
 	MPT_SCSI_HOST *  hd;
@@ -2919,7 +2713,6 @@
 			dtmprintk((MYIOC_s_WARN_FMT " TaskMgmt SUCCESS\n", ioc->name));
 
 			hd->abortSCpnt = NULL;
-			flush_doneQ(hd);
 
 		}
 	}
@@ -2937,7 +2730,7 @@
 /*
  *	This is anyones guess quite frankly.
  */
-int
+static int
 mptscsih_bios_param(struct scsi_device * sdev, struct block_device *bdev,
 		sector_t capacity, int geom[])
 {
@@ -2984,7 +2777,7 @@
  *	Return non-zero if allocation fails.
  *	Init memory once per id (not LUN).
  */
-int
+static int
 mptscsih_slave_alloc(struct scsi_device *device)
 {
 	struct Scsi_Host	*host = device->host;
@@ -3033,7 +2826,7 @@
  *	OS entry point to allow for host driver to free allocated memory
  *	Called if no device present or device being unloaded
  */
-void
+static void
 mptscsih_slave_destroy(struct scsi_device *device)
 {
 	struct Scsi_Host	*host = device->host;
@@ -3098,7 +2891,7 @@
  *	member to 1 if a device does not support Q tags.
  *	Return non-zero if fails.
  */
-int
+static int
 mptscsih_slave_configure(struct scsi_device *device)
 {
 	struct Scsi_Host	*sh = device->host;
@@ -3257,102 +3050,6 @@
 	return -1;
 }
 
-
-/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
-/* Search the pendingQ for a command with specific index.
- * If found, delete and return mf pointer
- * If not found, return NULL
- */
-static MPT_FRAME_HDR *
-mptscsih_search_pendingQ(MPT_SCSI_HOST *hd, int scpnt_idx)
-{
-	unsigned long	 flags;
-	MPT_DONE_Q	*buffer;
-	MPT_FRAME_HDR	*mf = NULL;
-	MPT_FRAME_HDR	*cmdMfPtr;
-
-	ddvtprintk((MYIOC_s_INFO_FMT ": search_pendingQ ...", hd->ioc->name));
-	cmdMfPtr = MPT_INDEX_2_MFPTR(hd->ioc, scpnt_idx);
-	spin_lock_irqsave(&hd->freedoneQlock, flags);
-	if (!Q_IS_EMPTY(&hd->pendingQ)) {
-		buffer = hd->pendingQ.head;
-		do {
-			mf = (MPT_FRAME_HDR *) buffer->argp;
-			if (mf == cmdMfPtr) {
-				Q_DEL_ITEM(buffer);
-
-				/* clear the arg pointer
-				 */
-				buffer->argp = NULL;
-
-				/* Add to the freeQ
-				 */
-				Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-				break;
-			}
-			mf = NULL;
-		} while ((buffer = buffer->forw) != (MPT_DONE_Q *) &hd->pendingQ);
-	}
-	spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-	ddvtprintk((" ...return %p\n", mf));
-	return mf;
-}
-
-/* Post all commands on the pendingQ to the FW.
- * Lock Q when deleting/adding members
- * Lock io_request_lock for OS callback.
- */
-static void
-post_pendingQ_commands(MPT_SCSI_HOST *hd)
-{
-	MPT_FRAME_HDR	*mf;
-	MPT_DONE_Q	*buffer;
-	unsigned long	 flags;
-
-	/* Flush the pendingQ.
-	 */
-	ddvtprintk((MYIOC_s_INFO_FMT ": post_pendingQ_commands\n", hd->ioc->name));
-	while (1) {
-		spin_lock_irqsave(&hd->freedoneQlock, flags);
-		if (Q_IS_EMPTY(&hd->pendingQ)) {
-			spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-			break;
-		}
-
-		buffer = hd->pendingQ.head;
-		/* Delete from Q
-		 */
-		Q_DEL_ITEM(buffer);
-
-		mf = (MPT_FRAME_HDR *) buffer->argp;
-		buffer->argp = NULL;
-
-		/* Add to the freeQ
-		 */
-		Q_ADD_TAIL(&hd->freeQ.head, buffer, MPT_DONE_Q);
-		spin_unlock_irqrestore(&hd->freedoneQlock, flags);
-
-		if (!mf) {
-			/* This should never happen */
-			printk(MYIOC_s_WARN_FMT "post_pendingQ_commands: mf %p\n", hd->ioc->name, (void *) mf);
-			continue;
-		}
-
-		mpt_put_msg_frame(ScsiDoneCtx, hd->ioc, mf);
-
-#if defined(MPT_DEBUG_DV) || defined(MPT_DEBUG_DV_TINY)
-		{
-			u16		 req_idx = le16_to_cpu(mf->u.frame.hwhdr.msgctxu.fld.req_idx);
-			struct scsi_cmnd	*sc = hd->ScsiLookup[req_idx];
-			printk(MYIOC_s_INFO_FMT "Issued SCSI cmd (sc=%p) idx=%d (mf=%p)\n",
-					hd->ioc->name, sc, req_idx, mf);
-		}
-#endif
-	}
-
-	return;
-}
-
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 static int
 mptscsih_ioc_reset(MPT_ADAPTER *ioc, int reset_phase)
@@ -3472,11 +3169,7 @@
 			hd->cmdPtr = NULL;
 		}
 
-		/* 7. Flush doneQ
-		 */
-		flush_doneQ(hd);
-
-		/* 8. Set flag to force DV and re-read IOC Page 3
+		/* 7. Set flag to force DV and re-read IOC Page 3
 		 */
 		if (hd->is_spi) {
 			ioc->spi_data.forceDv = MPT_SCSICFG_NEED_DV | MPT_SCSICFG_RELOAD_IOC_PG3;
@@ -3743,7 +3436,8 @@
  *  the Inquiry data, adapter capabilities, and NVRAM settings.
  *
  */
-void mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56)
+static void
+mptscsih_setTargetNegoParms(MPT_SCSI_HOST *hd, VirtDevice *target, char byte56)
 {
 	ScsiCfgData *pspi_data = &hd->ioc->spi_data;
 	int  id = (int) target->target_id;
@@ -5175,11 +4869,6 @@
 						}
 					}
 
-					/* Post OS IOs that were pended while
-					 * DV running.
-					 */
-					post_pendingQ_commands(hd);
-
 					if (hd->ioc->spi_data.noQas)
 						mptscsih_qas_check(hd, id);
 				}
@@ -5712,6 +5401,10 @@
 		}
 	}
 	ddvprintk((MYIOC_s_NOTE_FMT "DV: Basic test on id=%d completed OK.\n", ioc->name, id));
+
+	if (driver_setup.dv == 0)
+		goto target_done;
+
 	inq0 = (*pbuf1) & 0x1F;
 
 	/* Continue only for disks

^ permalink raw reply	[flat|nested] 18+ messages in thread
* RE: [PATCH] fix dma mapping leak in fusion
@ 2004-08-17 16:42 Moore, Eric Dean
  2004-08-17 16:46 ` Christoph Hellwig
  2004-08-17 18:18 ` Luben Tuikov
  0 siblings, 2 replies; 18+ messages in thread
From: Moore, Eric Dean @ 2004-08-17 16:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-scsi, fukuchi.masao

Ok - your proposed change will Opps the driver.

Sequence to oops:
1.SCSI midlayer detects command timeout and issue abort 
message to fusion MPT driver.
2.Fusion MPT driver issues abort message to IOC and returns 
SCSI midlayer with FAILED status.
3.SCSI midlayer gives up the operation and call 
scsi_release_buffers()to free buffers; this 
routine does "cmd->request_buffer=NULL;"
4.Fusion MPT driver initiates IOC reset and calls 
mptscsih_flush_running_cmds() to cancel SCSI command.
5.Oops - occurs when calling pci_unmap_sg when request_buffer=NULL.


The current fix testing for scsi_device_online 
was provided last April by Fukuchi Masao.

Eric Moore


On Tuesday, August 17, 2004 10:17 AM, Christoph Hellwig wrote:
> 
> stop fusion from leaking dma maps when the device has been offlined
> 
> 
> --- 1.13/drivers/message/fusion/linux_compat.h	
> 2004-06-23 16:48:43 +02:00
> +++ edited/drivers/message/fusion/linux_compat.h	
> 2004-08-17 20:09:21 +02:00
> @@ -3,16 +3,4 @@
>  #ifndef FUSION_LINUX_COMPAT_H
>  #define FUSION_LINUX_COMPAT_H
>  
> -#include <linux/version.h>
> -#include <scsi/scsi_device.h>
> -
> -#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,6,6))
> -static int inline scsi_device_online(struct scsi_device *sdev)
> -{
> -	return sdev->online;
> -}
> -#endif
> -
> -
> -/*}-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> -=-=-=-=-=-=-=-=*/
>  #endif /* _LINUX_COMPAT_H */
> ===== drivers/message/fusion/mptscsih.c 1.45 vs edited =====
> --- 1.45/drivers/message/fusion/mptscsih.c	2004-08-03 
> 01:01:01 +02:00
> +++ edited/drivers/message/fusion/mptscsih.c	2004-08-17 
> 20:02:54 +02:00
> @@ -1019,19 +1019,18 @@
>  			 * Do OS callback
>  			 * Free driver resources (chain, msg buffers)
>  			 */
> -			if (scsi_device_online(SCpnt->device)) {
> -				if (SCpnt->use_sg) {
> -					pci_unmap_sg(ioc->pcidev,
> -						(struct 
> scatterlist *) SCpnt->request_buffer,
> -						SCpnt->use_sg,
> -						
> SCpnt->sc_data_direction);
> -				} else if (SCpnt->request_bufflen) {
> -					pci_unmap_single(ioc->pcidev,
> -						SCpnt->SCp.dma_handle,
> -						SCpnt->request_bufflen,
> -						
> SCpnt->sc_data_direction);
> -				}
> +			if (SCpnt->use_sg) {
> +				pci_unmap_sg(ioc->pcidev,
> +					(struct scatterlist *) 
> SCpnt->request_buffer,
> +					SCpnt->use_sg,
> +					SCpnt->sc_data_direction);
> +			} else if (SCpnt->request_bufflen) {
> +				pci_unmap_single(ioc->pcidev,
> +					SCpnt->SCp.dma_handle,
> +					SCpnt->request_bufflen,
> +					SCpnt->sc_data_direction);
>  			}
> +
>  			SCpnt->result = DID_RESET << 16;
>  			SCpnt->host_scribble = NULL;
>  
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH] fix dma mapping leak in fusion
@ 2004-08-17 16:16 Christoph Hellwig
  2004-08-19  3:01 ` Masao Fukuchi
  0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2004-08-17 16:16 UTC (permalink / raw)
  To: moore; +Cc: linux-scsi

stop fusion from leaking dma maps when the device has been offlined


--- 1.13/drivers/message/fusion/linux_compat.h	2004-06-23 16:48:43 +02:00
+++ edited/drivers/message/fusion/linux_compat.h	2004-08-17 20:09:21 +02:00
@@ -3,16 +3,4 @@
 #ifndef FUSION_LINUX_COMPAT_H
 #define FUSION_LINUX_COMPAT_H
 
-#include <linux/version.h>
-#include <scsi/scsi_device.h>
-
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,6,6))
-static int inline scsi_device_online(struct scsi_device *sdev)
-{
-	return sdev->online;
-}
-#endif
-
-
-/*}-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 #endif /* _LINUX_COMPAT_H */
===== drivers/message/fusion/mptscsih.c 1.45 vs edited =====
--- 1.45/drivers/message/fusion/mptscsih.c	2004-08-03 01:01:01 +02:00
+++ edited/drivers/message/fusion/mptscsih.c	2004-08-17 20:02:54 +02:00
@@ -1019,19 +1019,18 @@
 			 * Do OS callback
 			 * Free driver resources (chain, msg buffers)
 			 */
-			if (scsi_device_online(SCpnt->device)) {
-				if (SCpnt->use_sg) {
-					pci_unmap_sg(ioc->pcidev,
-						(struct scatterlist *) SCpnt->request_buffer,
-						SCpnt->use_sg,
-						SCpnt->sc_data_direction);
-				} else if (SCpnt->request_bufflen) {
-					pci_unmap_single(ioc->pcidev,
-						SCpnt->SCp.dma_handle,
-						SCpnt->request_bufflen,
-						SCpnt->sc_data_direction);
-				}
+			if (SCpnt->use_sg) {
+				pci_unmap_sg(ioc->pcidev,
+					(struct scatterlist *) SCpnt->request_buffer,
+					SCpnt->use_sg,
+					SCpnt->sc_data_direction);
+			} else if (SCpnt->request_bufflen) {
+				pci_unmap_single(ioc->pcidev,
+					SCpnt->SCp.dma_handle,
+					SCpnt->request_bufflen,
+					SCpnt->sc_data_direction);
 			}
+
 			SCpnt->result = DID_RESET << 16;
 			SCpnt->host_scribble = NULL;
 

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2004-08-31 14:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-30 22:29 [PATCH] fix dma mapping leak in fusion Moore, Eric Dean
2004-08-31 12:56 ` Luben Tuikov
2004-08-31 13:14   ` Luben Tuikov
  -- strict thread matches above, loose matches on Subject: below --
2004-08-31 14:42 Moore, Eric Dean
2004-08-23 15:56 Moore, Eric Dean
2004-08-28 19:02 ` Christoph Hellwig
2004-08-20 15:01 Moore, Eric Dean
2004-08-23  1:41 ` Masao Fukuchi
2004-08-19 14:47 Moore, Eric Dean
2004-08-20  3:05 ` Masao Fukuchi
2004-08-17 16:42 Moore, Eric Dean
2004-08-17 16:46 ` Christoph Hellwig
2004-08-17 17:17   ` Christoph Hellwig
2004-08-17 18:18 ` Luben Tuikov
2004-08-17 16:16 Christoph Hellwig
2004-08-19  3:01 ` Masao Fukuchi
2004-08-19 10:14   ` Christoph Hellwig
2004-08-19 13:14     ` Masao Fukuchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).