This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Fix MIPS o32 posix_fadvise [committed]
On 12/01/2017 12:08, Joseph Myers wrote:
> 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?)
Indeed this __ALIGNMENT_ARG on posix_fadvise.c code for __NR_fadvise64_64
with __ASSUME_FADVISE64_64_6ARG seems wrong. We are not hitting it because
no architecture actually uses this syscall issue option.
On posix_fadvise.c powerpc32 will use the first option (__NR_fadvise64).
Tile 32-bits will use the third, _NR_fadvise64_64 with
__ASSUME_FADVISE64_64_NO_ALIGN, so a 6 argument call with advise as last
argument.
So with fix below, ARM can use the generic code since it will use the
2 option (__NR_fadvise64 with __ASSUME_FADVISE64_64_6ARG):
diff --git a/sysdeps/unix/sysv/linux/posix_fadvise.c b/sysdeps/unix/sysv/linux/posix_fadvise.c
index 15d72d4..f3b5d22 100644
--- a/sysdeps/unix/sysv/linux/posix_fadvise.c
+++ b/sysdeps/unix/sysv/linux/posix_fadvise.c
@@ -46,8 +46,7 @@ posix_fadvise (int fd, off_t offset, off_t len, int advise)
# else
# ifdef __ASSUME_FADVISE64_64_6ARG
int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd, advise,
- __ALIGNMENT_ARG SYSCALL_LL (offset),
- SYSCALL_LL (len));
+ SYSCALL_LL (offset), SYSCALL_LL (len));
# else
# ifdef __ASSUME_FADVISE64_64_NO_ALIGN
>
> 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).
>
The only missing ABI that defines __ASSUME_ALIGNED_REGISTER_PAIRS is mips32
and as you noted we can't use default code as is. I have a local patch that
add this mips behaviour on posix_fadvise and I will send upstream.