[PATCH] fixing sendfile on 64bit architectures

[PATCH] fixing sendfile on 64bit architectures

[Andi Kleen]

sys_sendfile has a hardcoded 2GB limit. 64bit architectures should probably
always use sys_sendfile64() in their native system tables, because
for them sizeof(off_t) == sizeof(loff_t).  This patch does
this. It seemed easier to just change the 64bit entry tables instead of 
fixing up all the emulation layers to do 2GB checks on their own.

I changed all 64bit architectures except for parisc64, which seemed to 
already have a sendfile64.

-Andi



diff -u linux/include/asm-x86_64/unistd.h-o linux/include/asm-x86_64/unistd.h
--- linux/include/asm-x86_64/unistd.h-o	2004-05-19 11:42:07.000000000 +0200
+++ linux/include/asm-x86_64/unistd.h	2004-05-19 11:42:15.000000000 +0200
@@ -98,7 +98,7 @@
 __SYSCALL(__NR_getpid, sys_getpid)
 
 #define __NR_sendfile                           40
-__SYSCALL(__NR_sendfile, sys_sendfile)
+__SYSCALL(__NR_sendfile, sys_sendfile64)
 #define __NR_socket                             41
 __SYSCALL(__NR_socket, sys_socket)
 #define __NR_connect                            42
diff -u linux/arch/sparc64/kernel/systbls.S-o linux/arch/sparc64/kernel/systbls.S
--- linux/arch/sparc64/kernel/systbls.S-o	2004-04-06 13:12:04.000000000 +0200
+++ linux/arch/sparc64/kernel/systbls.S	2004-05-19 11:45:58.000000000 +0200
@@ -88,7 +88,7 @@
 /*20*/	.word sys_getpid, sys_capget, sys_capset, sys_setuid, sys_getuid
 /*25*/	.word sys_nis_syscall, sys_ptrace, sys_alarm, sys_sigaltstack, sys_nis_syscall
 /*30*/	.word sys_utime, sys_nis_syscall, sys_nis_syscall, sys_access, sys_nice
-	.word sys_nis_syscall, sys_sync, sys_kill, sys_newstat, sys_sendfile
+	.word sys_nis_syscall, sys_sync, sys_kill, sys_newstat, sys_sendfile64
 /*40*/	.word sys_newlstat, sys_dup, sys_pipe, sys_times, sys_nis_syscall
 	.word sys_umount, sys_setgid, sys_getgid, sys_signal, sys_geteuid
 /*50*/	.word sys_getegid, sys_acct, sys_memory_ordering, sys_nis_syscall, sys_ioctl
diff -u linux/arch/ia64/kernel/entry.S-o linux/arch/ia64/kernel/entry.S
--- linux/arch/ia64/kernel/entry.S-o	2004-05-19 11:33:01.000000000 +0200
+++ linux/arch/ia64/kernel/entry.S	2004-05-19 11:42:26.000000000 +0200
@@ -1437,7 +1437,7 @@
 	data8 sys_getcwd
 	data8 sys_capget			// 1185
 	data8 sys_capset
-	data8 sys_sendfile
+	data8 sys_sendfile64
 	data8 sys_ni_syscall		// sys_getpmsg (STREAMS)
 	data8 sys_ni_syscall		// sys_putpmsg (STREAMS)
 	data8 sys_socket			// 1190
diff -u linux/arch/alpha/kernel/systbls.S-o linux/arch/alpha/kernel/systbls.S
--- linux/arch/alpha/kernel/systbls.S-o	1970-01-01 01:12:51.000000000 +0100
+++ linux/arch/alpha/kernel/systbls.S	2004-05-19 11:43:52.000000000 +0200
@@ -389,7 +389,7 @@
 	.quad sys_getcwd
 	.quad sys_capget
 	.quad sys_capset
-	.quad sys_sendfile			/* 370 */
+	.quad sys_sendfile64			/* 370 */
 	.quad sys_setresgid
 	.quad sys_getresgid
 	.quad sys_ni_syscall			/* sys_dipc */
diff -u linux/arch/mips/kernel/scall64-64.S-o linux/arch/mips/kernel/scall64-64.S
--- linux/arch/mips/kernel/scall64-64.S-o	1970-01-01 01:12:51.000000000 +0100
+++ linux/arch/mips/kernel/scall64-64.S	2004-05-19 11:44:22.000000000 +0200
@@ -244,7 +244,7 @@
 	PTR	sys_setitimer
 	PTR	sys_alarm
 	PTR	sys_getpid
