[powerpc] Use DIRECT_SYSVIPC_SYSCALLS

  From: "Paul A. Clarke" <pc@us.ibm.com>

Support for direct (non-multiplexed) SysV IPC syscalls was added for
powerpc in Linux 5.0.

Support in glibc is already present, but disabled for powerpc.

1. Enable this support for powerpc if glibc is built for kernels >= 5.0.0.
2. Stop passing __IPC_64 bit in associated commands, as this is no longer
   required or expected with this support on powerpc.
3. "semop" is in the set of "SysV IPC" syscalls which are enabled in glibc,
   but semop is not supported on powerpc even in the Linux 5.0 kernel, so
   disable this syscall if __NR_semop is not defined.

2019-10-09  Paul A. Clarke  <pc@us.ibm.com>

	* sysdeps/unix/sysv/linux/powerpc/kernel-features.h: Enable
	(do not disable) __ASSUME_DIRECT_SYSVIPC_SYSCALLS for Linux >= 5.0.0.
	* sysdeps/unix/sysv/linux/powerpc/ipc_priv.h: Nullify __IPC_64
	for Linux >= 5.0.0.
	* sysdeps/unix/sysv/linux/semop.c: Disable DIRECT_SYSCALL unless
	__NR_semop is defined.
---
Tested on openSUSE Tumbleweed 20190814 with kernel 5.1.3-1-default.
A hacked up benchmark (which called msgctl (0,0,0) and thus returned failure,
so basically just testing system call overhead), showed about 12% improvement
in latency.

 sysdeps/unix/sysv/linux/powerpc/ipc_priv.h        | 4 ++++
 sysdeps/unix/sysv/linux/powerpc/kernel-features.h | 2 ++
 sysdeps/unix/sysv/linux/semop.c                   | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)
  

* Paul A. Clarke:

> 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.

Thanks,
Florian
  

On 10/10/2019 03:07, Florian Weimer wrote:
> * Paul A. Clarke:
> 
>> 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).

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

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.  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
#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. 

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

On 10/10/19 1:07 AM, 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).

This would present a problem for POWER only when __ASSUME_DIRECT_SYSVIPC_SYSCALLS is set. This is only set (or, not unset) in another part of this same patch and only when __LINUX_KERNEL_VERSION >= 0x050000 (that I believe will come only when "--enable-kernel=5.0.0" is passed to configure).  Is that not sufficient?  (I understood this to be sufficient based on Joseph's comments in [1].)

> 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.

The same historical thread referenced above started with a patch [2] from 2015 to realize the same effect. In that thread, Michael Ellerman and Arnd Bergman discuss reverting the original support patches from Linux 4.4, which don't appear again, it seems, until Linux 5.0.  But, it seems that "semop" was not included in that patchset (Linux kernel commit 0d6040d468173).  Is that expected, Arnd?

> 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.

So, something like this, in sysdeps/unix/sysv/linux/semop.c?
--
 +/* [powerpc64 only] While most SysV IPC syscalls are implemented as direct
 +   syscalls on powerpc64, semop is not.  */
 +#if defined (__ASSUME_DIRECT_SYSVIPC_SYSCALLS) && !defined (__powerpc64__)
    return INLINE_SYSCALL_CALL (semop, semid, sops, nsops);
  #else
    return INLINE_SYSCALL_CALL (ipc, IPCOP_semop, semid, nsops, 0, sops);
 +#endif
--

[1] https://sourceware.org/ml/libc-alpha/2015-12/msg00065.html
[2] https://sourceware.org/ml/libc-alpha/2015-12/msg00051.html

PC
  

* Paul Clarke:

> On 10/10/19 1:07 AM, 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).
>
> This would present a problem for POWER only when
> __ASSUME_DIRECT_SYSVIPC_SYSCALLS is set. This is only set (or, not
> unset) in another part of this same patch and only when
> __LINUX_KERNEL_VERSION >= 0x050000 (that I believe will come only when
> "--enable-kernel=5.0.0" is passed to configure).  Is that not
> sufficient?  (I understood this to be sufficient based on Joseph's
> comments in [1].)

I would expect --enable-kernel=5.0.0 to set a kernel 5.0 baseline even
if kernel 5.11 (a made that up) adds the semop system call on POWER.  My
understanding that this wouldn't work with your patch, it would require
kernel 5.11 for a working semop function, not kernel 5.0 as requested.

I hope this clarifies my concern.

