Re: [PATCH] [powerpc] Use DIRECT_SYSVIPC_SYSCALLS

[Paul Clarke <pc at us dot ibm dot com>]

On 10/10/19 10:13 AM, Adhemerval Zanella wrote:
> On 10/10/2019 03:07, Florian Weimer wrote:
>>> diff --git a/sysdeps/unix/sysv/linux/semop.c b/sysdeps/unix/sysv/linux/semop.c
>>> index 687fdcb..e15bd5e 100644
>>> --- a/sysdeps/unix/sysv/linux/semop.c
>>> +++ b/sysdeps/unix/sysv/linux/semop.c
>>> @@ -26,7 +26,7 @@
>>>  int
>>>  semop (int semid, struct sembuf *sops, size_t nsops)
>>>  {
>>> -#ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
>>> +#if defined (__ASSUME_DIRECT_SYSVIPC_SYSCALLS) && defined (__NR_semop)
>>>    return INLINE_SYSCALL_CALL (semop, semid, sops, nsops);
>>>  #else
>>>    return INLINE_SYSCALL_CALL (ipc, IPCOP_semop, semid, nsops, 0, sops);
>>
>> Sorry, but I think this is wrong: If a future kernel version defines
>> __NR_semop, we suddenly build glibc in such a way that it is
>> incompatible with kernel 5.0 on POWER (assuming that the user requests
>> the 5.0 baseline).
>>
>> I think the best way forward here is to fix the kernel to provide the
>> semop system call on POWER, and change __ASSUME_DIRECT_SYSVIPC_SYSCALLS
>> so that it requires that kernel version as the minimum on POWER.
>>
>> If you do not want to do that, maybe the right solution is a sysdeps
>> override for POWER, unconditionally using IPCOP_semop for now, with a
>> comment why this is necessary.
> 
> I have sent a similar patch some months ago that addresses it not only
> for powerpc, but in all architecture that have wired up the sysvc syscalls
> on linux 5.1 [1][2].  My understanding back them was that it was added in
> fact for Linux 5.1 instead of 5.0 (as perf kernel commit 0d6040d4681735dfc47).

That commit is in v5.0:
--
$ git checkout v5.0
...
Note: checking out 'v5.0'.
...
HEAD is now at 1c163f4c7b3f Linux 5.0
$ git log 0d6040d4681735dfc47 -1
commit 0d6040d4681735dfc47565de288525de405a5c99
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Mon Dec 31 14:38:26 2018 +0100

    arch: add split IPC system calls where needed
--

> And I think this patch is missing addressing the compat symbol usage 
> as well.

Likely due to my ignorance.  :-/

> For the code snippet, my understanding from previous discussions is
> semtimedop and semop does not exist on 32-bit ABIs and the idea is to
> *not* provide them in future kernels version (only *semtimedop_time64*).
> I would expect that powerpc32 would follow the same strategy, however 
> indeed powerpc64 might decide do add in the future.
> 
> As you have pointed out, tying up wire-up semop with __NR_semop
> definition might indeed create a wrong build.

I asked to clarify this concern in my reply to Florian.  Since it is also gated by __ASSUME_DIRECT_SYSVIPC_SYSCALLS and that would be gated by __LINUX_VERSION >= 0x050000, which in turn comes from "--enable-kernel=5.0.0", is that not sufficient?

> For this case I 
> would prefer to instead of a sysdep override to do something as below:
> --
> #__ASSUME_DIRECT_SYSVIPC_SYSCALLS
> # ifdef __NR_semop
>   int res = INLINE_SYSCALL_CALL (semop, semid, sops, nsops);
>   if (ret >= 0 || errno != ENOSYS)
>     return ret;
> # endif

what happens here if __NR_semop is not defined?

> #else
>   return INLINE_SYSCALL_CALL (ipc, IPCOP_semop, semid, nsops, 0, sops);
> --
> 
> I will probably rework my patches to use this strategy and check
> if other 64-bit architectures are affected as well. 

I presume this would make my patch moot (which is fine, if so).  Let me know if not.

> [1] https://sourceware.org/ml/libc-alpha/2019-05/msg00438.html
> [2] https://sourceware.org/ml/libc-alpha/2019-05/msg00437.html

PC