[3/5] sysvipc: Consolidate semtimedop s390
Commit Message
This patch consolidates the s390-32 semtimedop implementation by defining
a arch-specific SEMTIMEDOP_IPC_ARGS to rearrange the arguments expected
by s390 Linux kABI. The idea is to avoid have multiples semtimedop
implementation changes for Linux v5.1 change to enable wire-up sysvipc
support.
Checked with a s390-linux-gnu and checking that resulting semtimedop
objects do not change.
* sysdeps/unix/sysv/linux/ipc_priv.h (SEMTIMEDOP_IPC_ARGS): New
define.
* sysdpes/unix/sysv/linux/s390/s390-32/ipc_priv.h: New file.
* sysdeps/unix/sysv/linux/s390/semtimedop.c: Remove file.
* sysdeps/unix/sysv/linux/semtimedop.c (semtimedop): Use
SEMTIMEDOP_IPC_ARGS for calls with __NR_ipc.
---
sysdeps/unix/sysv/linux/ipc_priv.h | 3 ++
.../unix/sysv/linux/s390/s390-32/ipc_priv.h | 29 +++++++++++++++
sysdeps/unix/sysv/linux/s390/semtimedop.c | 36 -------------------
sysdeps/unix/sysv/linux/semtimedop.c | 4 +--
4 files changed, 34 insertions(+), 38 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h
delete mode 100644 sysdeps/unix/sysv/linux/s390/semtimedop.c
Comments
* Adhemerval Zanella:
> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
> index 65adbb093e..49018c1b28 100644
> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
> @@ -34,4 +34,7 @@ struct __old_ipc_perm
> #define MSGRCV_ARGS(__msgp, __msgtyp) \
> ((long int []){ (long int) __msgp, __msgtyp })
>
> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> + (__nsops), 0, (__sops), (__timeout)
Maybe add a reference to the s390 version?
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h b/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h
> new file mode 100644
> index 0000000000..9d74e92289
> +/* The s390 sys_ipc variant has only five parameters instead of six
> + (as for default variant) and the only difference is the handling of
> + SEMTIMEDOP where on s390 the third parameter is used as a pointer
> + to a struct timespec where the generic variant uses fifth parameter. */
> +#undef SEMTIMEDOP_IPC_ARGS
> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> + (__nsops), (__timeout), (__sops)
I find “and the only difference is” a bit confusing here. Maybe write
“. The difference is” instead?
Rest of the patch looks okay to me.
Thanks,
Florian
On 16/05/2019 12:41, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
>> index 65adbb093e..49018c1b28 100644
>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
>> @@ -34,4 +34,7 @@ struct __old_ipc_perm
>> #define MSGRCV_ARGS(__msgp, __msgtyp) \
>> ((long int []){ (long int) __msgp, __msgtyp })
>>
>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
>> + (__nsops), 0, (__sops), (__timeout)
>
> Maybe add a reference to the s390 version?
What about:
/* This macro is required to handle the s390 variants, which passes the
arguments in a different order than default. */
>
>> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h b/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h
>> new file mode 100644
>> index 0000000000..9d74e92289
>
>> +/* The s390 sys_ipc variant has only five parameters instead of six
>> + (as for default variant) and the only difference is the handling of
>> + SEMTIMEDOP where on s390 the third parameter is used as a pointer
>> + to a struct timespec where the generic variant uses fifth parameter. */
>> +#undef SEMTIMEDOP_IPC_ARGS
>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
>> + (__nsops), (__timeout), (__sops)
>
> I find “and the only difference is” a bit confusing here. Maybe write
> “. The difference is” instead?
Ack, I change it locally.
>
> Rest of the patch looks okay to me.
>
> Thanks,
> Florian
>
* Adhemerval Zanella:
> On 16/05/2019 12:41, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
>>> index 65adbb093e..49018c1b28 100644
>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm
>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \
>>> ((long int []){ (long int) __msgp, __msgtyp })
>>>
>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
>>> + (__nsops), 0, (__sops), (__timeout)
>>
>> Maybe add a reference to the s390 version?
>
> What about:
>
> /* This macro is required to handle the s390 variants, which passes the
> arguments in a different order than default. */
Isn't this for s390 (31-bit) only?
Thanks,
Florian
> Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto:
>
> * Adhemerval Zanella:
>
>>> On 16/05/2019 12:41, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
>>>> index 65adbb093e..49018c1b28 100644
>>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
>>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
>>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm
>>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \
>>>> ((long int []){ (long int) __msgp, __msgtyp })
>>>>
>>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
>>>> + (__nsops), 0, (__sops), (__timeout)
>>>
>>> Maybe add a reference to the s390 version?
>>
>> What about:
>>
>> /* This macro is required to handle the s390 variants, which passes the
>> arguments in a different order than default. */
>
> Isn't this for s390 (31-bit) only?
>
Indeed, I will change to “s390-32 variant”.
> Thanks,
> Florian
* Adhemerval Zanella:
>> Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto:
>>
>> * Adhemerval Zanella:
>>
>>>> On 16/05/2019 12:41, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
>>>>> index 65adbb093e..49018c1b28 100644
>>>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
>>>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
>>>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm
>>>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \
>>>>> ((long int []){ (long int) __msgp, __msgtyp })
>>>>>
>>>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
>>>>> + (__nsops), 0, (__sops), (__timeout)
>>>>
>>>> Maybe add a reference to the s390 version?
>>>
>>> What about:
>>>
>>> /* This macro is required to handle the s390 variants, which passes the
>>> arguments in a different order than default. */
>>
>> Isn't this for s390 (31-bit) only?
>>
>
> Indeed, I will change to “s390-32 variant”.
Thanks, looks good to me.
Florian
On Fri, May 17, 2019 at 2:30 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> > Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto:
> >
> > * Adhemerval Zanella:
> >
> >>> On 16/05/2019 12:41, Florian Weimer wrote:
> >>> * Adhemerval Zanella:
> >>>
> >>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
> >>>> index 65adbb093e..49018c1b28 100644
> >>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
> >>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
> >>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm
> >>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \
> >>>> ((long int []){ (long int) __msgp, __msgtyp })
> >>>>
> >>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
> >>>> + (__nsops), 0, (__sops), (__timeout)
> >>>
> >>> Maybe add a reference to the s390 version?
> >>
> >> What about:
> >>
> >> /* This macro is required to handle the s390 variants, which passes the
> >> arguments in a different order than default. */
> >
> > Isn't this for s390 (31-bit) only?
> >
>
> Indeed, I will change to “s390-32 variant”.
According to the kernel sources, the odd sys_s390_ipc function is
used in both native 64-bit and compat 32-bit variants. The 64-bit
s390 ABI also started out with a maximum of five arguments,
so the wrapper was kept there, see:
arch/s390/kernel/syscalls/syscall.tbl:117 common ipc
sys_s390_ipc compat_sys_s390_ipc
Arnd
On 17/05/2019 16:09, Arnd Bergmann wrote:
> On Fri, May 17, 2019 at 2:30 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>> Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto:
>>>
>>> * Adhemerval Zanella:
>>>
>>>>> On 16/05/2019 12:41, Florian Weimer wrote:
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
>>>>>> index 65adbb093e..49018c1b28 100644
>>>>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
>>>>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
>>>>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm
>>>>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \
>>>>>> ((long int []){ (long int) __msgp, __msgtyp })
>>>>>>
>>>>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
>>>>>> + (__nsops), 0, (__sops), (__timeout)
>>>>>
>>>>> Maybe add a reference to the s390 version?
>>>>
>>>> What about:
>>>>
>>>> /* This macro is required to handle the s390 variants, which passes the
>>>> arguments in a different order than default. */
>>>
>>> Isn't this for s390 (31-bit) only?
>>>
>>
>> Indeed, I will change to “s390-32 variant”.
>
> According to the kernel sources, the odd sys_s390_ipc function is
> used in both native 64-bit and compat 32-bit variants. The 64-bit
> s390 ABI also started out with a maximum of five arguments,
> so the wrapper was kept there, see:
>
> arch/s390/kernel/syscalls/syscall.tbl:117 common ipc
> sys_s390_ipc compat_sys_s390_ipc
>
Thanks for remind me this is not for 31-bit only. I moved the
sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h to
sysdeps/unix/sysv/linux/s390/ipc_priv.h.
@@ -34,4 +34,7 @@ struct __old_ipc_perm
#define MSGRCV_ARGS(__msgp, __msgtyp) \
((long int []){ (long int) __msgp, __msgtyp })
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+ (__nsops), 0, (__sops), (__timeout)
+
#include <ipc_ops.h>
new file mode 100644
@@ -0,0 +1,29 @@
+/* Arch-specific SysV IPC definitions for Linux. s390 version.
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <sysdeps/unix/sysv/linux/ipc_priv.h>
+
+/* The s390 sys_ipc variant has only five parameters instead of six
+ (as for default variant) and the only difference is the handling of
+ SEMTIMEDOP where on s390 the third parameter is used as a pointer
+ to a struct timespec where the generic variant uses fifth parameter. */
+#undef SEMTIMEDOP_IPC_ARGS
+#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \
+ (__nsops), (__timeout), (__sops)
+
+#include <ipc_ops.h>
deleted file mode 100644
@@ -1,36 +0,0 @@
-/* Copyright (C) 2003-2019 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <http://www.gnu.org/licenses/>. */
-
-#include <sys/sem.h>
-#include <ipc_priv.h>
-#include <sysdep.h>
-#include <errno.h>
-
-/* Perform user-defined atomical operation of array of semaphores. */
-
-int
-semtimedop (int semid, struct sembuf *sops, size_t nsops,
- const struct timespec *timeout)
-{
- /* The s390 sys_ipc variant has only five parameters instead of six
- (as for default variant) and the only difference is the handling of
- SEMTIMEDOP where on s390 the third parameter is used as a pointer
- to a struct timespec where the generic variant uses fifth parameter. */
- return INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, nsops, timeout,
- sops);
-}
@@ -30,7 +30,7 @@ semtimedop (int semid, struct sembuf *sops, size_t nsops,
#ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
return INLINE_SYSCALL_CALL (semtimedop, semid, sops, nsops, timeout);
#else
- return INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, nsops, 0, sops,
- timeout);
+ return INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid,
+ SEMTIMEDOP_IPC_ARGS (nsops, sops, timeout));
#endif
}