-	PTR	sys_sendfile
+	PTR	sys_sendfile64
 	PTR	sys_socket			/* 5040 */
 	PTR	sys_connect
 	PTR	sys_accept
diff -u linux/arch/ppc64/kernel/misc.S-o linux/arch/ppc64/kernel/misc.S
--- linux/arch/ppc64/kernel/misc.S-o	2004-05-19 11:32:12.000000000 +0200
+++ linux/arch/ppc64/kernel/misc.S	2004-05-19 11:43:33.000000000 +0200
@@ -1061,7 +1061,7 @@
 	.llong .sys_capget
 	.llong .sys_capset
 	.llong .sys_sigaltstack	        /* 185 */
-	.llong .sys_sendfile
+	.llong .sys_sendfile64
 	.llong .sys_ni_syscall		/* reserved for streams1 */
 	.llong .sys_ni_syscall		/* reserved for streams2 */
 	.llong .sys_vfork

Re: [PATCH] fixing sendfile on 64bit architectures

[Arnd Bergmann]

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

On Wednesday 19 May 2004 11:49, Andi Kleen wrote:

> I changed all 64bit architectures except for parisc64, which seemed to 
> already have a sendfile64.

And it looks like you missed s390.

===== arch/s390/kernel/syscalls.S 1.11 vs edited =====
--- 1.11/arch/s390/kernel/syscalls.S	Thu Apr 29 11:41:09 2004
+++ edited/arch/s390/kernel/syscalls.S	Wed May 19 14:15:36 2004
@@ -195,7 +195,7 @@
 SYSCALL(sys_capget,sys_capget,sys32_capget_wrapper)
 SYSCALL(sys_capset,sys_capset,sys32_capset_wrapper)		/* 185 */
 SYSCALL(sys_sigaltstack_glue,sys_sigaltstack_glue,sys32_sigaltstack_glue)
-SYSCALL(sys_sendfile,sys_sendfile,sys32_sendfile_wrapper)
+SYSCALL(sys_sendfile,sys_sendfile64,sys32_sendfile_wrapper)
 NI_SYSCALL							/* streams1 */
 NI_SYSCALL							/* streams2 */
 SYSCALL(sys_vfork_glue,sys_vfork_glue,sys_vfork_glue)		/* 190 */

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] fixing sendfile on 64bit architectures

[David Mosberger]

>>>>> On Wed, 19 May 2004 11:49:40 +0200, Andi Kleen <ak@suse.de> said:

  Andi> sys_sendfile has a hardcoded 2GB limit. 64bit architectures
  Andi> should probably always use sys_sendfile64() in their native
  Andi> system tables, because for them sizeof(off_t) ==
  Andi> sizeof(loff_t).  This patch does this. It seemed easier to
  Andi> just change the 64bit entry tables instead of fixing up all
  Andi> the emulation layers to do 2GB checks on their own.

Argh, I had that fixed at one point (I think it was prior to
sys_sendfile64()) but it appears that got lost somewhere along the
line.  Thanks for (re-)catching this.

  Andi> I changed all 64bit architectures except for parisc64, which
  Andi> seemed to already have a sendfile64.

