linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
@ 2004-02-10 16:10 Moore, Eric Dean
  2004-02-10 16:24 ` Mikael Pettersson
  0 siblings, 1 reply; 7+ messages in thread
From: Moore, Eric Dean @ 2004-02-10 16:10 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Marcelo Tosatti, Keith Owens, linux-kernel

If we pass NULL as the 2nd parameter for register_ioctl32_conversion(),
the mpt_ioctl() entry point is *not* called when running a 32 bit
application in x86_64 mode.

The problem comes down to a couple IOCTL structures in mptctl.h are having
pointers and longs, and are incompatible between 32bit and 64 bit mode.
The register conversion function copy the data to proper packing, and
doesn't require people in the field to have to recompile their applications.

EXPORT_SYMBOL(sys_ioctl) is already there in s390x, ppc64, and sparc64 for
generic kernels, and in Redhat/SuSE kernels they have
EXPORT_SYMBOL(sys_ioctl)
under x86_64.


Eric Moore


On Tuesday, February 10, 2004 2:47 AM, Mikael Pettersson wrote:
> Can't you just use register_ioctl32_conversion()'s convention that
> a NULL handler defaults to sys_ioctl? Alternatively you could just
> write the one-liner 
> 
> filp->f_op->ioctl(filp->f_dentry->d_inode, filp, cmd, arg)
> 
> in your handler.
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
  2004-02-10 16:10 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c Moore, Eric Dean
@ 2004-02-10 16:24 ` Mikael Pettersson
  0 siblings, 0 replies; 7+ messages in thread
From: Mikael Pettersson @ 2004-02-10 16:24 UTC (permalink / raw)
  To: Moore, Eric Dean; +Cc: Marcelo Tosatti, Keith Owens, linux-kernel

Moore, Eric Dean writes:
 > If we pass NULL as the 2nd parameter for register_ioctl32_conversion(),
 > the mpt_ioctl() entry point is *not* called when running a 32 bit
 > application in x86_64 mode.

Ok, but you still don't need sys_ioctl() since the one-liner

 > > filp->f_op->ioctl(filp->f_dentry->d_inode, filp, cmd, arg)

(or a hardcoded call to your ioctl() method) suffices.

sys_ioctl() mostly just checks for special case ioctls before
doing the line above, but those special cases can't occur
since the kernel has already matched your particular ioctl.

/Mikael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
  2004-02-10 23:53 Moore, Eric Dean
@ 2004-02-11 11:14 ` Mikael Pettersson
  0 siblings, 0 replies; 7+ messages in thread
From: Mikael Pettersson @ 2004-02-11 11:14 UTC (permalink / raw)
  To: Moore, Eric Dean; +Cc: Marcelo Tosatti, Keith Owens, linux-kernel, linux-scsi

Moore, Eric Dean writes:
 > On Tuesday, February 10, 2004 9:25 AM, Mikael Pettersson  wrote:
 > > Moore, Eric Dean writes:
 > >  > If we pass NULL as the 2nd parameter for 
 > > register_ioctl32_conversion(),
 > >  > the mpt_ioctl() entry point is *not* called when running a 32 bit
 > >  > application in x86_64 mode.
 > > 
 > > Ok, but you still don't need sys_ioctl() since the one-liner
 > > 
 > >  > > filp->f_op->ioctl(filp->f_dentry->d_inode, filp, cmd, arg)
 > > 
 > > (or a hardcoded call to your ioctl() method) suffices.
 > > 
 > > sys_ioctl() mostly just checks for special case ioctls before
 > > doing the line above, but those special cases can't occur
 > > since the kernel has already matched your particular ioctl.
 > > 
 > > /Mikael
 > > 
 > 
 > 
 > Ok - I have modified the mpt fusion driver per your suggestions.
 > Please advise if this would work.
...
 >  static int
 > +compat_mptctl_ioctl(unsigned int fd, unsigned int cmd,
 > +			unsigned long arg, struct file *filp)
 > +{
 > +	dctlprintk((KERN_INFO MYNAM "::compat_mptctl_ioctl() called\n"));
 > +
 > +	return mptctl_ioctl(filp->f_dentry->d_inode, filp, cmd, arg);
 > +}
 > +
 > +static int
 >  compat_mptfwxfer_ioctl(unsigned int fd, unsigned int cmd,
 >  			unsigned long arg, struct file *filp)
 >  {
 > @@ -2864,30 +2872,31 @@
 >  	}
 >  
 >  #ifdef MPT_CONFIG_COMPAT
 > -	err = register_ioctl32_conversion(MPTIOCINFO, sys_ioctl);
 > +	err = register_ioctl32_conversion(MPTIOCINFO, compat_mptctl_ioctl);

Looks fine to me.

I forgot to mention that sys_ioctl() also does lock_kernel(), but
AMD64's sys32_ioctl() does not. So if you rely on having the BKL
you'll need to add lock_kernel() to your wrapper above.

/Mikael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
@ 2004-02-10 23:53 Moore, Eric Dean
  2004-02-11 11:14 ` Mikael Pettersson
  0 siblings, 1 reply; 7+ messages in thread