>> 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.
>
> So, something like this, in sysdeps/unix/sysv/linux/semop.c?
> --
>  +/* [powerpc64 only] While most SysV IPC syscalls are implemented as direct
>  +   syscalls on powerpc64, semop is not.  */
>  +#if defined (__ASSUME_DIRECT_SYSVIPC_SYSCALLS) && !defined (__powerpc64__)
>     return INLINE_SYSCALL_CALL (semop, semid, sops, nsops);
>   #else
>     return INLINE_SYSCALL_CALL (ipc, IPCOP_semop, semid, nsops, 0, sops);
>  +#endif
> --

If ou want to restrict it to powerpc64, you'd have to put a file into
sysdeps/unix/sysv/linux/powerpc/powerpc64/semop.c with essentially this:

int
semop (int semid, struct sembuf *sops, size_t nsops)
{
  /* POWER does not support the semop system call even with
     __ASSUME_DIRECT_SYSVIPC_SYSCALLS.  */
  return INLINE_SYSCALL_CALL (ipc, IPCOP_semop, semid, nsops, 0, sops);
}

You can't write an __ASSUME check yet because we don't know the correct
kernel version yet.

Thanks,
Florian
  

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
  

On 10/10/2019 13:35, Paul Clarke wrote:
> 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

I am sure this command gives you the correct answer, afaik this command
gives you the first tag containing the commit:

$ git describe --contains 0d6040d4681735dfc47
v5.1-rc1~160^2~3^2~4

In fact a simple test to check if 'semget' is present at the system call
table shows:

$ git checkout v5.0
[...]
HEAD is now at 1c163f4c7b3f Linux 5.0
$ grep semget arch/powerpc/kernel/syscalls/syscall.tbl
$ 

> --
> 
>> 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?

The issue is assuming __NR_semop is wire-up on 5.x and you build glibc
with 5.y headers (with x >= y) along with --enable-kernel=5.0.0.  The
__NR_op will be defines and the wire-up path will be always used.  This
will make semop fail on kernels 5.0 to 5.(x-1).

> 
>> 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?

The fallback ipc call will be used instead, which always works with
minimum kernel version supported (3.2).

> 
>> #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
> 

I am working on fixing this on my patchset, it should enable the new wire-up
sysvipc interface on all architectures that supports it (instead of just
powerpc).
  

On 10/10/2019 13:49, Adhemerval Zanella wrote:
> 
> 
> On 10/10/2019 13:35, Paul Clarke wrote:
>> On 10/10/19 10:13 AM, Adhemerval Zanella wrote:
> 
> I am sure this command gives you the correct answer, afaik this command

I am *not* sure [...]
  

On 10/10/19 11:49 AM, Adhemerval Zanella wrote:
> On 10/10/2019 13:35, Paul Clarke wrote:
>> 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
> 
> I am [not] sure this command gives you the correct answer, afaik this command
> gives you the first tag containing the commit:
> 
> $ git describe --contains 0d6040d4681735dfc47
> v5.1-rc1~160^2~3^2~4
> 
> In fact a simple test to check if 'semget' is present at the system call
> table shows:
> 
> $ git checkout v5.0
> [...]
> HEAD is now at 1c163f4c7b3f Linux 5.0
> $ grep semget arch/powerpc/kernel/syscalls/syscall.tbl
> $ 

Ack.  My git-fu is still developing...

PC
  

On Thu, Oct 10, 2019 at 6:18 PM Paul Clarke <pc@us.ibm.com> wrote:
> On 10/10/19 1:07 AM, Florian Weimer wrote:

> > 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.
>
> The same historical thread referenced above started with a patch [2] from 2015 to realize the same effect. In that thread, Michael Ellerman and Arnd Bergman discuss reverting the original support patches from Linux 4.4, which don't appear again, it seems, until Linux 5.0.  But, it seems that "semop" was not included in that patchset (Linux kernel commit 0d6040d468173).  Is that expected, Arnd?

Yes, that was intentional: as semop is a trivial subset of
semtimedop/semtimedop_time64,
it seemed pointless to add both, as I wrote in the changeset text then:

   "I'm not adding the new semtimedop() or semop() on 32-bit architectures,
    those will get implemented using the new semtimedop_time64() version
    that gets added along with the other time64 calls.
    Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop()."

My expectation then was that glibc would make its semop() interface a
wrapper around its own semtimedop() interface, which needs to be
implemented anyway. That implementation would then be shared across
all architectures.

However, I also did not expect any existing C library to migrate from the
existing ipc() syscall to the direct calls unless it requires linux-5.1
kernels, which I guess would be 2025 at the earliest for architectures
already supported in glibc.

