diff mbox

Fix MIPS o32 posix_fadvise [committed]

Message ID alpine.DEB.2.20.1701120219300.27294@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Jan. 12, 2017, 2:20 a.m. UTC
The posix_fadvise consolidation broke posix_fadvise for MIPS o32, so
resulting in posix/tst-posix_fadvise failing.

MIPS o32 (and the other ABIs) has only the posix_fadvise64 syscall,
which acts like posix_fadvise64_64 (in the o32 case, because of the
alignment argument it's actually a 7-argument syscall).  The generic
posix_fadvise implementation presumes that if __NR_fadvise64 is
defined, it's for the case where a single len argument is passed to
the syscall rather than two syscall arguments in the case of a 32-bit
system.

The generic posix_fadvise64 works fine for this case (defining
__NR_fadvise64_64 to __NR_fadvise64 as needed).  ARM has a
posix_fadvise.c that uses __posix_fadvise64_l64 in posix_fadvise, and
that approach also works for MIPS o32, so this patch makes MIPS o32
include the ARM file.

Tested for MIPS o32.  Committed.

2017-01-12  Joseph Myers  <joseph@codesourcery.com>

	* sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c: New file.

Comments

Adhemerval Zanella Netto Jan. 12, 2017, 12:04 p.m. UTC | #1
Thanks for fixing it Joseph and I although I agree this fix should be the
most straightforward one for 2.25, I would like to get back on it on 2.26.
If arm behavior for posix_advise is not an outliner (as I wrongly supposed)
I think we can incorporate it on generic code an get rid of both arch
specific implementations.

On 12/01/2017 00:20, Joseph Myers wrote:
> The posix_fadvise consolidation broke posix_fadvise for MIPS o32, so
> resulting in posix/tst-posix_fadvise failing.
> 
> MIPS o32 (and the other ABIs) has only the posix_fadvise64 syscall,
> which acts like posix_fadvise64_64 (in the o32 case, because of the
> alignment argument it's actually a 7-argument syscall).  The generic
> posix_fadvise implementation presumes that if __NR_fadvise64 is
> defined, it's for the case where a single len argument is passed to
> the syscall rather than two syscall arguments in the case of a 32-bit
> system.
> 
> The generic posix_fadvise64 works fine for this case (defining
> __NR_fadvise64_64 to __NR_fadvise64 as needed).  ARM has a
> posix_fadvise.c that uses __posix_fadvise64_l64 in posix_fadvise, and
> that approach also works for MIPS o32, so this patch makes MIPS o32
> include the ARM file.
> 
> Tested for MIPS o32.  Committed.
> 
> 2017-01-12  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c: New file.
> 
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c b/sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c
> new file mode 100644
> index 0000000..6e9c3f9
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c
> @@ -0,0 +1,4 @@
> +/* The o32 MIPS fadvise64 syscall behaves as fadvise64_64.  The ARM
> +   implementation of posix_fadvise works correctly for this case; the
> +   generic implementation mishandles it.  */
> +#include <sysdeps/unix/sysv/linux/arm/posix_fadvise.c>
>
Joseph Myers Jan. 12, 2017, 2:08 p.m. UTC | #2
On Thu, 12 Jan 2017, Adhemerval Zanella wrote:

> Thanks for fixing it Joseph and I although I agree this fix should be the
> most straightforward one for 2.25, I would like to get back on it on 2.26.
> If arm behavior for posix_advise is not an outliner (as I wrongly supposed)
> I think we can incorporate it on generic code an get rid of both arch
> specific implementations.

Note that at the syscall level ARM and MIPS are different; it's just using 
__posix_fadvise64_l64 that works for both.

ARM defines only __NR_arm_fadvise64_64.  That syscall has permuted 
arguments (__ASSUME_FADVISE64_64_6ARG defined in kernel-features.h for 
ARM).  kernel-features.h then defines __NR_fadvise64_64 to 
__NR_arm_fadvise64_64 so that generic code can work for ARM.  I suspect 
the generic C version of posix_fadvise should work for ARM, given those 
definitions and care about avoiding the alignment argument.  (Why does 
posix_fadvise64.c do

#ifdef __ASSUME_FADVISE64_64_6ARG
  int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd, advise,
                                   SYSCALL_LL64 (offset), SYSCALL_LL64 (len));
#else

with the alignment argument never used in that case, but posix_fadvise.c 
do

#  ifdef __ASSUME_FADVISE64_64_6ARG
  int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd, advise,
                                   __ALIGNMENT_ARG SYSCALL_LL (offset),
                                   SYSCALL_LL (len));
#  else

which does use an alignment argument with the same syscall and argument 
order?)

MIPS defines only __NR_fadvise64, but it has the fadvise64_64 semantics 
(and does not permute arguments, so uses a 7-argument syscall for o32).  
To use generic C code for o32, you'd need to e.g. have a macro that says 
to prefer fadvise64_64 to fadvise64 (and then define __NR_fadvise64_64 to 
__NR_fadvise64 if not already defined, as done in posix_fadvise64.c).
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c b/sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c
new file mode 100644
index 0000000..6e9c3f9
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/mips32/posix_fadvise.c
@@ -0,0 +1,4 @@ 
+/* The o32 MIPS fadvise64 syscall behaves as fadvise64_64.  The ARM
+   implementation of posix_fadvise works correctly for this case; the
+   generic implementation mishandles it.  */
+#include <sysdeps/unix/sysv/linux/arm/posix_fadvise.c>