Message ID | alpine.DEB.2.20.1701120219300.27294@digraph.polyomino.org.uk |
---|---|
State | New |
Headers | show |
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> >
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 --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>