From: Moore, Eric Dean @ 2004-02-10 23:53 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Marcelo Tosatti, Keith Owens, linux-kernel, linux-scsi

On Tuesday, February 10, 2004 9:25 AM, Mikael Pettersson  wrote:
> Moore, Eric Dean writes:
>  > If we pass NULL as the 2nd parameter for 
> register_ioctl32_conversion(),
>  > the mpt_ioctl() entry point is *not* called when running a 32 bit
>  > application in x86_64 mode.
> 
> Ok, but you still don't need sys_ioctl() since the one-liner
> 
>  > > filp->f_op->ioctl(filp->f_dentry->d_inode, filp, cmd, arg)
> 
> (or a hardcoded call to your ioctl() method) suffices.
> 
> sys_ioctl() mostly just checks for special case ioctls before
> doing the line above, but those special cases can't occur
> since the kernel has already matched your particular ioctl.
> 
> /Mikael
> 


Ok - I have modified the mpt fusion driver per your suggestions.
Please advise if this would work.
This is 2.05.11.03 version.

You can download full source and 2.05.11.03 patches here:
ftp://ftp.lsil.com/HostAdapterDrivers/linux/Fusion-MPT/2.05.11.03



diff -uarN linux-2.4.25-rc1-ref/drivers/message/fusion/linux_compat.h
linux-2.4.25-rc1/drivers/message/fusion/linux_compat.h
--- linux-2.4.25-rc1-ref/drivers/message/fusion/linux_compat.h	2004-02-10
16:54:38.000000000 -0700
+++ linux-2.4.25-rc1/drivers/message/fusion/linux_compat.h	2004-02-10
15:05:32.000000000 -0700
@@ -12,7 +12,7 @@
 
/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
=*/
 
 
-#if (defined(__sparc__) && defined(__sparc_v9__)) || defined(__x86_64__) ||
defined(__ia64__)
+#if (defined(__sparc__) && defined(__sparc_v9__)) || defined(__x86_64__)
 #define MPT_CONFIG_COMPAT
 #endif
 
diff -uarN linux-2.4.25-rc1-ref/drivers/message/fusion/mptbase.h
linux-2.4.25-rc1/drivers/message/fusion/mptbase.h
--- linux-2.4.25-rc1-ref/drivers/message/fusion/mptbase.h	2004-02-10
16:54:38.000000000 -0700
+++ linux-2.4.25-rc1/drivers/message/fusion/mptbase.h	2004-02-10
15:45:03.000000000 -0700
@@ -80,8 +80,8 @@
 #define COPYRIGHT	"Copyright (c) 1999-2003 " MODULEAUTHOR
 #endif
 
