This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 02/16] Consolidate Linux msgctl implementation
On 04/11/2016 13:33, Yury Norov wrote:
> On Wed, Nov 02, 2016 at 05:26:39PM -0200, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>> - Use __ASSUME_SYSVIPC_SYSCALL instead of __NR_syscall to issue the
>> wired syscall or the ipc one.
>>
>> --
>>
>> This patch consolidates the msgctl Linux implementation in only
>> one default file, sysdeps/unix/sysv/linux/msgctl.c. If tries to use
>> the direct syscall if it is supported, otherwise will use the old ipc
>> multiplex mechanism.
>>
>> The patch also simplify header inclusion and reorganize internal
>> compat symbol to be built only if old ipc is defined.
>>
>> Checked on x86_64, i686, powerpc64le, aarch64, and armhf.
>>
>> * sysdeps/unix/sysv/linux/alpha/msgctl.c: Remove file.
>> * sysdeps/unix/sysv/linux/arm/msgctl.c: Likewise.
>> * sysdeps/unix/sysv/linux/microblaze/msgctl.c: Likewise.
>> * sysdeps/unix/sysv/linux/mips/mips64/msgctl.c: Use default
>> implementation.
>> * sysdeps/unix/sysv/linux/msgctl.c (__new_msgctl): Use msgctl syscall
>> if defined.
>> ---
>> ChangeLog | 8 ++++++
>> sysdeps/unix/sysv/linux/alpha/msgctl.c | 1 -
>> sysdeps/unix/sysv/linux/arm/msgctl.c | 33 ----------------------
>> sysdeps/unix/sysv/linux/microblaze/msgctl.c | 1 -
>> sysdeps/unix/sysv/linux/mips/mips64/msgctl.c | 17 ++---------
>> sysdeps/unix/sysv/linux/msgctl.c | 42 +++++++++++++---------------
>> 6 files changed, 30 insertions(+), 72 deletions(-)
>> delete mode 100644 sysdeps/unix/sysv/linux/alpha/msgctl.c
>> delete mode 100644 sysdeps/unix/sysv/linux/arm/msgctl.c
>> delete mode 100644 sysdeps/unix/sysv/linux/microblaze/msgctl.c
>
> Hi Adhemerval,
>
> I'm testing your series with aarch64/ilp32 + LTP, and see regressions:
> msgctl01 FAIL 1
> msgctl06 FAIL 1
> msgsnd01 FAIL 1
> semctl01 FAIL 1
> semctl07 FAIL 1
> semget01 FAIL 1
> shmat01 FAIL 1
> shmctl01 FAIL 1
> shmget01 FAIL 1
>
> Regarding msgctl. objdump for libc.so shows that syscall is generated
> from stub in sysdeps/unix/sysv/linux/generic/syscalls.list.
> 000d0e80 <msgctl>:
> #else
>
> /* This is a "normal" system call stub: if there is an error,
> it returns -1 and sets errno. */
>
> T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> d0e80: d2801768 mov x8, #0xbb
> // #187
> d0e84: d4000001 svc #0x0
> d0e88: b13ffc1f cmn x0, #0xfff
> d0e8c: 54000042 b.cs d0e94
> <msgctl+0x14>
> ret
> d0e90: d65f03c0 ret
> T_PSEUDO_END (SYSCALL_SYMBOL)
> d0e94: 17fd194b b
> 173c0 <__GI___syscall_error>
> d0e98: d503201f nop
> d0e9c: d503201f nop
>
> For aarch64/ilp32 to drop custom implementation, we need
> to force __IPC_64, and so remove stubs for msgctl, semctl and shmctl
> to let glibc find proper implementation under sysv/linux.
>
> The minimal fix for my regression is below. Though, I'm not sure that it's
> complete, and that it will not affect other ports. BTW, grepping for
> msgctl in syscalls.list files shows that they has outdated comment
> which should be fixed in this series too - in arm, microblaze and mips64 ports.
>
> # Semaphore and shm system calls. msgctl, shmctl, and semctl have C
> # wrappers (to set __IPC_64).
>
> Yury.
>
> diff --git a/sysdeps/unix/sysv/linux/generic/syscalls.list b/sysdeps/unix/sysv/linux/generic/syscalls.list
> index 5993ab4..459892d 100644
> --- a/sysdeps/unix/sysv/linux/generic/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/generic/syscalls.list
> @@ -2,15 +2,12 @@
>
> # SysV APIs
> msgget - msgget i:ii __msgget msgget
> -msgctl - msgctl i:iip __msgctl msgctl
> msgrcv - msgrcv Ci:ibnii __msgrcv msgrcv
> msgsnd - msgsnd Ci:ibni __msgsnd msgsnd
> semget - semget i:iii __semget semget
> -semctl - semctl i:iiii __semctl semctl
> semtimedop - semtimedop i:ipip semtimedop
> semop - semop i:ipi __semop semop
> shmget - shmget i:iii __shmget shmget
> -shmctl - shmctl i:iip __shmctl shmctl
> shmat - shmat i:ipi __shmat shmat
> shmdt - shmdt i:s __shmdt shmdt
Yes, auto-generation syscall won't add the __IPC_64 required for
some architectures. And that's why I have added in default
implementation, since kernel will just ignore for architecture
that do not require it.
And I think by removing them from generic syscalls.list seems
reasonable, since for some architecture. I will change my
local branch accordingly.