[PATCH] - mptfusion - fix panic loading driver statically compile d

[PATCH] - mptfusion - fix panic loading driver statically compile d

[Moore, Eric Dean]

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

This patch can also be found at this URL:
ftp://ftp.lsil.com/HostAdapterDrivers/linux/FiberChannel/2.6-kernel/3.03.01/

Changelog:
(1) For statically linked drivers, fusion_init was not called,
added call from mpt_register so its called when one of the scsi lld load.
(2) little endian fix in mpt_put_msg_frame/mpt_get_msg_frame
(3) Clarify help desc in Kconfig for FUSION_MAX_SGE.


Signed-off-by: Eric Moore <Eric.Moore@lsil.com>







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

diff -uarN b/drivers/message/fusion/Kconfig a/drivers/message/fusion/Kconfig
--- b/drivers/message/fusion/Kconfig	2005-04-25 15:45:54.000000000 -0600
+++ a/drivers/message/fusion/Kconfig	2005-04-28 14:48:40.000000000 -0600
@@ -36,17 +36,16 @@
 	  LSIFC929XL
 
 config FUSION_MAX_SGE
-	int "Maximum number of scatter gather entries"
+	int "Maximum number of scatter gather entries (16 - 128)"
 	depends on FUSION_SPI || FUSION_FC
 	default "128"
+	range 16 128
 	help
 	  This option allows you to specify the maximum number of scatter-
-	  gather entries per I/O. The driver defaults to 40, a reasonable number
-	  for most systems. However, the user may increase this up to 128.
-	  Increasing this parameter will require significantly more memory
-	  on a per controller instance. Increasing the parameter is not
-	  necessary (or recommended) unless the user will be running
-	  large I/O's via the raw interface.
+	  gather entries per I/O. The driver default is 128, which matches
+	  SCSI_MAX_PHYS_SEGMENTS. However, it may decreased down to 16.
+	  Decreasing this parameter will reduce memory requirements
+	  on a per controller instance.
 
 config FUSION_CTL
 	tristate "Fusion MPT misc device (ioctl) driver"
diff -uarN b/drivers/message/fusion/mptbase.c a/drivers/message/fusion/mptbase.c
--- b/drivers/message/fusion/mptbase.c	2005-04-25 15:45:54.000000000 -0600
+++ a/drivers/message/fusion/mptbase.c	2005-04-28 14:46:43.000000000 -0600
@@ -113,6 +113,7 @@
 					/* Reset handler lookup table */
 static MPT_RESETHANDLER		 MptResetHandlers[MPT_MAX_PROTOCOL_DRIVERS];
 static struct mpt_pci_driver 	*MptDeviceDriverHandlers[MPT_MAX_PROTOCOL_DRIVERS];
+static int	FusionInitCalled = 0;
 
 static int	mpt_base_index = -1;
 static int	last_drv_idx = -1;
@@ -360,15 +361,8 @@
 		}
 
 		if (freeme) {
-			unsigned long flags;
-
 			/*  Put Request back on FreeQ!  */
-			spin_lock_irqsave(&ioc->FreeQlock, flags);
-			list_add_tail(&mf->u.frame.linkage.list, &ioc->FreeQ);
-#ifdef MFCNT
-			ioc->mfcnt--;
-#endif
-			spin_unlock_irqrestore(&ioc->FreeQlock, flags);
+			mpt_free_msg_frame(ioc, mf);
 		}
 
 		mb();
@@ -530,6 +524,21 @@
 
 	last_drv_idx = -1;
 
+#ifndef MODULE
+	/*
+	 *  Handle possibility of the mptscsih_detect() routine getting
+	 *  called *before* fusion_init!
+	 */
+	if (!FusionInitCalled) {
+		dinitprintk((KERN_INFO MYNAM ": Hmmm, calling fusion_init from mpt_register!\n"));
+		/*
+		 *  NOTE! We'll get recursion here, as fusion_init()
+		 *  calls mpt_register()!
+		 */
+		fusion_init();
+		FusionInitCalled++;
+	}
+#endif
 	/*
 	 *  Search for empty callback slot in this order: {N,...,7,6,5,...,1}
 	 *  (slot/handle 0 is reserved!)
@@ -735,8 +744,8 @@
 		mf->u.frame.hwhdr.msgctxu.fld.cb_idx = handle;	/* byte */
 		req_offset = (u8 *)mf - (u8 *)ioc->req_frames;
 								/* u16! */
-		req_idx = cpu_to_le16(req_offset / ioc->req_sz);
-		mf->u.frame.hwhdr.msgctxu.fld.req_idx = req_idx;
+		req_idx = req_offset / ioc->req_sz;
+		mf->u.frame.hwhdr.msgctxu.fld.req_idx = cpu_to_le16(req_idx);
 		mf->u.frame.hwhdr.msgctxu.fld.rsvd = 0;
 		ioc->RequestNB[req_idx] = ioc->NB_for_64_byte_frame; /* Default, will be changed if necessary in SG generation */
 #ifdef MFCNT
@@ -782,8 +791,8 @@
 	mf->u.frame.hwhdr.msgctxu.fld.cb_idx = handle;		/* byte */
 	req_offset = (u8 *)mf - (u8 *)ioc->req_frames;
 								/* u16! */
-	req_idx = cpu_to_le16(req_offset / ioc->req_sz);
-	mf->u.frame.hwhdr.msgctxu.fld.req_idx = req_idx;
+	req_idx = req_offset / ioc->req_sz;
+	mf->u.frame.hwhdr.msgctxu.fld.req_idx = cpu_to_le16(req_idx);
 	mf->u.frame.hwhdr.msgctxu.fld.rsvd = 0;
 
 #ifdef MPT_DEBUG_MSG_FRAME