In particular, the decision to drop the IPC_64 flag was intended to
simplify a libc implementation that only cares about the new syscalls
by limiting the special-case architectures to alpha, arm32, microblaze,
mips, sparc and xtensa.  If you need to deal with an architecture
having different IPC_64 semantics for direct and indirect syscalls,
that clearly adds complexity instead of reducing it as I had hoped.

      Arnd
  

On Thu, 10 Oct 2019, Adhemerval Zanella wrote:

> As you have pointed out, tying up wire-up semop with __NR_semop
> definition might indeed create a wrong build.  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;

If __ASSUME_DIRECT_SYSVIPC_SYSCALLS does not imply presence of the semop 
syscall, that should be made clear in the comment on the default 
definition.  If in addition the semop syscall might be added on 
architectures that do define __ASSUME_DIRECT_SYSVIPC_SYSCALLS but don't 
currently define semop, to me that indicates a separate 
__ASSUME_SEMOP_SYSCALL is needed.  (Note how we have lots of separate 
__ASSUME_* macros for socket syscalls, which avoids such complications in 
using a macro that's supposed to relate to lots of syscalls that aren't 
all present together.)
  

* Arnd Bergmann:

> On Thu, Oct 10, 2019 at 6:18 PM Paul Clarke <pc@us.ibm.com> wrote:
>> On 10/10/19 1:07 AM, Florian Weimer wrote:
>
>> > 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.
>>
>> The same historical thread referenced above started with a patch [2] from 2015 to realize the same effect. In that thread, Michael Ellerman and Arnd Bergman discuss reverting the original support patches from Linux 4.4, which don't appear again, it seems, until Linux 5.0.  But, it seems that "semop" was not included in that patchset (Linux kernel commit 0d6040d468173).  Is that expected, Arnd?
>
> Yes, that was intentional: as semop is a trivial subset of
> semtimedop/semtimedop_time64,

It's not so trivial anymore because you need to pick the right syscall
to replace it. 8-/

> it seemed pointless to add both, as I wrote in the changeset text then:
>
>    "I'm not adding the new semtimedop() or semop() on 32-bit architectures,
>     those will get implemented using the new semtimedop_time64() version
>     that gets added along with the other time64 calls.
>     Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop()."
>
> My expectation then was that glibc would make its semop() interface a
> wrapper around its own semtimedop() interface, which needs to be
> implemented anyway. That implementation would then be shared across
> all architectures.

Thanks for this background.  This suggests to me that with
__ASSUME_DIRECT_SYSVIPC_SYSCALLS, the generic implementation should use
semtimedop or semtimedop_time64, and that in the interim, Paul's patch
(with a commont explaining why) is safe with regards future evolution of
the kernel.

Thanks,
Florian
  

On 11/10/2019 06:19, Florian Weimer wrote:
> * Arnd Bergmann:
> 
>> On Thu, Oct 10, 2019 at 6:18 PM Paul Clarke <pc@us.ibm.com> wrote:
>>> On 10/10/19 1:07 AM, Florian Weimer wrote:
>>
>>>> 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.
>>>
>>> The same historical thread referenced above started with a patch [2] from 2015 to realize the same effect. In that thread, Michael Ellerman and Arnd Bergman discuss reverting the original support patches from Linux 4.4, which don't appear again, it seems, until Linux 5.0.  But, it seems that "semop" was not included in that patchset (Linux kernel commit 0d6040d468173).  Is that expected, Arnd?
>>
>> Yes, that was intentional: as semop is a trivial subset of
>> semtimedop/semtimedop_time64,
> 
> It's not so trivial anymore because you need to pick the right syscall
> to replace it. 8-/
> 
>> it seemed pointless to add both, as I wrote in the changeset text then:
>>
>>    "I'm not adding the new semtimedop() or semop() on 32-bit architectures,
>>     those will get implemented using the new semtimedop_time64() version
>>     that gets added along with the other time64 calls.
>>     Three 64-bit architectures (powerpc, s390 and sparc) get semtimedop()."
>>
>> My expectation then was that glibc would make its semop() interface a
>> wrapper around its own semtimedop() interface, which needs to be
>> implemented anyway. That implementation would then be shared across
>> all architectures.
> 
> Thanks for this background.  This suggests to me that with
> __ASSUME_DIRECT_SYSVIPC_SYSCALLS, the generic implementation should use
> semtimedop or semtimedop_time64, and that in the interim, Paul's patch
> (with a commont explaining why) is safe with regards future evolution of
> the kernel.
I think it would simpler then to just refactor semop to call semtimedop,
so the wire-up support only will need to touch semtimedop code.  In any
case I will send a updated version on my previous proposal (which is
similar of what Paul sent, but with wire-up support for affected
architectures).