-#define MPT_LINUX_VERSION_COMMON	"2.05.11.01"
-#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-2.05.11.01"
+#define MPT_LINUX_VERSION_COMMON	"2.05.11.03"
+#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-2.05.11.03"
 #define WHAT_MAGIC_STRING		"@" "(" "#" ")"
 
 #define show_mptmod_ver(s,ver)  \
diff -uarN linux-2.4.25-rc1-ref/drivers/message/fusion/mptctl.c
linux-2.4.25-rc1/drivers/message/fusion/mptctl.c
--- linux-2.4.25-rc1-ref/drivers/message/fusion/mptctl.c	2004-02-10
16:54:38.000000000 -0700
+++ linux-2.4.25-rc1/drivers/message/fusion/mptctl.c	2004-02-10
16:39:55.000000000 -0700
@@ -2715,7 +2715,6 @@
 						      unsigned long,
 						      struct file *));
 int unregister_ioctl32_conversion(unsigned int cmd);
-extern asmlinkage int sys_ioctl(unsigned int fd, unsigned int cmd, unsigned
long arg);
 
 
/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
=*/
 /* compat_XXX functions are used to provide a conversion between
@@ -2725,6 +2724,15 @@
  * to ensure the structure contents is properly processed by mptctl.
  */
 static int
+compat_mptctl_ioctl(unsigned int fd, unsigned int cmd,
+			unsigned long arg, struct file *filp)
+{
+	dctlprintk((KERN_INFO MYNAM "::compat_mptctl_ioctl() called\n"));
+
+	return mptctl_ioctl(filp->f_dentry->d_inode, filp, cmd, arg);
+}
+
+static int
 compat_mptfwxfer_ioctl(unsigned int fd, unsigned int cmd,
 			unsigned long arg, struct file *filp)
 {
@@ -2864,30 +2872,31 @@
 	}
 
 #ifdef MPT_CONFIG_COMPAT