@@ -5769,6 +5778,11 @@
 {
 	int i;
 
+	if (FusionInitCalled++) {
+		dprintk((KERN_INFO MYNAM ": INFO - Driver late-init entry point called\n"));
+		return 0;
+	}
+	
 	show_mptmod_ver(my_NAME, my_VERSION);
 	printk(KERN_INFO COPYRIGHT "\n");
 
diff -uarN b/drivers/message/fusion/mptbase.h a/drivers/message/fusion/mptbase.h
--- b/drivers/message/fusion/mptbase.h	2005-04-25 15:45:54.000000000 -0600
+++ a/drivers/message/fusion/mptbase.h	2005-04-28 14:47:25.000000000 -0600
@@ -76,8 +76,8 @@
 #define COPYRIGHT	"Copyright (c) 1999-2005 " MODULEAUTHOR
 #endif
 
-#define MPT_LINUX_VERSION_COMMON	"3.03.00"
-#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.03.00"
+#define MPT_LINUX_VERSION_COMMON	"3.03.01"
+#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.03.01"
 #define WHAT_MAGIC_STRING		"@" "(" "#" ")"
 
 #define show_mptmod_ver(s,ver)  \

Re: [PATCH] - mptfusion - fix panic loading driver statically compile d

[Christoph Hellwig]

On Thu, Apr 28, 2005 at 04:15:23PM -0600, Moore, Eric Dean wrote:
> This patch can also be found at this URL:
> ftp://ftp.lsil.com/HostAdapterDrivers/linux/FiberChannel/2.6-kernel/3.03.01/
> 
> Changelog:
> (1) For statically linked drivers, fusion_init was not called,
> added call from mpt_register so its called when one of the scsi lld load.

please find the real cause for this.  The kernel does call all module_init
handlers.  Is your link order incorrect maybe?

Re: [PATCH] - mptfusion - fix panic loading driver statically compile d

[James Bottomley]

On Thu, 2005-04-28 at 16:15 -0600, Moore, Eric Dean wrote:
> This patch can also be found at this URL:
> ftp://ftp.lsil.com/HostAdapterDrivers/linux/FiberChannel/2.6-kernel/3.03.01/
> 
> Changelog:
> (1) For statically linked drivers, fusion_init was not called,
> added call from mpt_register so its called when one of the scsi lld load.
> (2) little endian fix in mpt_put_msg_frame/mpt_get_msg_frame
> (3) Clarify help desc in Kconfig for FUSION_MAX_SGE.

Surely this is just a simple link order problem?  Can't we solve it like
the attached?

James

--- k/drivers/message/fusion/Makefile  (mode:100644)
+++ l/drivers/message/fusion/Makefile  (mode:100644)
@@ -32,7 +32,7 @@
 
 #=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-} LSI_LOGIC
 
-obj-$(CONFIG_FUSION_SPI)	+= mptspi.o mptscsih.o mptbase.o
-obj-$(CONFIG_FUSION_FC)		+= mptfc.o mptscsih.o mptbase.o
+obj-$(CONFIG_FUSION_SPI)	+=  mptbase.o mptspi.o mptscsih.o
+obj-$(CONFIG_FUSION_FC)		+=  mptbase.o mptfc.o mptscsih.o
 obj-$(CONFIG_FUSION_CTL)	+= mptctl.o
 obj-$(CONFIG_FUSION_LAN)	+= mptlan.o

Re: [PATCH] - mptfusion - fix panic loading driver statically compile d

[Andrew Morton]

James Bottomley <James.Bottomley@SteelEye.com> wrote:
>
> On Thu, 2005-04-28 at 16:15 -0600, Moore, Eric Dean wrote:
> > This patch can also be found at this URL:
> > ftp://ftp.lsil.com/HostAdapterDrivers/linux/FiberChannel/2.6-kernel/3.03.01/
> > 
> > Changelog:
> > (1) For statically linked drivers, fusion_init was not called,
> > added call from mpt_register so its called when one of the scsi lld load.
> > (2) little endian fix in mpt_put_msg_frame/mpt_get_msg_frame
> > (3) Clarify help desc in Kconfig for FUSION_MAX_SGE.
> 
> Surely this is just a simple link order problem?  Can't we solve it like
> the attached?
> 
> James
> 
> --- k/drivers/message/fusion/Makefile  (mode:100644)
> +++ l/drivers/message/fusion/Makefile  (mode:100644)
> @@ -32,7 +32,7 @@
>  
>  #=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-} LSI_LOGIC
>  
> -obj-$(CONFIG_FUSION_SPI)	+= mptspi.o mptscsih.o mptbase.o
> -obj-$(CONFIG_FUSION_FC)		+= mptfc.o mptscsih.o mptbase.o
> +obj-$(CONFIG_FUSION_SPI)	+=  mptbase.o mptspi.o mptscsih.o
> +obj-$(CONFIG_FUSION_FC)		+=  mptbase.o mptfc.o mptscsih.o
>  obj-$(CONFIG_FUSION_CTL)	+= mptctl.o
>  obj-$(CONFIG_FUSION_LAN)	+= mptlan.o

That would be a last resort :( I seem to recall people saying that this
might not work over all extant binutilses, too.

Doing something explicit via initcall levels sounds nicer.

I agree that we don't understand what's going on yet - fusion_init() should
have been called *sometime*, no matter what the linkage order is, and no
matter what the chosen initcall levels.

Booting with `initcall_debug' will tell us which initcalls are being
called, and in which order.