The patch looks fine to me.  My only minor quibble is that it would be
nice if sys_sendfile() wasn't defined at all when it isn't needed and,
in that case, do_sendfile() should be inlined (unit-at-a-time
compilation will take care of the inlining if sys_sendfile() isn't
defined when not needed, so that part doesn't require extra code).

I don't like adding yet another "#ifdef __ia64 && ..." though.  It
would be nice if there was a cleaner way to get rid of unwanted
syscall-stubs in generic code.  How would people feel about replacing
the current #ifdef mess with "#ifdef SYS_NEED_foo" instead?  By my
count, that would get rid of about a dozen of those ugly #ifdefs.  For
example, in fs/stat.c we could have:

#ifdef SYS_NEED_OLD_STAT

instead of:

#if !defined(__alpha__) && !defined(__sparc__) && !defined(__ia64__) \
  && !defined(CONFIG_ARCH_S390) && !defined(__hppa__) \
  && !defined(__arm__) && !defined(CONFIG_V850) && !defined(__powerpc64__) \
  && !defined(__mips__)

	--david

Re: [PATCH] fixing sendfile on 64bit architectures

[Andi Kleen]

On Wed, 19 May 2004 11:17:44 -0700
David Mosberger <davidm@napali.hpl.hp.com> wrote:


> 
> The patch looks fine to me.  My only minor quibble is that it would be
> nice if sys_sendfile() wasn't defined at all when it isn't needed and,
> in that case, do_sendfile() should be inlined (unit-at-a-time
> compilation will take care of the inlining if sys_sendfile() isn't
> defined when not needed, so that part doesn't require extra code).

And I thought IA64 didn't care about code size... ;-)

> 
> I don't like adding yet another "#ifdef __ia64 && ..." though.  It
> would be nice if there was a cleaner way to get rid of unwanted
> syscall-stubs in generic code.  How would people feel about replacing
> the current #ifdef mess with "#ifdef SYS_NEED_foo" instead?  By my
> count, that would get rid of about a dozen of those ugly #ifdefs.  For
> example, in fs/stat.c we could have:
> 
> #ifdef SYS_NEED_OLD_STAT
> 
> instead of:
> 
> #if !defined(__alpha__) && !defined(__sparc__) && !defined(__ia64__) \
>   && !defined(CONFIG_ARCH_S390) && !defined(__hppa__) \
>   && !defined(__arm__) && !defined(CONFIG_V850) && !defined(__powerpc64__) \
>   && !defined(__mips__)

How about just putting it into an a extra file and using lib-y in the Makefile?

Then it will be only linked when someone references it.

-Andi

Re: [PATCH] fixing sendfile on 64bit architectures

[David Mosberger]

>>>>> On Wed, 19 May 2004 21:01:56 +0200, Andi Kleen <ak@suse.de> said:

  Andi> How about just putting it into an a extra file and using lib-y
  Andi> in the Makefile?

  Andi> Then it will be only linked when someone references it.

That may work in some cases, but usually the stubs depend on other
functions which are static only.  Moreover, putting them in separate
files prevents inlining.

	--david

Re: [PATCH] fixing sendfile on 64bit architectures

[Arnd Bergmann]

David Mosberger <davidm@napali.hpl.hp.com> schrieb am 19.05.2004,
20:17:44:

> The patch looks fine to me.  My only minor quibble is that it would be
> nice if sys_sendfile() wasn't defined at all when it isn't needed and,
> in that case, do_sendfile() should be inlined (unit-at-a-time
> compilation will take care of the inlining if sys_sendfile() isn't
> defined when not needed, so that part doesn't require extra code).

Note that we practically always need sys_sendfile anyway for calling
from sys32_sendfile. The only exception I can see is (CONFIG_64BIT &&
!CONFIG_COMPAT).
[OT: This reminds me that I have a bunch of mostly-finished compat_sys_*
consolidation patches on my disk that need to be completed. 
compat_sys_sendfile() could of course use do_sendfile() instead of
sys_sendfile().]

> I don't like adding yet another "#ifdef __ia64 && ..." though.  It
> would be nice if there was a cleaner way to get rid of unwanted
> syscall-stubs in generic code.  How would people feel about replacing
> the current #ifdef mess with "#ifdef SYS_NEED_foo" instead?  By my
> count, that would get rid of about a dozen of those ugly #ifdefs.  For
> example, in fs/stat.c we could have:
> 
> #ifdef SYS_NEED_OLD_STAT
> 
> instead of:
> 
> #if !defined(__alpha__) && !defined(__sparc__) && !defined(__ia64__) \
>   && !defined(CONFIG_ARCH_S390) && !defined(__hppa__) \
>   && !defined(__arm__) && !defined(CONFIG_V850) && !defined(__powerpc64__) \
>   && !defined(__mips__)

Seconded. While it may not be that drastic in case of sys_sendfile,
such constructions are always ugly and I suspect many of them would
turn out to be incorrect after closer inspection.

       Arnd <><

Re: [PATCH] fixing sendfile on 64bit architectures

[David S. Miller]

On Wed, 19 May 2004 23:19:55 -0700
David Mosberger <davidm@napali.hpl.hp.com> wrote:

> How about the attached?  I decided to use __ARCH_OMIT_foo instead
> of SYS_NEED_foo since that keeps the extra code in those platforms
> that want to drop the code.

This looks OK to me.