From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Yury Norov <ynorov@caviumnetworks.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH v2 02/16] Consolidate Linux msgctl implementation
Date: Fri, 04 Nov 2016 17:03:00 -0000 [thread overview]
Message-ID: <e4a77ad7-78aa-eea8-d090-4d9c00d91942@linaro.org> (raw)
In-Reply-To: <20161104153321.GA1271@yury-N73SV>
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.
next prev parent reply other threads:[~2016-11-04 17:03 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-02 19:27 [PATCH v2 00/16] Consolidate Linux sysvipc implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 13/16] Consolidate Linux shmctl implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 12/16] Use shmat syscall for Linux implementation Adhemerval Zanella
2016-11-07 11:02 ` Arnd Bergmann
2016-11-07 13:10 ` Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 04/16] Use msgsnd " Adhemerval Zanella
2016-11-03 14:15 ` Yury Norov
2016-11-03 20:27 ` Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 01/16] Add __ASSUME_SYSVIPC_SYSCALL for Linux Adhemerval Zanella
2016-11-02 21:10 ` Joseph Myers
2016-11-02 22:27 ` Adhemerval Zanella
2016-11-02 22:34 ` Adhemerval Zanella
2016-11-03 17:07 ` Yury Norov
2016-11-03 20:13 ` Adhemerval Zanella
2016-11-07 11:28 ` Arnd Bergmann
2016-11-07 13:17 ` Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 16/16] Add SYSV shared memory test Adhemerval Zanella
2016-11-03 17:14 ` Yury Norov
2016-11-02 19:27 ` [PATCH v2 14/16] Use shmdt syscall for linux implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 03/16] Consolidate Linux msgrcv implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 07/16] Consolidate Linux semctl implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 08/16] Use semget syscall for Linux implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 06/16] Add SYSV message queue test Adhemerval Zanella
2016-11-03 17:09 ` Yury Norov
2016-11-03 20:12 ` Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 09/16] Use semop syscall for Linux implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 15/16] Use shmget syscall for linux implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 10/16] Consolidate Linux semtimedop implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 02/16] Consolidate Linux msgctl implementation Adhemerval Zanella
2016-11-04 15:33 ` Yury Norov
2016-11-04 17:03 ` Adhemerval Zanella [this message]
2016-11-02 19:27 ` [PATCH v2 05/16] Use msgget syscall for Linux implementation Adhemerval Zanella
2016-11-02 19:27 ` [PATCH v2 11/16] Add SYSV semaphore test Adhemerval Zanella
2016-11-03 17:10 ` Yury Norov
2016-11-03 20:11 ` Adhemerval Zanella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e4a77ad7-78aa-eea8-d090-4d9c00d91942@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
--cc=ynorov@caviumnetworks.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).