This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [powerpc] Use DIRECT_SYSVIPC_SYSCALLS


* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]