[v2] Consolidate Linux readahead() implementations

  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
  

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
>
  

On Mon, Sep 26, 2016 at 02:14:29PM -0700, Adhemerval Zanella wrote:
> 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).
> 
Ok, I'll send v3 with __ASSUME_READAHEAD_NO_ALIGN option.