This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] sysv: linux: Pass 64-bit version of semctl syscall
- From: Alistair Francis <alistair23 at gmail dot com>
- To: Lukasz Majewski <lukma at denx dot de>
- Cc: Alistair Francis <alistair dot francis at wdc dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 3 Feb 2020 10:08:36 -0800
- Subject: Re: [PATCH] sysv: linux: Pass 64-bit version of semctl syscall
- References: <20200131170457.25952-1-alistair.francis@wdc.com> <20200201150851.362f404f@jawa>
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