-	err = register_ioctl32_conversion(MPTIOCINFO, sys_ioctl);
+	err = register_ioctl32_conversion(MPTIOCINFO, compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTIOCINFO1, sys_ioctl);
+	err = register_ioctl32_conversion(MPTIOCINFO1, compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTTARGETINFO, sys_ioctl);
+	err = register_ioctl32_conversion(MPTTARGETINFO,
compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTTEST, sys_ioctl);
+	err = register_ioctl32_conversion(MPTTEST, compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTEVENTQUERY, sys_ioctl);
+	err = register_ioctl32_conversion(MPTEVENTQUERY,
compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTEVENTENABLE, sys_ioctl);
+	err = register_ioctl32_conversion(MPTEVENTENABLE,
compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTEVENTREPORT, sys_ioctl);
+	err = register_ioctl32_conversion(MPTEVENTREPORT,
compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(MPTHARDRESET, sys_ioctl);
+	err = register_ioctl32_conversion(MPTHARDRESET,
compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
 	err = register_ioctl32_conversion(MPTCOMMAND32, compat_mpt_command);
 	if (++where && err) goto out_fail;
 	err = register_ioctl32_conversion(MPTFWDOWNLOAD32,
 					  compat_mptfwxfer_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(HP_GETHOSTINFO, sys_ioctl);
+	err = register_ioctl32_conversion(HP_GETHOSTINFO,
compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
-	err = register_ioctl32_conversion(HP_GETTARGETINFO, sys_ioctl);
+	err = register_ioctl32_conversion(HP_GETTARGETINFO,
+	    				compat_mptctl_ioctl);
 	if (++where && err) goto out_fail;
 #endif
 
diff -uarN linux-2.4.25-rc1-ref/drivers/message/fusion/mptscsih.c
linux-2.4.25-rc1/drivers/message/fusion/mptscsih.c
--- linux-2.4.25-rc1-ref/drivers/message/fusion/mptscsih.c	2004-02-10
16:54:38.000000000 -0700
+++ linux-2.4.25-rc1/drivers/message/fusion/mptscsih.c	2004-02-10
11:33:39.000000000 -0700
@@ -914,8 +914,8 @@
 			sc->resid = sc->request_bufflen - xfer_cnt;
 			dprintk((KERN_NOTICE "  SET sc->resid=%02xh\n",
sc->resid));
 #endif
-			if (sc->underflow > xfer_cnt) {
-				sc->result = DID_SOFT_ERROR;
+			if((xfer_cnt == 0 ) || (sc->underflow > xfer_cnt)) {
+				sc->result = DID_SOFT_ERROR << 16;
 			}
 
 			/* Report Queue Full

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
  2004-02-09 19:43 Moore, Eric Dean
@ 2004-02-10  9:47 ` Mikael Pettersson
  0 siblings, 0 replies; 7+ messages in thread
From: Mikael Pettersson @ 2004-02-10  9:47 UTC (permalink / raw)
  To: Moore, Eric Dean; +Cc: Marcelo Tosatti, Keith Owens, linux-kernel

Moore, Eric Dean writes:
 > On Monday, February 09, 2004 5:27 AM, Marcelo Tosatti wrote
 > > Hi Eric,
 > > 
 > > Can you please fix this up?
 > > 
 > > On Mon, 9 Feb 2004, Keith Owens wrote:
 > > 
 > > > 2.4.25-rc1 drivers/message/fusion/mptctl.c expects sys_ioctl,
 > > > register_ioctl32_conversion and unregister_ioctl32_conversion to be
 > > > exported symbols when MPT_CONFIG_COMPAT is defined.  That symbol is
 > > > defined for __sparc_v9__, __x86_64__ and __ia64__.
 > > >
 > > > The symbols are not exported in ia64, mptctl.o gets 
 > > unresolved symbols
 > > > when it is a module on ia64.
 > > >
 > > > x64_64 exports register_ioctl32_conversion and 
 > > unregister_ioctl32_conversion,
 > > > but not sys_ioctl.
 > >
 > 
 > 
 > Marcelo - Here is a fix for the x86_64 issue.
 > In Redhat/Suse kernels this "sys_ioctl" symbol is exported, but
 > not in generic kernel.  The ia64 problem is going to require
 > a fix in the mptctl driver.
 > 
 > 
 > --- linux-2.4.25-pre8-ref/arch/x86_64/ia32/ia32_ioctl.c	2004-02-09
 > 12:49:05.000000000 -0700
 > +++ linux-2.4.25-pre8/arch/x86_64/ia32/ia32_ioctl.c	2004-02-09
 > 12:00:52.000000000 -0700
 > @@ -129,6 +129,8 @@
 >  #define EXT2_IOC32_GETVERSION             _IOR('v', 1, int)
 >  #define EXT2_IOC32_SETVERSION             _IOW('v', 2, int)
 >  
 > +EXPORT_SYMBOL(sys_ioctl);
 > +

Can't you just use register_ioctl32_conversion()'s convention that
a NULL handler defaults to sys_ioctl? Alternatively you could just
write the one-liner 

filp->f_op->ioctl(filp->f_dentry->d_inode, filp, cmd, arg)

in your handler.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
@ 2004-02-09 19:43 Moore, Eric Dean
  2004-02-10  9:47 ` Mikael Pettersson
  0 siblings, 1 reply; 7+ messages in thread
From: Moore, Eric Dean @ 2004-02-09 19:43 UTC (permalink / raw)
  To: Marcelo Tosatti, Keith Owens; +Cc: linux-kernel

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

On Monday, February 09, 2004 5:27 AM, Marcelo Tosatti wrote
> Hi Eric,
> 
> Can you please fix this up?
> 
> On Mon, 9 Feb 2004, Keith Owens wrote:
> 
> > 2.4.25-rc1 drivers/message/fusion/mptctl.c expects sys_ioctl,
> > register_ioctl32_conversion and unregister_ioctl32_conversion to be
> > exported symbols when MPT_CONFIG_COMPAT is defined.  That symbol is
> > defined for __sparc_v9__, __x86_64__ and __ia64__.
> >
> > The symbols are not exported in ia64, mptctl.o gets 
> unresolved symbols
> > when it is a module on ia64.
> >
> > x64_64 exports register_ioctl32_conversion and 
> unregister_ioctl32_conversion,
> > but not sys_ioctl.
>


Marcelo - Here is a fix for the x86_64 issue.
In Redhat/Suse kernels this "sys_ioctl" symbol is exported, but
not in generic kernel.  The ia64 problem is going to require
a fix in the mptctl driver.


--- linux-2.4.25-pre8-ref/arch/x86_64/ia32/ia32_ioctl.c	2004-02-09
12:49:05.000000000 -0700
+++ linux-2.4.25-pre8/arch/x86_64/ia32/ia32_ioctl.c	2004-02-09
12:00:52.000000000 -0700
@@ -129,6 +129,8 @@
 #define EXT2_IOC32_GETVERSION             _IOR('v', 1, int)
 #define EXT2_IOC32_SETVERSION             _IOW('v', 2, int)
 
+EXPORT_SYMBOL(sys_ioctl);
+
 extern asmlinkage int sys_ioctl(unsigned int fd, unsigned int cmd, unsigned
long arg);
 
 static int w_long(unsigned int fd, unsigned int cmd, unsigned long arg) 


[-- Attachment #2: mpt-x86_64-fix.patch --]
[-- Type: application/octet-stream, Size: 527 bytes --]

--- linux-2.4.25-pre8-ref/arch/x86_64/ia32/ia32_ioctl.c	2004-02-09 12:49:05.000000000 -0700
+++ linux-2.4.25-pre8/arch/x86_64/ia32/ia32_ioctl.c	2004-02-09 12:00:52.000000000 -0700
@@ -129,6 +129,8 @@
 #define EXT2_IOC32_GETVERSION             _IOR('v', 1, int)
 #define EXT2_IOC32_SETVERSION             _IOW('v', 2, int)
 
+EXPORT_SYMBOL(sys_ioctl);
+
 extern asmlinkage int sys_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg);
 
 static int w_long(unsigned int fd, unsigned int cmd, unsigned long arg)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c
@ 2004-02-09 15:58 Moore, Eric Dean
  0 siblings, 0 replies; 7+ messages in thread
From: Moore, Eric Dean @ 2004-02-09 15:58 UTC (permalink / raw)
  To: Marcelo Tosatti, Keith Owens; +Cc: linux-kernel

Sure I'm looking at it.


On Monday, February 09, 2004 5:27 AM, Marcelo Tosatti wrote:
> 
> Hi Eric,
> 
> Can you please fix this up?
> 
> On Mon, 9 Feb 2004, Keith Owens wrote:
> 
> > 2.4.25-rc1 drivers/message/fusion/mptctl.c expects sys_ioctl,
> > register_ioctl32_conversion and unregister_ioctl32_conversion to be
> > exported symbols when MPT_CONFIG_COMPAT is defined.  That symbol is
> > defined for __sparc_v9__, __x86_64__ and __ia64__.
> >
> > The symbols are not exported in ia64, mptctl.o gets 
> unresolved symbols
> > when it is a module on ia64.
> >
> > x64_64 exports register_ioctl32_conversion and 
> unregister_ioctl32_conversion,
> > but not sys_ioctl.
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2004-02-11 11:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-02-10 16:10 2.4.25-rc1: Inconsistent ioctl symbol usage in drivers/messag e/fusion/mptctl.c Moore, Eric Dean
2004-02-10 16:24 ` Mikael Pettersson
  -- strict thread matches above, loose matches on Subject: below --
2004-02-10 23:53 Moore, Eric Dean
2004-02-11 11:14 ` Mikael Pettersson
2004-02-09 19:43 Moore, Eric Dean
2004-02-10  9:47 ` Mikael Pettersson
2004-02-09 15:58 Moore, Eric Dean

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).