> From: "Ted Unangst" <[email protected]>
> Date: Wed, 08 Jul 2020 05:20:23 -0400
>
> On 2020-07-08, Ted Unangst wrote:
> > I think this makes sem_init(pshared) work.
>
> Whoops, need more context to include the header file changes.
It is a bit of a pity that we have to expose the internals here, but I
don't see an easy way to avoid that, especially since hppa requires
16-byte alignment. At least <machine/spinlock.h> doesn't do any
namespace pollution as I believe a single underscore is enough here as
everything is in file scope?
This will require a libpthread major bump, and those are really
painful! So I'm not sure we should do this just for pshared
semaphores which hardly anybody uses.
I wonder if we do this, should we include some additional padding in
the struct for future expansion?
> Index: include/semaphore.h
> ===================================================================
> RCS file: /home/cvs/src/include/semaphore.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 semaphore.h
> --- include/semaphore.h 15 Oct 2017 23:40:33 -0000 1.1
> +++ include/semaphore.h 8 Jul 2020 08:47:06 -0000
> @@ -38,10 +38,16 @@
> #define _SEMAPHORE_H_
>
> #include <sys/cdefs.h>
> +#include <machine/spinlock.h>
>
> /* Opaque type definition. */
> -struct __sem;
> -typedef struct __sem *sem_t;
> +struct __sem {
> + _atomic_lock_t lock;
> + volatile int waitcount;
> + volatile int value;
> + int shared;
> +};
> +typedef struct __sem sem_t;
> struct timespec;
>
> #define SEM_FAILED ((sem_t *)0)
> Index: lib/libc/include/thread_private.h
> ===================================================================
> RCS file: /home/cvs/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.35
> diff -u -p -r1.35 thread_private.h
> --- lib/libc/include/thread_private.h 13 Feb 2019 13:22:14 -0000 1.35
> +++ lib/libc/include/thread_private.h 8 Jul 2020 08:44:45 -0000
> @@ -273,13 +273,6 @@ __END_HIDDEN_DECLS
>
> #define _SPINLOCK_UNLOCKED _ATOMIC_LOCK_UNLOCKED
>
> -struct __sem {
> - _atomic_lock_t lock;
> - volatile int waitcount;
> - volatile int value;
> - int shared;
> -};
> -
> TAILQ_HEAD(pthread_queue, pthread);
>
> #ifdef FUTEX
> Index: lib/librthread/rthread.h
> ===================================================================
> RCS file: /home/cvs/src/lib/librthread/rthread.h,v
> retrieving revision 1.64
> diff -u -p -r1.64 rthread.h
> --- lib/librthread/rthread.h 13 Feb 2019 13:22:14 -0000 1.64
> +++ lib/librthread/rthread.h 8 Jul 2020 09:18:34 -0000
> @@ -79,8 +79,8 @@ struct pthread_spinlock {
> (((size) + (_thread_pagesize - 1)) & ~(_thread_pagesize - 1))
>
> __BEGIN_HIDDEN_DECLS
> -int _sem_wait(sem_t, int, const struct timespec *, int *);
> -int _sem_post(sem_t);
> +int _sem_wait(sem_t *, int, const struct timespec *, int *);
> +int _sem_post(sem_t *);
>
> void _rthread_init(void);
> struct stack *_rthread_alloc_stack(pthread_t);
> Index: lib/librthread/rthread_sem.c
> ===================================================================
> RCS file: /home/cvs/src/lib/librthread/rthread_sem.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 rthread_sem.c
> --- lib/librthread/rthread_sem.c 6 Apr 2020 00:01:08 -0000 1.32
> +++ lib/librthread/rthread_sem.c 8 Jul 2020 09:18:34 -0000
> @@ -55,7 +55,7 @@
> * Internal implementation of semaphores
> */
> int
> -_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
> +_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
> int *delayed_cancel)
> {
> unsigned int val;
> @@ -86,7 +86,7 @@ _sem_wait(sem_t sem, int can_eintr, cons
>
> /* always increment count */
> int
> -_sem_post(sem_t sem)
> +_sem_post(sem_t *sem)
> {
> membar_exit_before_atomic();
> atomic_inc_int(&sem->value);
> @@ -98,70 +98,29 @@ _sem_post(sem_t sem)
> * exported semaphores
> */
> int
> -sem_init(sem_t *semp, int pshared, unsigned int value)
> +sem_init(sem_t *sem, int pshared, unsigned int value)
> {
> - sem_t sem;
>
> if (value > SEM_VALUE_MAX) {
> errno = EINVAL;
> return (-1);
> }
>
> - if (pshared) {
> - errno = EPERM;
> - return (-1);
> -#ifdef notyet
> - char name[SEM_RANDOM_NAME_LEN];
> - sem_t *sempshared;
> - int i;
> -
> - for (;;) {
> - for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
> - name[i] = arc4random_uniform(255) + 1;
> - name[SEM_RANDOM_NAME_LEN - 1] = '\0';
> - sempshared = sem_open(name, O_CREAT | O_EXCL, 0, value);
> - if (sempshared != SEM_FAILED)
> - break;
> - if (errno == EEXIST)
> - continue;
> - if (errno != EPERM)
> - errno = ENOSPC;
> - return (-1);
> - }
> -
> - /* unnamed semaphore should not be opened twice */
> - if (sem_unlink(name) == -1) {
> - sem_close(sempshared);
> - errno = ENOSPC;
> - return (-1);
> - }
> -
> - *semp = *sempshared;
> - free(sempshared);
> - return (0);
> -#endif
> - }
> -
> - sem = calloc(1, sizeof(*sem));
> - if (!sem) {
> - errno = ENOSPC;
> - return (-1);
> - }
> + memset(sem, 0, sizeof(*sem));
> sem->value = value;
> - *semp = sem;
> + sem->shared = pshared;
>
> return (0);
> }
>
> int
> -sem_destroy(sem_t *semp)
> +sem_destroy(sem_t *sem)
> {
> - sem_t sem;
>
> if (!_threads_ready) /* for SEM_MMAP_SIZE */
> _rthread_init();
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -174,21 +133,19 @@ sem_destroy(sem_t *semp)
> return (-1);
> }
>
> - *semp = NULL;
> if (sem->shared)
> munmap(sem, SEM_MMAP_SIZE);
> else
> - free(sem);
> + memset(sem, 0, sizeof(*sem));
>
> return (0);
> }
>
> int
> -sem_getvalue(sem_t *semp, int *sval)
> +sem_getvalue(sem_t *sem, int *sval)
> {
> - sem_t sem;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -199,11 +156,10 @@ sem_getvalue(sem_t *semp, int *sval)
> }
>
> int
> -sem_post(sem_t *semp)
> +sem_post(sem_t *sem)
> {
> - sem_t sem;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -214,11 +170,10 @@ sem_post(sem_t *semp)
> }
>
> int
> -sem_wait(sem_t *semp)
> +sem_wait(sem_t *sem)
> {
> struct tib *tib = TIB_GET();
> pthread_t self;
> - sem_t sem;
> int error;
> PREP_CANCEL_POINT(tib);
>
> @@ -226,7 +181,7 @@ sem_wait(sem_t *semp)
> _rthread_init();
> self = tib->tib_thread;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -246,15 +201,14 @@ sem_wait(sem_t *semp)
> }
>
> int
> -sem_timedwait(sem_t *semp, const struct timespec *abstime)
> +sem_timedwait(sem_t *sem, const struct timespec *abstime)
> {
> struct tib *tib = TIB_GET();
> pthread_t self;
> - sem_t sem;
> int error;
> PREP_CANCEL_POINT(tib);
>
> - if (!semp || !(sem = *semp) || abstime == NULL ||
> + if (!sem || abstime == NULL ||
> abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) {
> errno = EINVAL;
> return (-1);
> @@ -279,12 +233,11 @@ sem_timedwait(sem_t *semp, const struct
> }
>
> int
> -sem_trywait(sem_t *semp)
> +sem_trywait(sem_t *sem)
> {
> - sem_t sem;
> unsigned int val;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -316,7 +269,7 @@ sem_open(const char *name, int oflag, ..
> {
> char sempath[SEM_PATH_SIZE];
> struct stat sb;
> - sem_t sem, *semp;
> + sem_t *sem;
> unsigned int value = 0;
> int created = 0, fd;
>
> @@ -382,34 +335,24 @@ sem_open(const char *name, int oflag, ..
> errno = EINVAL;
> return (SEM_FAILED);
> }
> - semp = malloc(sizeof(*semp));
> - if (!semp) {
> - munmap(sem, SEM_MMAP_SIZE);
> - errno = ENOSPC;
> - return (SEM_FAILED);
> - }
> if (created) {
> sem->value = value;
> sem->shared = 1;
> }
> - *semp = sem;
>
> - return (semp);
> + return (sem);
> }
>
> int
> -sem_close(sem_t *semp)
> +sem_close(sem_t *sem)
> {
> - sem_t sem;
>
> - if (!semp || !(sem = *semp) || !sem->shared) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
>
> - *semp = NULL;
> munmap(sem, SEM_MMAP_SIZE);
> - free(semp);
>
> return (0);
> }
> Index: lib/librthread/rthread_sem_compat.c
> ===================================================================
> RCS file: /home/cvs/src/lib/librthread/rthread_sem_compat.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 rthread_sem_compat.c
> --- lib/librthread/rthread_sem_compat.c 8 Jun 2018 13:53:01 -0000
> 1.1
> +++ lib/librthread/rthread_sem_compat.c 8 Jul 2020 09:18:35 -0000
> @@ -53,7 +53,7 @@
> * Internal implementation of semaphores
> */
> int
> -_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
> +_sem_wait(sem_t *sem, int can_eintr, const struct timespec *abstime,
> int *delayed_cancel)
> {
> void *ident = (void *)&sem->waitcount;
> @@ -87,7 +87,7 @@ _sem_wait(sem_t sem, int can_eintr, cons
>
> /* always increment count */
> int
> -_sem_post(sem_t sem)
> +_sem_post(sem_t *sem)
> {
> void *ident = (void *)&sem->waitcount;
> int rv = 0;
> @@ -109,71 +109,31 @@ _sem_post(sem_t sem)
> * exported semaphores
> */
> int
> -sem_init(sem_t *semp, int pshared, unsigned int value)
> +sem_init(sem_t *sem, int pshared, unsigned int value)
> {
> - sem_t sem;
>
> if (value > SEM_VALUE_MAX) {
> errno = EINVAL;
> return (-1);
> }
>
> - if (pshared) {
> - errno = EPERM;
> - return (-1);
> -#ifdef notyet
> - char name[SEM_RANDOM_NAME_LEN];
> - sem_t *sempshared;
> - int i;
> -
> - for (;;) {
> - for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
> - name[i] = arc4random_uniform(255) + 1;
> - name[SEM_RANDOM_NAME_LEN - 1] = '\0';
> - sempshared = sem_open(name, O_CREAT | O_EXCL, 0, value);
> - if (sempshared != SEM_FAILED)
> - break;
> - if (errno == EEXIST)
> - continue;
> - if (errno != EPERM)
> - errno = ENOSPC;
> - return (-1);
> - }
> -
> - /* unnamed semaphore should not be opened twice */
> - if (sem_unlink(name) == -1) {
> - sem_close(sempshared);
> - errno = ENOSPC;
> - return (-1);
> - }
>
> - *semp = *sempshared;
> - free(sempshared);
> - return (0);
> -#endif
> - }
> -
> - sem = calloc(1, sizeof(*sem));
> - if (!sem) {
> - errno = ENOSPC;
> - return (-1);
> - }
> + memset(sem, 0, sizeof(*sem));
> sem->lock = _SPINLOCK_UNLOCKED;
> sem->value = value;
> - *semp = sem;
> + sem->shared = pshared;
>
> return (0);
> }
>
> int
> -sem_destroy(sem_t *semp)
> +sem_destroy(sem_t *sem)
> {
> - sem_t sem;
>
> if (!_threads_ready) /* for SEM_MMAP_SIZE */
> _rthread_init();
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -186,21 +146,17 @@ sem_destroy(sem_t *semp)
> return (-1);
> }
>
> - *semp = NULL;
> if (sem->shared)
> munmap(sem, SEM_MMAP_SIZE);
> - else
> - free(sem);
>
> return (0);
> }
>
> int
> -sem_getvalue(sem_t *semp, int *sval)
> +sem_getvalue(sem_t *sem, int *sval)
> {
> - sem_t sem;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -213,11 +169,10 @@ sem_getvalue(sem_t *semp, int *sval)
> }
>
> int
> -sem_post(sem_t *semp)
> +sem_post(sem_t *sem)
> {
> - sem_t sem;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -228,11 +183,10 @@ sem_post(sem_t *semp)
> }
>
> int
> -sem_wait(sem_t *semp)
> +sem_wait(sem_t *sem)
> {
> struct tib *tib = TIB_GET();
> pthread_t self;
> - sem_t sem;
> int r;
> PREP_CANCEL_POINT(tib);
>
> @@ -240,7 +194,7 @@ sem_wait(sem_t *semp)
> _rthread_init();
> self = tib->tib_thread;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -258,15 +212,14 @@ sem_wait(sem_t *semp)
> }
>
> int
> -sem_timedwait(sem_t *semp, const struct timespec *abstime)
> +sem_timedwait(sem_t *sem, const struct timespec *abstime)
> {
> struct tib *tib = TIB_GET();
> pthread_t self;
> - sem_t sem;
> int r;
> PREP_CANCEL_POINT(tib);
>
> - if (!semp || !(sem = *semp) || abstime == NULL ||
> + if (!sem || abstime == NULL ||
> abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) {
> errno = EINVAL;
> return (-1);
> @@ -289,12 +242,11 @@ sem_timedwait(sem_t *semp, const struct
> }
>
> int
> -sem_trywait(sem_t *semp)
> +sem_trywait(sem_t *sem)
> {
> - sem_t sem;
> int r;
>
> - if (!semp || !(sem = *semp)) {
> + if (!sem) {
> errno = EINVAL;
> return (-1);
> }
> @@ -330,7 +282,7 @@ sem_open(const char *name, int oflag, ..
> {
> char sempath[SEM_PATH_SIZE];
> struct stat sb;
> - sem_t sem, *semp;
> + sem_t *sem;
> unsigned int value = 0;
> int created = 0, fd;
>
> @@ -396,35 +348,25 @@ sem_open(const char *name, int oflag, ..
> errno = EINVAL;
> return (SEM_FAILED);
> }
> - semp = malloc(sizeof(*semp));
> - if (!semp) {
> - munmap(sem, SEM_MMAP_SIZE);
> - errno = ENOSPC;
> - return (SEM_FAILED);
> - }
> if (created) {
> sem->lock = _SPINLOCK_UNLOCKED;
> sem->value = value;
> sem->shared = 1;
> }
> - *semp = sem;
>
> - return (semp);
> + return (sem);
> }
>
> int
> -sem_close(sem_t *semp)
> +sem_close(sem_t *sem)
> {
> - sem_t sem;
>
> - if (!semp || !(sem = *semp) || !sem->shared) {
> + if (!sem || !sem->shared) {
> errno = EINVAL;
> return (-1);
> }
>
> - *semp = NULL;
> munmap(sem, SEM_MMAP_SIZE);
> - free(semp);
>
> return (0);
> }
>
>
