IA-32 support patch: fcntl64 calls are broken

IA-32 support patch: fcntl64 calls are broken

[Arun Sharma]

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


Currently, sys32_fcntl64() is broken, because it passes F_*64 commands to sys_fcntl(), which it doesn't understand. The F_XXX64 commands need to be converted to F_XXX before calling sys_fcntl().

We also eliminated a copy, because the 32 bit struct flock64 and the 64 bit struct flock seem to be identical.

This patch is relevant only to 2.4, as it has already been taken care of in 2.5.

	-Arun

[-- Attachment #2: fcntl.29.txt --]
[-- Type: text/plain, Size: 654 bytes --]

--- linux/arch/ia64/ia32/sys_ia32.c-	2003-05-28 23:37:02.000000000 -0700
+++ linux/arch/ia64/ia32/sys_ia32.c	2003-06-24 02:47:10.000000000 -0700
@@ -3618,14 +3595,9 @@
 	      case F_GETLK64:
 	      case F_SETLK64:
 	      case F_SETLKW64:
-		if (ia32_get_flock(&f, arg))
-			return -EFAULT;
-		old_fs = get_fs();
-		set_fs(KERNEL_DS);
-		ret = sys_fcntl(fd, cmd, (unsigned long) &f);
-		set_fs(old_fs);
-		if (cmd == F_GETLK && ia32_put_flock(&f, arg))
-			return -EFAULT;
+		ret = sys_fcntl(fd, (cmd == F_GETLK64) ? F_GETLK :
+			((cmd == F_SETLK64) ? F_SETLK : F_SETLKW),
+			(unsigned long)arg);
 		break;
 
 	      default:

Re: IA-32 support patch: fcntl64 calls are broken

[Bjorn Helgaas]

On Tuesday 24 June 2003 4:51 pm, Arun Sharma wrote:
> Currently, sys32_fcntl64() is broken, because it passes F_*64 commands to sys_fcntl(), which it doesn't understand. The F_XXX64 commands need to be converted to F_XXX before calling sys_fcntl().
> 
> We also eliminated a copy, because the 32 bit struct flock64 and the 64 bit struct flock seem to be identical.
> 
> This patch is relevant only to 2.4, as it has already been taken care of in 2.5.

2.5 has this code, which looks suspicious to me (do we really mean
to call sys_fcntl() twice?):

        case F_GETLK64:
        case F_SETLK64:
        case F_SETLKW64:
                ret = get_compat_flock64(&f, compat_ptr(arg));
                if (ret != 0)
                        break;
                old_fs = get_fs();
                set_fs(KERNEL_DS);
                ret = sys_fcntl(fd, F_GETLK, (unsigned long)&f);
                ret = sys_fcntl(fd, (cmd = F_GETLK64) ? F_GETLK :
                                ((cmd = F_SETLK64) ? F_SETLK : F_SETLKW),
                                (unsigned long)&f);

Re: IA-32 support patch: fcntl64 calls are broken

[Arun Sharma]

Bjorn Helgaas wrote:
> 2.5 has this code, which looks suspicious to me (do we really mean
> to call sys_fcntl() twice?):

Cc'ing the author of this patch in 2.5. Stephen, did you mean to call sys_fcntl() twice ?

	-Arun

> 
>         case F_GETLK64:
>         case F_SETLK64:
>         case F_SETLKW64:
>                 ret = get_compat_flock64(&f, compat_ptr(arg));
>                 if (ret != 0)
>                         break;
>                 old_fs = get_fs();
>                 set_fs(KERNEL_DS);
>                 ret = sys_fcntl(fd, F_GETLK, (unsigned long)&f);
>                 ret = sys_fcntl(fd, (cmd = F_GETLK64) ? F_GETLK :
>                                 ((cmd = F_SETLK64) ? F_SETLK : F_SETLKW),
>                                 (unsigned long)&f);
> 

Re: IA-32 support patch: fcntl64 calls are broken

[Bjorn Helgaas]

On Tuesday 24 June 2003 4:51 pm, Arun Sharma wrote:
> Currently, sys32_fcntl64() is broken, because it passes F_*64 commands to sys_fcntl(), which it doesn't understand. The F_XXX64 commands need to be converted to F_XXX before calling sys_fcntl().

As far as I can tell, this is almost equivalent to, but uglier than,
the mips64 version:

	sys32_fcntl64(unsigned int fd, unsigned int cmd, unsigned long arg)
	{
	        switch (cmd) {
	        case F_GETLK64:
	                return sys_fcntl(fd, F_GETLK, arg);
	        case F_SETLK64:
	                return sys_fcntl(fd, F_SETLK, arg);
	        case F_SETLKW64:
	                return sys_fcntl(fd, F_SETLKW, arg);
	        }

	        return sys32_fcntl(fd, cmd, arg);
	}

parisc, ppc64, s390, and sparc64 have this, which I think is
also equivalent, but uglier:

	asmlinkage long sys32_fcntl64(unsigned int fd, unsigned int cmd, unsigned long arg)
	{
	        if (cmd >= F_GETLK64 && cmd <= F_SETLKW64)
	                return sys_fcntl(fd, cmd + F_GETLK - F_GETLK64, arg);
	        return sys32_fcntl(fd, cmd, arg);
	}

I'd like to see the ia64 version be similar to the mips64 version.
The remaining difference is the int/long argument; maybe you
can resolve that too...

Bjorn

Re: IA-32 support patch: fcntl64 calls are broken

[Stephen Rothwell]

On Wed, 25 Jun 2003 11:18:17 -0700 Arun Sharma <arun.sharma@intel.com>
wrote:
>
> Bjorn Helgaas wrote:
> > 2.5 has this code, which looks suspicious to me (do we really mean
> > to call sys_fcntl() twice?):
> 
> Cc'ing the author of this patch in 2.5. Stephen, did you mean to call
> sys_fcntl() twice ?

No. Rats! :-(  Cut and paste error (I assume).  I will submit a patch
to Linus.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: IA-32 support patch: fcntl64 calls are broken

[Bjorn Helgaas]

On Wednesday 25 June 2003 3:06 pm, I wrote:
> On Tuesday 24 June 2003 4:51 pm, Arun Sharma wrote:
> > Currently, sys32_fcntl64() is broken, because it passes F_*64 commands to sys_fcntl(), which it doesn't understand. The F_XXX64 commands need to be converted to F_XXX before calling sys_fcntl().
> 
> As far as I can tell, this is almost equivalent to, but uglier than,
> the mips64 version:

Never mind.  Besides being unnecessarily rude, I had obviouisly
forgotten the fragment from 2.5 that I just quoted.  Your patch
looks very much like the 2.5 code, which I think is the best
approach.  Even though I think the switch is more readable than
the nested ternary expressions, it's better to follow 2.5.
I apologize.

Bjorn

Re: IA-32 support patch: fcntl64 calls are broken

[Arun Sharma]

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

Arun Sharma wrote:
> 
> Currently, sys32_fcntl64() is broken, because it passes F_*64 commands 
> to sys_fcntl(), which it doesn't understand. The F_XXX64 commands need 
> to be converted to F_XXX before calling sys_fcntl().
> 
> We also eliminated a copy, because the 32 bit struct flock64 and the 64 
> bit struct flock seem to be identical.
> 

Turns out that the structures are not identical (even though the data types of all the members have the same size) due to alignment issues. So the copy can not be eliminated.

> This patch is relevant only to 2.4, as it has already been taken care of 
> in 2.5.

A new patch is attached.

	-Arun

[-- Attachment #2: fcntl.29.txt --]
[-- Type: text/plain, Size: 594 bytes --]

--- linux/arch/ia64/ia32/sys_ia32.c-	Tue Jun 24 15:43:39 2003
+++ linux/arch/ia64/ia32/sys_ia32.c	Tue Jul  1 14:09:27 2003
@@ -3622,9 +3622,11 @@
 			return -EFAULT;
 		old_fs = get_fs();
 		set_fs(KERNEL_DS);
-		ret = sys_fcntl(fd, cmd, (unsigned long) &f);
+                ret = sys_fcntl(fd, (cmd == F_GETLK64) ? F_GETLK :
+                        ((cmd == F_SETLK64) ? F_SETLK : F_SETLKW),
+                        (unsigned long) &f);
 		set_fs(old_fs);
-		if (cmd == F_GETLK && ia32_put_flock(&f, arg))
+		if (cmd == F_GETLK64 && ia32_put_flock(&f, arg))
 			return -EFAULT;
 		break;
 

Re: IA-32 support patch: fcntl64 calls are broken

[Matthew Wilcox]

On Tue, Jul 08, 2003 at 11:57:21AM -0700, Arun Sharma wrote:
> --- linux/arch/ia64/ia32/sys_ia32.c-	Tue Jun 24 15:43:39 2003
> +++ linux/arch/ia64/ia32/sys_ia32.c	Tue Jul  1 14:09:27 2003
> @@ -3622,9 +3622,11 @@
>  			return -EFAULT;
>  		old_fs = get_fs();
>  		set_fs(KERNEL_DS);
> -		ret = sys_fcntl(fd, cmd, (unsigned long) &f);
> +                ret = sys_fcntl(fd, (cmd = F_GETLK64) ? F_GETLK :

looks like you're using spaces instead of tabs.  tsk.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: IA-32 support patch: fcntl64 calls are broken

[Arun Sharma]

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

Matthew Wilcox wrote:
> On Tue, Jul 08, 2003 at 11:57:21AM -0700, Arun Sharma wrote:
> 
>>--- linux/arch/ia64/ia32/sys_ia32.c-	Tue Jun 24 15:43:39 2003
>>+++ linux/arch/ia64/ia32/sys_ia32.c	Tue Jul  1 14:09:27 2003
>>@@ -3622,9 +3622,11 @@
>> 			return -EFAULT;
>> 		old_fs = get_fs();
>> 		set_fs(KERNEL_DS);
>>-		ret = sys_fcntl(fd, cmd, (unsigned long) &f);
>>+                ret = sys_fcntl(fd, (cmd == F_GETLK64) ? F_GETLK :
> 
> 
> looks like you're using spaces instead of tabs.  tsk.
> 

I admit to cutting and pasting from 2.5 :) An updated patch with tabs is attached.

	-Arun

[-- Attachment #2: fcntl.29.txt --]
[-- Type: text/plain, Size: 538 bytes --]

--- linux/arch/ia64/ia32/sys_ia32.c-	Tue Jun 24 15:43:39 2003
+++ linux/arch/ia64/ia32/sys_ia32.c	Tue Jul  8 14:46:57 2003
@@ -3622,9 +3622,11 @@
 			return -EFAULT;
 		old_fs = get_fs();
 		set_fs(KERNEL_DS);
-		ret = sys_fcntl(fd, cmd, (unsigned long) &f);
+		ret = sys_fcntl(fd, (cmd == F_GETLK64) ? F_GETLK :
+			((cmd == F_SETLK64) ? F_SETLK : F_SETLKW),
+			(unsigned long) &f);
 		set_fs(old_fs);
-		if (cmd == F_GETLK && ia32_put_flock(&f, arg))
+		if (cmd == F_GETLK64 && ia32_put_flock(&f, arg))
 			return -EFAULT;
 		break;
 

Re: IA-32 support patch: fcntl64 calls are broken

[Bjorn Helgaas]

On Tuesday 08 July 2003 3:51 pm, Arun Sharma wrote:
> Matthew Wilcox wrote:
> > On Tue, Jul 08, 2003 at 11:57:21AM -0700, Arun Sharma wrote:
> > 
> >>--- linux/arch/ia64/ia32/sys_ia32.c-	Tue Jun 24 15:43:39 2003
> >>+++ linux/arch/ia64/ia32/sys_ia32.c	Tue Jul  1 14:09:27 2003
> >>@@ -3622,9 +3622,11 @@
> >> 			return -EFAULT;
> >> 		old_fs = get_fs();
> >> 		set_fs(KERNEL_DS);
> >>-		ret = sys_fcntl(fd, cmd, (unsigned long) &f);
> >>+                ret = sys_fcntl(fd, (cmd = F_GETLK64) ? F_GETLK :
> > 
> > 
> > looks like you're using spaces instead of tabs.  tsk.
> > 
> 
> I admit to cutting and pasting from 2.5 :) An updated patch with tabs is attached.

Applied this to 2.4.  Whew :-)