[PATCH v2] Consolidate Linux readahead() implementations
Adhemerval Zanella
adhemerval.zanella@linaro.org
Mon Sep 26 21:14:00 GMT 2016
Hi some comments below.
On 25/09/2016 04:37, Yury Norov wrote:
> There are 4 different implementations of readahead() in glibc:
> - arm and mips32 use one located under sysdeps/unix/sysv/linux/arm;
> - powerpc32 and wordsize64 has 2 another implementations in their
> syscalls.list;
> - and there's generic implementation under sysdeps/unix/sysv/linux for
> other ports.
>
> With the [1] it's possible to consolidate all of them except tile32
> under sysdeps/unix/sysv/linux. Tile32 like other 32-bit ports passes
> offset in a pair of registers, but doesn't need register alignment,
> though has __ASSUME_ALIGNED_REGISTER_PAIRS enabled. So it should be
> handled as exception.
>
> [1] https://sourceware.org/ml/libc-alpha/2016-09/msg00452.html
>
> v2:
> - remove powerpc and wordsize64 implementations;
> - remove __ASSUME_READAHEAD_ALIGN option;
> - add exception for tile32.
> v1: https://sourceware.org/ml/libc-alpha/2016-09/msg00469.html
>
> 2016-09-25: Yury Norov <ynorov@caviumnetworks.com>
>
> * sysdeps/unix/sysv/linux/mips/mips32/readahead.c: Delete.
> * sysdeps/unix/sysv/linux/powerpc/powerpc32/syscalls.list: Delete
> readahead generation.
> * sysdeps/unix/sysv/linux/wordsize-64/syscalls.list: Likewise.
> * sysdeps/unix/sysv/linux/readahead.c: Consolidate implementation.
> * sysdeps/unix/sysv/linux/tile/readahead.c: New file for
> exceptional readahead() implementation.
>
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> CC: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> CC: Florian Weimer <fw@deneb.enyo.de>
> ---
> sysdeps/unix/sysv/linux/arm/readahead.c | 37 ----------------------
> sysdeps/unix/sysv/linux/mips/mips32/readahead.c | 1 -
> .../sysv/linux/powerpc/powerpc32/syscalls.list | 3 --
> sysdeps/unix/sysv/linux/readahead.c | 6 ++--
> sysdeps/unix/sysv/linux/tile/readahead.c | 26 +++++++++++++++
> sysdeps/unix/sysv/linux/wordsize-64/syscalls.list | 1 -
> 6 files changed, 28 insertions(+), 46 deletions(-)
> delete mode 100644 sysdeps/unix/sysv/linux/arm/readahead.c
> delete mode 100644 sysdeps/unix/sysv/linux/mips/mips32/readahead.c
> create mode 100644 sysdeps/unix/sysv/linux/tile/readahead.c
>
> diff --git a/sysdeps/unix/sysv/linux/arm/readahead.c b/sysdeps/unix/sysv/linux/arm/readahead.c
> deleted file mode 100644
> index 9824e6f..0000000
> --- a/sysdeps/unix/sysv/linux/arm/readahead.c
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -/* Provide kernel hint to read ahead.
> - Copyright (C) 2002-2016 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library. If not, see
> - <http://www.gnu.org/licenses/>. */
> -
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <sys/types.h>
> -#include <endian.h>
> -
> -#include <sysdep.h>
> -#include <sys/syscall.h>
> -
> -
> -ssize_t
> -__readahead (int fd, off64_t offset, size_t count)
> -{
> - return INLINE_SYSCALL (readahead, 5, fd, 0,
> - __LONG_LONG_PAIR ((off_t) (offset >> 32),
> - (off_t) (offset & 0xffffffff)),
> - count);
> -}
> -
> -weak_alias (__readahead, readahead)
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/readahead.c b/sysdeps/unix/sysv/linux/mips/mips32/readahead.c
> deleted file mode 100644
> index 80170c3..0000000
> --- a/sysdeps/unix/sysv/linux/mips/mips32/readahead.c
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include <sysdeps/unix/sysv/linux/arm/readahead.c>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/syscalls.list b/sysdeps/unix/sysv/linux/powerpc/powerpc32/syscalls.list
> index 451d508..487bdd0 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/syscalls.list
> @@ -7,8 +7,5 @@ lchown - lchown i:sii __lchown lchown@@GLIBC_2.0 chown@GLIBC_2.0
> oldgetrlimit EXTRA getrlimit i:ip __old_getrlimit getrlimit@GLIBC_2.0
> setrlimit - setrlimit i:ip __setrlimit setrlimit@GLIBC_2.0 setrlimit@@GLIBC_2.2
>
> -# Due to 64bit alignment there is a dummy second parameter
> -readahead - readahead i:iiiii __readahead readahead
> -
> prlimit64 EXTRA prlimit64 i:iipp prlimit64
> fanotify_mark EXTRA fanotify_mark i:iiiiis fanotify_mark
> diff --git a/sysdeps/unix/sysv/linux/readahead.c b/sysdeps/unix/sysv/linux/readahead.c
> index 92e5428..ced289b 100644
> --- a/sysdeps/unix/sysv/linux/readahead.c
> +++ b/sysdeps/unix/sysv/linux/readahead.c
> @@ -29,10 +29,8 @@
> ssize_t
> __readahead (int fd, off64_t offset, size_t count)
> {
> - return INLINE_SYSCALL (readahead, 4, fd,
> - __LONG_LONG_PAIR ((off_t) (offset >> 32),
> - (off_t) (offset & 0xffffffff)),
> - count);
> + return INLINE_SYSCALL_CALL (readahead, fd, __ALIGNMENT_ARG
> + SYSCALL_LL64 (offset), count);
> }
> #else
> ssize_t
> diff --git a/sysdeps/unix/sysv/linux/tile/readahead.c b/sysdeps/unix/sysv/linux/tile/readahead.c
> new file mode 100644
> index 0000000..05d7822
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tile/readahead.c
> @@ -0,0 +1,26 @@
> +/* Provide kernel hint to read ahead.
> + Copyright (C) 2002-2016 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <sysdep.h>
> +
> +#ifndef _LP64
> +# undef __ALIGNMENT_ARG
> +# define __ALIGNMENT_ARG
> +#endif
> +
> +#include <sysdeps/unix/sysv/linux/readahead.c>
I think my assumption where one can safely redefine __ALIGNMENT_ARG seems
to be wrong. It due sysdep.h headers are included multiple times and
without guards, so building for tile I am seeing:
In file included from ../sysdeps/unix/sysv/linux/generic/sysdep.h:22:0,
from ../sysdeps/unix/sysv/linux/tile/sysdep.h:21,
from ../sysdeps/unix/sysv/linux/readahead.c:23,
from ../sysdeps/unix/sysv/linux/tile/readahead.c:26:
../sysdeps/unix/sysv/linux/sysdep.h:33:0: warning: "__ALIGNMENT_ARG" redefined
#define __ALIGNMENT_ARG 0,
^
../sysdeps/unix/sysv/linux/tile/readahead.c:23:0: note: this is the location of the previous definition
# define __ALIGNMENT_ARG
Based on this your first approach for using __ASSUME seems the most
straightforward solution (sorry for the noise).
> diff --git a/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list b/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
> index 3f3569f..1040e70 100644
> --- a/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
> @@ -7,7 +7,6 @@ ftruncate - ftruncate i:ii __ftruncate ftruncate ftruncate64 __ftruncate64
> truncate - truncate i:si truncate truncate64
> getrlimit - getrlimit i:ip __getrlimit getrlimit getrlimit64 __getrlimit64
> setrlimit - setrlimit i:ip __setrlimit setrlimit setrlimit64
> -readahead - readahead i:iii __readahead readahead
> sendfile - sendfile i:iipi sendfile sendfile64
> sync_file_range - sync_file_range Ci:iiii sync_file_range
> creat - creat Ci:si creat creat64
>
More information about the Libc-alpha
mailing list