Make non-compat preadv/pwritev use native register size
authorLinus Torvalds <torvalds@linux-foundation.org>
Fri, 3 Apr 2009 15:03:22 +0000 (08:03 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sat, 4 Apr 2009 21:20:34 +0000 (14:20 -0700)
Instead of always splitting the file offset into 32-bit 'high' and 'low'
parts, just split them into the largest natural word-size - which in C
terms is 'unsigned long'.

This allows 64-bit architectures to avoid the unnecessary 32-bit
shifting and masking for native format (while the compat interfaces will
obviously always have to do it).

This also changes the order of 'high' and 'low' to be "low first".  Why?
Because when we have it like this, the 64-bit system calls now don't use
the "pos_high" argument at all, and it makes more sense for the native
system call to simply match the user-mode prototype.

This results in a much more natural calling convention, and allows the
compiler to generate much more straightforward code.  On x86-64, we now
generate

        testq   %rcx, %rcx      # pos_l
        js      .L122   #,
        movq    %rcx, -48(%rbp) # pos_l, pos

from the C source

        loff_t pos = pos_from_hilo(pos_h, pos_l);
...
        if (pos < 0)
                return -EINVAL;

and the 'pos_h' register isn't even touched.  It used to generate code
like

        mov     %r8d, %r8d      # pos_low, pos_low
        salq    $32, %rcx       #, tmp71
        movq    %r8, %rax       # pos_low, pos.386
        orq     %rcx, %rax      # tmp71, pos.386
        js      .L122   #,
        movq    %rax, -48(%rbp) # pos.386, pos

which isn't _that_ horrible, but it does show how the natural word size
is just a more sensible interface (same arguments will hold in the user
level glibc wrapper function, of course, so the kernel side is just half
of the equation!)

Note: in all cases the user code wrapper can again be the same. You can
just do

#define HALF_BITS (sizeof(unsigned long)*4)
__syscall(PWRITEV, fd, iov, count, offset, (offset >> HALF_BITS) >> HALF_BITS);

or something like that.  That way the user mode wrapper will also be
nicely passing in a zero (it won't actually have to do the shifts, the
compiler will understand what is going on) for the last argument.

And that is a good idea, even if nobody will necessarily ever care: if
we ever do move to a 128-bit lloff_t, this particular system call might
be left alone.  Of course, that will be the least of our worries if we
really ever need to care, so this may not be worth really caring about.

[ Fixed for lost 'loff_t' cast noticed by Andrew Morton ]

Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Ralf Baechle <ralf@linux-mips.org>>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/compat.c
fs/read_write.c
include/linux/compat.h
include/linux/syscalls.h

index 1c859dae758f8a01efa3e1e793bcfffc42fce604..3f84d5f15889604b9ff1d7d4af758ff05edad24f 100644 (file)
@@ -1236,7 +1236,7 @@ compat_sys_readv(unsigned long fd, const struct compat_iovec __user *vec,
 
 asmlinkage ssize_t
 compat_sys_preadv(unsigned long fd, const struct compat_iovec __user *vec,
-                 unsigned long vlen, u32 pos_high, u32 pos_low)
+                 unsigned long vlen, u32 pos_low, u32 pos_high)
 {
        loff_t pos = ((loff_t)pos_high << 32) | pos_low;
        struct file *file;
@@ -1293,7 +1293,7 @@ compat_sys_writev(unsigned long fd, const struct compat_iovec __user *vec,
 
 asmlinkage ssize_t
 compat_sys_pwritev(unsigned long fd, const struct compat_iovec __user *vec,
-                  unsigned long vlen, u32 pos_high, u32 pos_low)
+                  unsigned long vlen, u32 pos_low, u32 pos_high)
 {
        loff_t pos = ((loff_t)pos_high << 32) | pos_low;
        struct file *file;
index 6d5d8ff238aa50cdce17c4126f775d9af9b00b8f..9d1e76bb9ee147b8236630ba953a53873b1847d4 100644 (file)
@@ -731,10 +731,16 @@ SYSCALL_DEFINE3(writev, unsigned long, fd, const struct iovec __user *, vec,
        return ret;
 }
 
+static inline loff_t pos_from_hilo(unsigned long high, unsigned long low)
+{
+#define HALF_LONG_BITS (BITS_PER_LONG / 2)
+       return (((loff_t)high << HALF_LONG_BITS) << HALF_LONG_BITS) | low;
+}
+
 SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct iovec __user *, vec,
-               unsigned long, vlen, u32, pos_high, u32, pos_low)
+               unsigned long, vlen, unsigned long, pos_l, unsigned long, pos_h)
 {
-       loff_t pos = ((loff_t)pos_high << 32) | pos_low;
+       loff_t pos = pos_from_hilo(pos_h, pos_l);
        struct file *file;
        ssize_t ret = -EBADF;
        int fput_needed;
@@ -757,9 +763,9 @@ SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct iovec __user *, vec,
 }
 
 SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct iovec __user *, vec,
-               unsigned long, vlen, u32, pos_high, u32, pos_low)
+               unsigned long, vlen, unsigned long, pos_l, unsigned long, pos_h)
 {
-       loff_t pos = ((loff_t)pos_high << 32) | pos_low;
+       loff_t pos = pos_from_hilo(pos_h, pos_l);
        struct file *file;
        ssize_t ret = -EBADF;
        int fput_needed;
index 9723edd6455cb8d612923f8a5706fb691e2a1525..f2ded21f9a3c37cdef070315659c57d5a066f4f9 100644 (file)
@@ -193,10 +193,10 @@ asmlinkage ssize_t compat_sys_writev(unsigned long fd,
                const struct compat_iovec __user *vec, unsigned long vlen);
 asmlinkage ssize_t compat_sys_preadv(unsigned long fd,
                const struct compat_iovec __user *vec,
-               unsigned long vlen, u32 pos_high, u32 pos_low);
+               unsigned long vlen, u32 pos_low, u32 pos_high);
 asmlinkage ssize_t compat_sys_pwritev(unsigned long fd,
                const struct compat_iovec __user *vec,
-               unsigned long vlen, u32 pos_high, u32 pos_low);
+               unsigned long vlen, u32 pos_low, u32 pos_high);
 
 int compat_do_execve(char * filename, compat_uptr_t __user *argv,
                compat_uptr_t __user *envp, struct pt_regs * regs);
index b299a82a05e7abd369f6b30d9b7182238112b685..18771cac2f8513a38c4296859a88c65544511c1a 100644 (file)
@@ -462,9 +462,9 @@ asmlinkage long sys_pread64(unsigned int fd, char __user *buf,
 asmlinkage long sys_pwrite64(unsigned int fd, const char __user *buf,
                             size_t count, loff_t pos);
 asmlinkage long sys_preadv(unsigned long fd, const struct iovec __user *vec,
-                          unsigned long vlen, u32 pos_high, u32 pos_low);
+                          unsigned long vlen, unsigned long pos_l, unsigned long pos_h);
 asmlinkage long sys_pwritev(unsigned long fd, const struct iovec __user *vec,
-                           unsigned long vlen, u32 pos_high, u32 pos_low);
+                           unsigned long vlen, unsigned long pos_l, unsigned long pos_h);
 asmlinkage long sys_getcwd(char __user *buf, unsigned long size);
 asmlinkage long sys_mkdir(const char __user *pathname, int mode);
 asmlinkage long sys_chdir(const char __user *filename);