sysv: linux: Pass 64-bit version of semctl syscall
Commit Message
Adjust the semctl syscall to match what the kernel expects. That is pass
a version with a *_high version of sem_otime and sem_ctime in the order
that the kernel expects.
---
include/sys/sem.h | 35 +++++++++++++++++++
sysdeps/unix/sysv/linux/bits/sem-pad.h | 1 +
sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h | 1 +
sysdeps/unix/sysv/linux/mips/bits/sem-pad.h | 2 ++
.../unix/sysv/linux/powerpc/bits/sem-pad.h | 2 ++
sysdeps/unix/sysv/linux/semctl.c | 30 +++++++++++++---
sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h | 2 ++
sysdeps/unix/sysv/linux/x86/bits/sem-pad.h | 1 +
8 files changed, 70 insertions(+), 4 deletions(-)
Comments
On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Adjust the semctl syscall to match what the kernel expects. That is pass
> a version with a *_high version of sem_otime and sem_ctime in the order
> that the kernel expects.
> +# ifdef __SEMID_DS_HIGH
> +# if defined (__SEMID_DS_HIGH_END)
...
> +# elif defined (__SEMID_DS_HIGH_SWAP)
The three new macros you check for here directly correspond to the
possible combinations of the existing __SEM_PAD_AFTER_TIME
and __SEM_PAD_BEFORE_TIME macros, right?
Maybe just use those directly?
Arnd
On Fri, Jan 31, 2020 at 1:54 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Adjust the semctl syscall to match what the kernel expects. That is pass
> > a version with a *_high version of sem_otime and sem_ctime in the order
> > that the kernel expects.
>
> > +# ifdef __SEMID_DS_HIGH
> > +# if defined (__SEMID_DS_HIGH_END)
> ...
> > +# elif defined (__SEMID_DS_HIGH_SWAP)
>
> The three new macros you check for here directly correspond to the
> possible combinations of the existing __SEM_PAD_AFTER_TIME
> and __SEM_PAD_BEFORE_TIME macros, right?
Hey Arnd,
I think you are asking why we don't just use the existing macros
instead of adding more (and keep the core logic as is). Besides the
naming being confusing then, they two types don't line up. For
example:
PowerPC has this:
#define __SEM_PAD_AFTER_TIME 0
#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
while HPPA has this:
#define __SEM_PAD_AFTER_TIME 0
#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
They have different __SEMID_DS macros, but the same __SEM_PAD macros.
Alistair
>
> Maybe just use those directly?
>
> Arnd
On Sat, Feb 1, 2020 at 12:59 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Jan 31, 2020 at 1:54 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> > >
> > > Adjust the semctl syscall to match what the kernel expects. That is pass
> > > a version with a *_high version of sem_otime and sem_ctime in the order
> > > that the kernel expects.
> >
> > > +# ifdef __SEMID_DS_HIGH
> > > +# if defined (__SEMID_DS_HIGH_END)
> > ...
> > > +# elif defined (__SEMID_DS_HIGH_SWAP)
> >
> > The three new macros you check for here directly correspond to the
> > possible combinations of the existing __SEM_PAD_AFTER_TIME
> > and __SEM_PAD_BEFORE_TIME macros, right?
>
> Hey Arnd,
>
> I think you are asking why we don't just use the existing macros
> instead of adding more (and keep the core logic as is). Besides the
> naming being confusing then, they two types don't line up. For
> example:
>
> PowerPC has this:
>
> #define __SEM_PAD_AFTER_TIME 0
> #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
>
> while HPPA has this:
>
> #define __SEM_PAD_AFTER_TIME 0
> #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
>
> They have different __SEMID_DS macros, but the same __SEM_PAD macros.
If they don't line up, that probably means that one of them is wrong ;-)
It looks like the HPPA version to also define __SEMID_DS_HIGH_SWAP.
Arnd
Hi Alistair,
> Adjust the semctl syscall to match what the kernel expects. That is
> pass a version with a *_high version of sem_otime and sem_ctime in
> the order that the kernel expects.
The change seems to be not a trivial one. Would it be possible to
elaborate the commit message a bit?
> ---
> include/sys/sem.h | 35
> +++++++++++++++++++ sysdeps/unix/sysv/linux/bits/sem-pad.h |
> 1 + sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h | 1 +
> sysdeps/unix/sysv/linux/mips/bits/sem-pad.h | 2 ++
> .../unix/sysv/linux/powerpc/bits/sem-pad.h | 2 ++
> sysdeps/unix/sysv/linux/semctl.c | 30 +++++++++++++---
> sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h | 2 ++
> sysdeps/unix/sysv/linux/x86/bits/sem-pad.h | 1 +
> 8 files changed, 70 insertions(+), 4 deletions(-)
>
> diff --git a/include/sys/sem.h b/include/sys/sem.h
> index 69fdf1f752..70b83127cb 100644
> --- a/include/sys/sem.h
> +++ b/include/sys/sem.h
> @@ -5,5 +5,40 @@
>
> __typeof__ (semtimedop) __semtimedop attribute_hidden;
>
> +# endif
> +
> +# ifdef __SEMID_DS_HIGH
> +# if defined (__SEMID_DS_HIGH_END)
> +struct __semid_ds32 {
> + struct ipc_perm sem_perm; /* permissions .. see ipc.h
> */
> + __syscall_ulong_t sem_otime; /* last semop time */
> + __syscall_ulong_t sem_ctime; /* last change time */
> + __syscall_ulong_t sem_nsems; /* no. of semaphores in
> array */
> + __syscall_ulong_t sem_otime_high;
> + __syscall_ulong_t sem_ctime_high;
> +};
> +# elif defined (__SEMID_DS_HIGH_SWAP)
> +struct __semid_ds32 {
> + struct ipc_perm sem_perm; /* operation permission
> struct */
> + __syscall_ulong_t sem_otime_high; /* last semop() time high */
> + __syscall_ulong_t sem_otime; /* last semop() time */
> + __syscall_ulong_t sem_ctime_high; /* last time changed by
> semctl() high */
> + __syscall_ulong_t sem_ctime; /* last time changed by
> semctl() */
> + __syscall_ulong_t sem_nsems; /* number of semaphores in
> set */
> + __syscall_ulong_t __glibc_reserved3;
> + __syscall_ulong_t __glibc_reserved4;
> +};
> +# else
> +struct __semid_ds32 {
> + struct ipc_perm sem_perm; /* operation permission
> struct */
> + __syscall_ulong_t sem_otime; /* last semop() time */
> + __syscall_ulong_t sem_otime_high; /* last semop() time high */
> + __syscall_ulong_t sem_ctime; /* last time changed by
> semctl() */
> + __syscall_ulong_t sem_ctime_high; /* last time changed by
> semctl() high */
> + __syscall_ulong_t sem_nsems; /* number of semaphores in
> set */
> + __syscall_ulong_t __glibc_reserved3;
> + __syscall_ulong_t __glibc_reserved4;
> +};
> +# endif
> # endif
> #endif
> diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> b/sysdeps/unix/sysv/linux/bits/sem-pad.h index 566ce039cc..18235e56df
> 100644 --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> @@ -31,3 +31,4 @@
>
> #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h index
> ee0332325b..7e112018a6 100644 ---
> a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h +++
> b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h @@ -24,3 +24,4 @@
>
> #define __SEM_PAD_AFTER_TIME 0
> #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h index
> 4c581f7694..12f2bd9c62 100644 ---
> a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h +++
> b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h @@ -22,3 +22,5 @@
>
> #define __SEM_PAD_AFTER_TIME 0
> #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h index
> 42d8827906..a13ca5f6bd 100644 ---
> a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h +++
> b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h @@ -24,3 +24,5 @@
>
> #define __SEM_PAD_AFTER_TIME 0
> #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/semctl.c
> b/sysdeps/unix/sysv/linux/semctl.c index 0c3eb0932f..34d3125fc3 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -28,6 +28,7 @@ union semun
> {
> int val; /* value for SETVAL */
> struct semid_ds *buf; /* buffer for IPC_STAT &
> IPC_SET */
> + struct __semid_ds32 *buf32;
> unsigned short int *array; /* array for GETALL & SETALL */
> struct seminfo *__buf; /* buffer for IPC_INFO */
> };
> @@ -43,12 +44,33 @@ union semun
> static int
> semctl_syscall (int semid, int semnum, int cmd, union semun arg)
> {
> -#ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> - arg.array);
> +#ifdef __SEMID_DS_HIGH
> + int ret;
> +# ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> + arg.array);
> +# else
> + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd |
> __IPC_64,
> + SEMCTL_ARG_ADDRESS (arg));
> +# endif
> +/* If we aren't going to overflow the time_t sem_ctime/sem_otime set
> it */ +# if __TIMESIZE == 64 && __WORDSIZE == 32
I guess that this is for RV32? Please correct me if I'm wrong, but
there shouldn't be need any extra code to support ARM32 without Y2038
support?
> + if (ret == 0)
> + {
> + arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t)
> arg.buf32->sem_ctime_high << 32);
> + arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t)
> arg.buf32->sem_otime_high << 32);
> + }
> +# endif
> + return ret;
> #else
> +# ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> + return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> + arg.array);
> +# else
> return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd
> | __IPC_64,
> - SEMCTL_ARG_ADDRESS (arg));
> + SEMCTL_ARG_ADDRESS (arg));
> +# endif
> +
> #endif
> }
>
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h index
> 5f4e214d12..79655b8149 100644 ---
> a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h +++
> b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h @@ -24,3 +24,5 @@
>
> #define __SEM_PAD_AFTER_TIME 0
> #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h index
> 102e226997..db397857e7 100644 ---
> a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h +++
> b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h @@ -22,3 +22,4 @@
>
> #define __SEM_PAD_AFTER_TIME 1
> #define __SEM_PAD_BEFORE_TIME 0
> +#define __SEMID_DS_HIGH 1
I do guess that this is a unification for existing archs/ports (with no
support for Y2038).
Just informative - there is already available semtimedop_time64 in
Linux 5.1+, which seems to be tricky to support.
(There was a discussion previously about syscalls not eligible for
__ASSUME_TIME64_SYSCALLS flag and semtimedop was one of them).
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Fri, Jan 31, 2020 at 11:58 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Feb 1, 2020 at 12:59 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Fri, Jan 31, 2020 at 1:54 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
> > > <alistair.francis@wdc.com> wrote:
> > > >
> > > > Adjust the semctl syscall to match what the kernel expects. That is pass
> > > > a version with a *_high version of sem_otime and sem_ctime in the order
> > > > that the kernel expects.
> > >
> > > > +# ifdef __SEMID_DS_HIGH
> > > > +# if defined (__SEMID_DS_HIGH_END)
> > > ...
> > > > +# elif defined (__SEMID_DS_HIGH_SWAP)
> > >
> > > The three new macros you check for here directly correspond to the
> > > possible combinations of the existing __SEM_PAD_AFTER_TIME
> > > and __SEM_PAD_BEFORE_TIME macros, right?
> >
> > Hey Arnd,
> >
> > I think you are asking why we don't just use the existing macros
> > instead of adding more (and keep the core logic as is). Besides the
> > naming being confusing then, they two types don't line up. For
> > example:
> >
> > PowerPC has this:
> >
> > #define __SEM_PAD_AFTER_TIME 0
> > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> >
> > while HPPA has this:
> >
> > #define __SEM_PAD_AFTER_TIME 0
> > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> >
> > They have different __SEMID_DS macros, but the same __SEM_PAD macros.
>
> If they don't line up, that probably means that one of them is wrong ;-)
Ha, ok I'll double check and update it.
Even still, I would rather not use the existing macros as they use
__TIMESIZE == 32 and the new ones are using __WORDSIZE == 32. We could
of course just have a RV32 override that fixes it for RV32, but then
we need another override.
The main reason not to is that I think it would be confusing to
overload the macro like that and I don't see a disadvantage in having
a new macro. It makes what is going on (which is already somewhat
confusing) more clear.
If everyone really disagrees I can use the existing ones.
Alistair
>
> It looks like the HPPA version to also define __SEMID_DS_HIGH_SWAP.
>
> Arnd
On Sat, Feb 1, 2020 at 8:57 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Fri, Jan 31, 2020 at 11:58 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sat, Feb 1, 2020 at 12:59 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > On Fri, Jan 31, 2020 at 1:54 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
> > > > <alistair.francis@wdc.com> wrote:
> > > > >
> > > > > Adjust the semctl syscall to match what the kernel expects. That is pass
> > > > > a version with a *_high version of sem_otime and sem_ctime in the order
> > > > > that the kernel expects.
> > > >
> > > > > +# ifdef __SEMID_DS_HIGH
> > > > > +# if defined (__SEMID_DS_HIGH_END)
> > > > ...
> > > > > +# elif defined (__SEMID_DS_HIGH_SWAP)
> > > >
> > > > The three new macros you check for here directly correspond to the
> > > > possible combinations of the existing __SEM_PAD_AFTER_TIME
> > > > and __SEM_PAD_BEFORE_TIME macros, right?
> > >
> > > Hey Arnd,
> > >
> > > I think you are asking why we don't just use the existing macros
> > > instead of adding more (and keep the core logic as is). Besides the
> > > naming being confusing then, they two types don't line up. For
> > > example:
> > >
> > > PowerPC has this:
> > >
> > > #define __SEM_PAD_AFTER_TIME 0
> > > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > >
> > > while HPPA has this:
> > >
> > > #define __SEM_PAD_AFTER_TIME 0
> > > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > >
> > > They have different __SEMID_DS macros, but the same __SEM_PAD macros.
> >
> > If they don't line up, that probably means that one of them is wrong ;-)
>
> Ha, ok I'll double check and update it.
>
> Even still, I would rather not use the existing macros as they use
> __TIMESIZE == 32 and the new ones are using __WORDSIZE == 32.
Ah, I had not noticed that difference.
> We could of course just have a RV32 override that fixes it for RV32, but
> then we need another override.
>
> The main reason not to is that I think it would be confusing to
> overload the macro like that and I don't see a disadvantage in having
> a new macro. It makes what is going on (which is already somewhat
> confusing) more clear.
Makes sense.
Another idea would be to not define a separate struct at all, except for
mips (which needs special handling anyway), and then define a
new macro fro the few targets that do need to swap the one word
with __TIMESIZE==64: s390-32, arm/be, sh/be, m68k and microblaze/be
and do nothing on the others.
Arnd
On Sat, Feb 1, 2020 at 1:07 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Feb 1, 2020 at 8:57 PM Alistair Francis <alistair23@gmail.com> wrote:
> > On Fri, Jan 31, 2020 at 11:58 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Sat, Feb 1, 2020 at 12:59 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > > On Fri, Jan 31, 2020 at 1:54 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > >
> > > > > On Fri, Jan 31, 2020 at 6:11 PM Alistair Francis
> > > > > <alistair.francis@wdc.com> wrote:
> > > > > >
> > > > > > Adjust the semctl syscall to match what the kernel expects. That is pass
> > > > > > a version with a *_high version of sem_otime and sem_ctime in the order
> > > > > > that the kernel expects.
> > > > >
> > > > > > +# ifdef __SEMID_DS_HIGH
> > > > > > +# if defined (__SEMID_DS_HIGH_END)
> > > > > ...
> > > > > > +# elif defined (__SEMID_DS_HIGH_SWAP)
> > > > >
> > > > > The three new macros you check for here directly correspond to the
> > > > > possible combinations of the existing __SEM_PAD_AFTER_TIME
> > > > > and __SEM_PAD_BEFORE_TIME macros, right?
> > > >
> > > > Hey Arnd,
> > > >
> > > > I think you are asking why we don't just use the existing macros
> > > > instead of adding more (and keep the core logic as is). Besides the
> > > > naming being confusing then, they two types don't line up. For
> > > > example:
> > > >
> > > > PowerPC has this:
> > > >
> > > > #define __SEM_PAD_AFTER_TIME 0
> > > > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > > > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > > > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > > >
> > > > while HPPA has this:
> > > >
> > > > #define __SEM_PAD_AFTER_TIME 0
> > > > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > > > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > > >
> > > > They have different __SEMID_DS macros, but the same __SEM_PAD macros.
> > >
> > > If they don't line up, that probably means that one of them is wrong ;-)
> >
> > Ha, ok I'll double check and update it.
> >
> > Even still, I would rather not use the existing macros as they use
> > __TIMESIZE == 32 and the new ones are using __WORDSIZE == 32.
>
> Ah, I had not noticed that difference.
>
> > We could of course just have a RV32 override that fixes it for RV32, but
> > then we need another override.
> >
> > The main reason not to is that I think it would be confusing to
> > overload the macro like that and I don't see a disadvantage in having
> > a new macro. It makes what is going on (which is already somewhat
> > confusing) more clear.
>
> Makes sense.
>
> Another idea would be to not define a separate struct at all, except for
> mips (which needs special handling anyway), and then define a
> new macro fro the few targets that do need to swap the one word
> with __TIMESIZE==64: s390-32, arm/be, sh/be, m68k and microblaze/be
> and do nothing on the others.
Ah, that is a good idea. I still prefer this way as it means that MIPS
does the same thing as everyone else (just with a different
structure). It also makes it clearer to follow as the values are
always in the right place instead of being swapped around after the
syscall.
I'll fix up the HPPA bug and send a v2.
Alistair
>
> Arnd
On Sat, Feb 1, 2020 at 6:08 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > Adjust the semctl syscall to match what the kernel expects. That is
> > pass a version with a *_high version of sem_otime and sem_ctime in
> > the order that the kernel expects.
>
> The change seems to be not a trivial one. Would it be possible to
> elaborate the commit message a bit?
Yep, I have done this in v2.
>
> > ---
> > include/sys/sem.h | 35
> > +++++++++++++++++++ sysdeps/unix/sysv/linux/bits/sem-pad.h |
> > 1 + sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h | 1 +
> > sysdeps/unix/sysv/linux/mips/bits/sem-pad.h | 2 ++
> > .../unix/sysv/linux/powerpc/bits/sem-pad.h | 2 ++
> > sysdeps/unix/sysv/linux/semctl.c | 30 +++++++++++++---
> > sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h | 2 ++
> > sysdeps/unix/sysv/linux/x86/bits/sem-pad.h | 1 +
> > 8 files changed, 70 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/sys/sem.h b/include/sys/sem.h
> > index 69fdf1f752..70b83127cb 100644
> > --- a/include/sys/sem.h
> > +++ b/include/sys/sem.h
> > @@ -5,5 +5,40 @@
> >
> > __typeof__ (semtimedop) __semtimedop attribute_hidden;
> >
> > +# endif
> > +
> > +# ifdef __SEMID_DS_HIGH
> > +# if defined (__SEMID_DS_HIGH_END)
> > +struct __semid_ds32 {
> > + struct ipc_perm sem_perm; /* permissions .. see ipc.h
> > */
> > + __syscall_ulong_t sem_otime; /* last semop time */
> > + __syscall_ulong_t sem_ctime; /* last change time */
> > + __syscall_ulong_t sem_nsems; /* no. of semaphores in
> > array */
> > + __syscall_ulong_t sem_otime_high;
> > + __syscall_ulong_t sem_ctime_high;
> > +};
> > +# elif defined (__SEMID_DS_HIGH_SWAP)
> > +struct __semid_ds32 {
> > + struct ipc_perm sem_perm; /* operation permission
> > struct */
> > + __syscall_ulong_t sem_otime_high; /* last semop() time high */
> > + __syscall_ulong_t sem_otime; /* last semop() time */
> > + __syscall_ulong_t sem_ctime_high; /* last time changed by
> > semctl() high */
> > + __syscall_ulong_t sem_ctime; /* last time changed by
> > semctl() */
> > + __syscall_ulong_t sem_nsems; /* number of semaphores in
> > set */
> > + __syscall_ulong_t __glibc_reserved3;
> > + __syscall_ulong_t __glibc_reserved4;
> > +};
> > +# else
> > +struct __semid_ds32 {
> > + struct ipc_perm sem_perm; /* operation permission
> > struct */
> > + __syscall_ulong_t sem_otime; /* last semop() time */
> > + __syscall_ulong_t sem_otime_high; /* last semop() time high */
> > + __syscall_ulong_t sem_ctime; /* last time changed by
> > semctl() */
> > + __syscall_ulong_t sem_ctime_high; /* last time changed by
> > semctl() high */
> > + __syscall_ulong_t sem_nsems; /* number of semaphores in
> > set */
> > + __syscall_ulong_t __glibc_reserved3;
> > + __syscall_ulong_t __glibc_reserved4;
> > +};
> > +# endif
> > # endif
> > #endif
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > b/sysdeps/unix/sysv/linux/bits/sem-pad.h index 566ce039cc..18235e56df
> > 100644 --- a/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sem-pad.h
> > @@ -31,3 +31,4 @@
> >
> > #define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
> > #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h
> > b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h index
> > ee0332325b..7e112018a6 100644 ---
> > a/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h +++
> > b/sysdeps/unix/sysv/linux/hppa/bits/sem-pad.h @@ -24,3 +24,4 @@
> >
> > #define __SEM_PAD_AFTER_TIME 0
> > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h
> > b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h index
> > 4c581f7694..12f2bd9c62 100644 ---
> > a/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h +++
> > b/sysdeps/unix/sysv/linux/mips/bits/sem-pad.h @@ -22,3 +22,5 @@
> >
> > #define __SEM_PAD_AFTER_TIME 0
> > #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h
> > b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h index
> > 42d8827906..a13ca5f6bd 100644 ---
> > a/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h +++
> > b/sysdeps/unix/sysv/linux/powerpc/bits/sem-pad.h @@ -24,3 +24,5 @@
> >
> > #define __SEM_PAD_AFTER_TIME 0
> > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/semctl.c
> > b/sysdeps/unix/sysv/linux/semctl.c index 0c3eb0932f..34d3125fc3 100644
> > --- a/sysdeps/unix/sysv/linux/semctl.c
> > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > @@ -28,6 +28,7 @@ union semun
> > {
> > int val; /* value for SETVAL */
> > struct semid_ds *buf; /* buffer for IPC_STAT &
> > IPC_SET */
> > + struct __semid_ds32 *buf32;
> > unsigned short int *array; /* array for GETALL & SETALL */
> > struct seminfo *__buf; /* buffer for IPC_INFO */
> > };
> > @@ -43,12 +44,33 @@ union semun
> > static int
> > semctl_syscall (int semid, int semnum, int cmd, union semun arg)
> > {
> > -#ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> > - return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > - arg.array);
> > +#ifdef __SEMID_DS_HIGH
> > + int ret;
> > +# ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> > + ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > + arg.array);
> > +# else
> > + ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd |
> > __IPC_64,
> > + SEMCTL_ARG_ADDRESS (arg));
> > +# endif
> > +/* If we aren't going to overflow the time_t sem_ctime/sem_otime set
> > it */ +# if __TIMESIZE == 64 && __WORDSIZE == 32
>
> I guess that this is for RV32? Please correct me if I'm wrong, but
> there shouldn't be need any extra code to support ARM32 without Y2038
> support?
Without y2038 shouldn't be any different. I have tidied up the patch
in v2 so this is clearer.
>
> > + if (ret == 0)
> > + {
> > + arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t)
> > arg.buf32->sem_ctime_high << 32);
> > + arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t)
> > arg.buf32->sem_otime_high << 32);
> > + }
> > +# endif
> > + return ret;
> > #else
> > +# ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> > + return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > + arg.array);
> > +# else
> > return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd
> > | __IPC_64,
> > - SEMCTL_ARG_ADDRESS (arg));
> > + SEMCTL_ARG_ADDRESS (arg));
> > +# endif
> > +
> > #endif
> > }
> >
> > diff --git a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h
> > b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h index
> > 5f4e214d12..79655b8149 100644 ---
> > a/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h +++
> > b/sysdeps/unix/sysv/linux/sparc/bits/sem-pad.h @@ -24,3 +24,5 @@
> >
> > #define __SEM_PAD_AFTER_TIME 0
> > #define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
> > +#define __SEMID_DS_HIGH (__WORDSIZE == 32)
> > +#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h
> > b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h index
> > 102e226997..db397857e7 100644 ---
> > a/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h +++
> > b/sysdeps/unix/sysv/linux/x86/bits/sem-pad.h @@ -22,3 +22,4 @@
> >
> > #define __SEM_PAD_AFTER_TIME 1
> > #define __SEM_PAD_BEFORE_TIME 0
> > +#define __SEMID_DS_HIGH 1
>
> I do guess that this is a unification for existing archs/ports (with no
> support for Y2038).
Yep!
>
> Just informative - there is already available semtimedop_time64 in
> Linux 5.1+, which seems to be tricky to support.
> (There was a discussion previously about syscalls not eligible for
> __ASSUME_TIME64_SYSCALLS flag and semtimedop was one of them).
Thanks
Alistair
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
@@ -5,5 +5,40 @@
__typeof__ (semtimedop) __semtimedop attribute_hidden;
+# endif
+
+# ifdef __SEMID_DS_HIGH
+# if defined (__SEMID_DS_HIGH_END)
+struct __semid_ds32 {
+ struct ipc_perm sem_perm; /* permissions .. see ipc.h */
+ __syscall_ulong_t sem_otime; /* last semop time */
+ __syscall_ulong_t sem_ctime; /* last change time */
+ __syscall_ulong_t sem_nsems; /* no. of semaphores in array */
+ __syscall_ulong_t sem_otime_high;
+ __syscall_ulong_t sem_ctime_high;
+};
+# elif defined (__SEMID_DS_HIGH_SWAP)
+struct __semid_ds32 {
+ struct ipc_perm sem_perm; /* operation permission struct */
+ __syscall_ulong_t sem_otime_high; /* last semop() time high */
+ __syscall_ulong_t sem_otime; /* last semop() time */
+ __syscall_ulong_t sem_ctime_high; /* last time changed by semctl() high */
+ __syscall_ulong_t sem_ctime; /* last time changed by semctl() */
+ __syscall_ulong_t sem_nsems; /* number of semaphores in set */
+ __syscall_ulong_t __glibc_reserved3;
+ __syscall_ulong_t __glibc_reserved4;
+};
+# else
+struct __semid_ds32 {
+ struct ipc_perm sem_perm; /* operation permission struct */
+ __syscall_ulong_t sem_otime; /* last semop() time */
+ __syscall_ulong_t sem_otime_high; /* last semop() time high */
+ __syscall_ulong_t sem_ctime; /* last time changed by semctl() */
+ __syscall_ulong_t sem_ctime_high; /* last time changed by semctl() high */
+ __syscall_ulong_t sem_nsems; /* number of semaphores in set */
+ __syscall_ulong_t __glibc_reserved3;
+ __syscall_ulong_t __glibc_reserved4;
+};
+# endif
# endif
#endif
@@ -31,3 +31,4 @@
#define __SEM_PAD_AFTER_TIME (__TIMESIZE == 32)
#define __SEM_PAD_BEFORE_TIME 0
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
@@ -24,3 +24,4 @@
#define __SEM_PAD_AFTER_TIME 0
#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
@@ -22,3 +22,5 @@
#define __SEM_PAD_AFTER_TIME 0
#define __SEM_PAD_BEFORE_TIME 0
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_END (__WORDSIZE == 32)
@@ -24,3 +24,5 @@
#define __SEM_PAD_AFTER_TIME 0
#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
@@ -28,6 +28,7 @@ union semun
{
int val; /* value for SETVAL */
struct semid_ds *buf; /* buffer for IPC_STAT & IPC_SET */
+ struct __semid_ds32 *buf32;
unsigned short int *array; /* array for GETALL & SETALL */
struct seminfo *__buf; /* buffer for IPC_INFO */
};
@@ -43,12 +44,33 @@ union semun
static int
semctl_syscall (int semid, int semnum, int cmd, union semun arg)
{
-#ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
- return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
- arg.array);
+#ifdef __SEMID_DS_HIGH
+ int ret;
+# ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
+ ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
+ arg.array);
+# else
+ ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64,
+ SEMCTL_ARG_ADDRESS (arg));
+# endif
+/* If we aren't going to overflow the time_t sem_ctime/sem_otime set it */
+# if __TIMESIZE == 64 && __WORDSIZE == 32
+ if (ret == 0)
+ {
+ arg.buf->sem_ctime = arg.buf32->sem_ctime | ((time_t) arg.buf32->sem_ctime_high << 32);
+ arg.buf->sem_otime = arg.buf32->sem_otime | ((time_t) arg.buf32->sem_otime_high << 32);
+ }
+# endif
+ return ret;
#else
+# ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
+ return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
+ arg.array);
+# else
return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64,
- SEMCTL_ARG_ADDRESS (arg));
+ SEMCTL_ARG_ADDRESS (arg));
+# endif
+
#endif
}
@@ -24,3 +24,5 @@
#define __SEM_PAD_AFTER_TIME 0
#define __SEM_PAD_BEFORE_TIME (__TIMESIZE == 32)
+#define __SEMID_DS_HIGH (__WORDSIZE == 32)
+#define __SEMID_DS_HIGH_SWAP (__WORDSIZE == 32)
@@ -22,3 +22,4 @@
#define __SEM_PAD_AFTER_TIME 1
#define __SEM_PAD_BEFORE_TIME 0
+#define __SEMID_DS_HIGH 1