Re: futex(2) based pthread_rwlock*
On 13.02.2019 15:08, Martin Pieuchot wrote:
+ val = rwlock->value;
+ if (val == UNLOCKED || (val & WAITING))
+ break;
+
+ SPIN_WAIT();
+ }
+
+ while ((error = _rthread_rwlock_tryrdlock(rwlock)) == EBUSY) {
+ val = rwlock->value;
+ if (val == UNLOCKED || (COUNT(val)) != WRITER)
+ continue;
+ new = val | WAITING;
+ if (atomic_cas_uint(>value, val, new) == val) {
Don't you need a membar_after_atomic() here?
Why? The lock hasn't been acquired here.
Right, but you are possibly changing the value of, erm, rwlock->value
which will be read in rthread_rwlock_unlock(). But I guess this will be
taken care of by the membar_exit_before_atomic() call before taking the
lock.
+ error = _twait(>value, new, CLOCK_REALTIME,
+ abs);
+ }
+ if (error == ETIMEDOUT)
+ break;
}
- _spinunlock(>lock);
return (error);
+
int
-pthread_rwlock_unlock(pthread_rwlock_t *lockp)
+pthread_rwlock_unlock(pthread_rwlock_t *rwlockp)
{
+ pthread_t self = pthread_self();
+ pthread_rwlock_t rwlock;
+ unsigned int val, new;
+
+ rwlock = *rwlockp;
+ _rthread_debug(5, "%p: rwlock_unlock %p\n", self, (void *)rwlock);
+
+ membar_exit_before_atomic();
Wouldn't this membar need to be inside the loop? Or perhaps a
corresponding membar_enter() after exiting the loop?
Why?
The membar is here to enforce that writes done during the critical
section are visible before the lock is released. Such that another
thread wont grab the lock and see outdated data inside the critical
section.
Can't another thread grab the lock right after cas() and before _wake()?
I thought after my semaphore implementation I managed to grasp how these
membars are supposed to be used, but here I am half a year later (or
more?) and I forgot or (most probably) never really understood them :)
Eitherway, please go ahead and commit this. OK pirofti@
Re: futex(2) based pthread_rwlock*
On 2019-01-30, Martin Pieuchot wrote: > Newer version with fewer atomics and some barrier corrections as pointed > out by visa@. This has now gone through a full amd64 package build without problems. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: futex(2) based pthread_rwlock*
On 12/02/19(Tue) 23:25, Paul Irofti wrote:
> On Wed, Jan 30, 2019 at 12:29:20PM -0200, Martin Pieuchot wrote:
> > On 28/01/19(Mon) 14:57, Martin Pieuchot wrote:
> > > Summer is really warm here. No need to make my machines hotter by
> > > spinning a lot. So here's a futex(2) based pthread_rwlock*
> > > implementation. I should look familiar to those of you that reviewed
> > > the mutex & semaphore implementations. It uses amotic cas & inc/dec.
> > >
> > > I moved the existing implementation to rthread_rwlock_compat.c to match
> > > what has been done for semaphores.
> > >
> > > Tests, benchmarks and reviews are more than welcome!
> >
> > Newer version with fewer atomics and some barrier corrections as pointed
> > out by visa@.
>
> Appologies again for the late review.
>
> I sprinkled various comments inline. The implementation is nice, clean
> and easy to read! Except, perhaps, for the COUNT macro.
>
> So I am generally OK with this diff. One question though, is the manual
> page statement about preventing writer starvation still true with your
> version? I don't think so. Or perhaps I am missing the bit where a
> reader checks if a writer is waiting for the lock...
I'll fix the manpages :)
> > @@ -52,26 +59,25 @@ DEF_STD(pthread_rwlock_init);
> > int
> > pthread_rwlock_destroy(pthread_rwlock_t *lockp)
> > {
> > - pthread_rwlock_t lock;
> > + pthread_rwlock_t rwlock;
> >
> > - assert(lockp);
> > - lock = *lockp;
> > - if (lock) {
> > - if (lock->readers || !TAILQ_EMPTY(>writers)) {
> > + rwlock = *lockp;
> > + if (rwlock) {
> > + if (rwlock->value != UNLOCKED) {
> > #define MSG "pthread_rwlock_destroy on rwlock with waiters!\n"
> > write(2, MSG, sizeof(MSG) - 1);
> > #undef MSG
>
> Is there no better way than a macro?
Feel free to improve it. Note that this idiom is used in all rthread
files.
> > return (EBUSY);
> > }
> > - free(lock);
> > + free((void *)rwlock);
>
> No need to cast.
Until we move it to libc/thread. This is done for coherency with
rthread_mutex.c from which this implementation is based :o)
>
> > + *lockp = NULL;
> > }
> > - *lockp = NULL;
> >
> > return (0);
> > }
> >
> > static int
> > -_rthread_rwlock_ensure_init(pthread_rwlock_t *lockp)
> > +_rthread_rwlock_ensure_init(pthread_rwlock_t *rwlockp)
> > {
> > int ret = 0;
>
> I really like this functionality. Is this mandated by POSIX? Can we add
> it to other locking or pthread primitives?
It's already present where needed, it was already there (:
> > {
> > - pthread_rwlock_t lock;
> > - pthread_t thread = pthread_self();
> > - int error;
> > + pthread_t self = pthread_self();
> > + pthread_rwlock_t rwlock;
> > + unsigned int val, new;
> > + int i, error;
>
> You need to check the timespec here
This is checked by _twait() like for mutexes.
> > + error = _rthread_rwlock_tryrdlock(rwlock);
> > + if (error != EBUSY || trywait)
> > + return (error);
> > +
> > + /* Try hard to not enter the kernel. */
> > + for (i = 0; i < SPIN_COUNT; i ++) {
> ^ style
Indeed (bad copy paste)!
> > + val = rwlock->value;
> > + if (val == UNLOCKED || (val & WAITING))
> > + break;
> > +
> > + SPIN_WAIT();
> > + }
> > +
> > + while ((error = _rthread_rwlock_tryrdlock(rwlock)) == EBUSY) {
> > + val = rwlock->value;
> > + if (val == UNLOCKED || (COUNT(val)) != WRITER)
> > + continue;
> > + new = val | WAITING;
> > + if (atomic_cas_uint(>value, val, new) == val) {
>
> Don't you need a membar_after_atomic() here?
Why? The lock hasn't been acquired here.
>
> > + error = _twait(>value, new, CLOCK_REALTIME,
> > + abs);
> > + }
> > + if (error == ETIMEDOUT)
> > + break;
> > }
> > - _spinunlock(>lock);
> >
> > return (error);
> > +
> > int
> > -pthread_rwlock_unlock(pthread_rwlock_t *lockp)
> > +pthread_rwlock_unlock(pthread_rwlock_t *rwlockp)
> > {
> > + pthread_t self = pthread_self();
> > + pthread_rwlock_t rwlock;
> > + unsigned int val, new;
> > +
> > + rwlock = *rwlockp;
> > + _rthread_debug(5, "%p: rwlock_unlock %p\n", self, (void *)rwlock);
> > +
> > + membar_exit_before_atomic();
>
> Wouldn't this membar need to be inside the loop? Or perhaps a
> corresponding membar_enter() after exiting the loop?
Why?
The membar is here to enforce that writes done during the critical
section are visible before the lock is released. Such that another
thread wont grab the lock and see outdated data inside the critical
section.
Re: futex(2) based pthread_rwlock*
On Wed, Jan 30, 2019 at 12:29:20PM -0200, Martin Pieuchot wrote:
> On 28/01/19(Mon) 14:57, Martin Pieuchot wrote:
> > Summer is really warm here. No need to make my machines hotter by
> > spinning a lot. So here's a futex(2) based pthread_rwlock*
> > implementation. I should look familiar to those of you that reviewed
> > the mutex & semaphore implementations. It uses amotic cas & inc/dec.
> >
> > I moved the existing implementation to rthread_rwlock_compat.c to match
> > what has been done for semaphores.
> >
> > Tests, benchmarks and reviews are more than welcome!
>
> Newer version with fewer atomics and some barrier corrections as pointed
> out by visa@.
Appologies again for the late review.
I sprinkled various comments inline. The implementation is nice, clean
and easy to read! Except, perhaps, for the COUNT macro.
So I am generally OK with this diff. One question though, is the manual
page statement about preventing writer starvation still true with your
version? I don't think so. Or perhaps I am missing the bit where a
reader checks if a writer is waiting for the lock...
It is late here. I'll have a fresh look in the morning.
This is my review until then.
>
> Index: libc/include/thread_private.h
> ===
> RCS file: /cvs/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 thread_private.h
> --- libc/include/thread_private.h 10 Jan 2019 18:45:33 - 1.34
> +++ libc/include/thread_private.h 30 Jan 2019 14:21:16 -
> @@ -298,6 +298,10 @@ struct pthread_cond {
> struct pthread_mutex *mutex;
> };
>
> +struct pthread_rwlock {
> + volatile unsigned int value;
> +};
> +
> #else
>
> struct pthread_mutex {
> @@ -314,6 +318,13 @@ struct pthread_cond {
> struct pthread_queue waiters;
> struct pthread_mutex *mutex;
> clockid_t clock;
> +};
> +
> +struct pthread_rwlock {
> + _atomic_lock_t lock;
> + pthread_t owner;
> + struct pthread_queue writers;
> + int readers;
> };
> #endif /* FUTEX */
>
> Index: librthread/Makefile
> ===
> RCS file: /cvs/src/lib/librthread/Makefile,v
> retrieving revision 1.54
> diff -u -p -r1.54 Makefile
> --- librthread/Makefile 12 Jan 2019 00:16:03 - 1.54
> +++ librthread/Makefile 23 Jan 2019 20:07:29 -
> @@ -27,7 +27,6 @@ SRCS= rthread.c \
> rthread_mutex_prio.c \
> rthread_mutexattr.c \
> rthread_np.c \
> - rthread_rwlock.c \
> rthread_rwlockattr.c \
> rthread_sched.c \
> rthread_stack.c \
> @@ -40,9 +39,12 @@ SRCS= rthread.c \
> ${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "mips64" || \
> ${MACHINE_ARCH} == "mips64el" || ${MACHINE_ARCH} == "powerpc" || \
> ${MACHINE_ARCH} == "sparc64"
> -SRCS+= rthread_sem.c
> +CFLAGS+= -DFUTEX
> +SRCS+= rthread_sem.c \
> + rthread_rwlock.c
> .else
> -SRCS+= rthread_sem_compat.c
> +SRCS+= rthread_sem_compat.c \
> + rthread_rwlock_compat.c
> .endif
>
> SRCDIR= ${.CURDIR}/../libpthread
> Index: librthread/rthread.h
> ===
> RCS file: /cvs/src/lib/librthread/rthread.h,v
> retrieving revision 1.63
> diff -u -p -r1.63 rthread.h
> --- librthread/rthread.h 5 Sep 2017 02:40:54 - 1.63
> +++ librthread/rthread.h 23 Jan 2019 17:02:51 -
> @@ -52,13 +52,6 @@ struct stack {
> #define PTHREAD_MAX_PRIORITY31
>
>
> -struct pthread_rwlock {
> - _atomic_lock_t lock;
> - pthread_t owner;
> - struct pthread_queue writers;
> - int readers;
> -};
> -
> struct pthread_rwlockattr {
> int pshared;
> };
> Index: librthread/rthread_rwlock.c
> ===
> RCS file: /cvs/src/lib/librthread/rthread_rwlock.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rthread_rwlock.c
> --- librthread/rthread_rwlock.c 24 Apr 2018 16:28:42 - 1.11
> +++ librthread/rthread_rwlock.c 30 Jan 2019 14:28:01 -
> @@ -1,8 +1,7 @@
> /* $OpenBSD: rthread_rwlock.c,v 1.11 2018/04/24 16:28:42 pirofti Exp $ */
> /*
> - * Copyright (c) 2004,2005 Ted Unangst
> + * Copyright (c) 2019 Martin Pieuchot
> * Copyright (c) 2012 Philip Guenther
> - * All Rights Reserved.
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that
Re: futex(2) based pthread_rwlock*
On Thu, Feb 07, 2019 at 06:37:35PM +, Stuart Henderson wrote: > On 2019/01/30 12:29, Martin Pieuchot wrote: > > On 28/01/19(Mon) 14:57, Martin Pieuchot wrote: > > > Summer is really warm here. No need to make my machines hotter by > > > spinning a lot. So here's a futex(2) based pthread_rwlock* > > > implementation. I should look familiar to those of you that reviewed > > > the mutex & semaphore implementations. It uses amotic cas & inc/dec. > > > > > > I moved the existing implementation to rthread_rwlock_compat.c to match > > > what has been done for semaphores. > > > > > > Tests, benchmarks and reviews are more than welcome! > > > > Newer version with fewer atomics and some barrier corrections as pointed > > out by visa@. > > This has been through 3 or 4 bulk ports builds on i386 (2 and 4 core > machines) and running on my amd64 workstation (4 core), no problems seen > on either. Has anyone else been running this? The reduction in spinning > is nice. Yes, I've been running this diff since it was posted on my main laptop (x280). No issues seen and it feels a lot better, especially noticeable when switching back to an install without the diff.
Re: futex(2) based pthread_rwlock*
On 2019/01/30 12:29, Martin Pieuchot wrote:
> On 28/01/19(Mon) 14:57, Martin Pieuchot wrote:
> > Summer is really warm here. No need to make my machines hotter by
> > spinning a lot. So here's a futex(2) based pthread_rwlock*
> > implementation. I should look familiar to those of you that reviewed
> > the mutex & semaphore implementations. It uses amotic cas & inc/dec.
> >
> > I moved the existing implementation to rthread_rwlock_compat.c to match
> > what has been done for semaphores.
> >
> > Tests, benchmarks and reviews are more than welcome!
>
> Newer version with fewer atomics and some barrier corrections as pointed
> out by visa@.
This has been through 3 or 4 bulk ports builds on i386 (2 and 4 core
machines) and running on my amd64 workstation (4 core), no problems seen
on either. Has anyone else been running this? The reduction in spinning
is nice.
> Index: libc/include/thread_private.h
> ===
> RCS file: /cvs/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 thread_private.h
> --- libc/include/thread_private.h 10 Jan 2019 18:45:33 - 1.34
> +++ libc/include/thread_private.h 30 Jan 2019 14:21:16 -
> @@ -298,6 +298,10 @@ struct pthread_cond {
> struct pthread_mutex *mutex;
> };
>
> +struct pthread_rwlock {
> + volatile unsigned int value;
> +};
> +
> #else
>
> struct pthread_mutex {
> @@ -314,6 +318,13 @@ struct pthread_cond {
> struct pthread_queue waiters;
> struct pthread_mutex *mutex;
> clockid_t clock;
> +};
> +
> +struct pthread_rwlock {
> + _atomic_lock_t lock;
> + pthread_t owner;
> + struct pthread_queue writers;
> + int readers;
> };
> #endif /* FUTEX */
>
> Index: librthread/Makefile
> ===
> RCS file: /cvs/src/lib/librthread/Makefile,v
> retrieving revision 1.54
> diff -u -p -r1.54 Makefile
> --- librthread/Makefile 12 Jan 2019 00:16:03 - 1.54
> +++ librthread/Makefile 23 Jan 2019 20:07:29 -
> @@ -27,7 +27,6 @@ SRCS= rthread.c \
> rthread_mutex_prio.c \
> rthread_mutexattr.c \
> rthread_np.c \
> - rthread_rwlock.c \
> rthread_rwlockattr.c \
> rthread_sched.c \
> rthread_stack.c \
> @@ -40,9 +39,12 @@ SRCS= rthread.c \
> ${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "mips64" || \
> ${MACHINE_ARCH} == "mips64el" || ${MACHINE_ARCH} == "powerpc" || \
> ${MACHINE_ARCH} == "sparc64"
> -SRCS+= rthread_sem.c
> +CFLAGS+= -DFUTEX
> +SRCS+= rthread_sem.c \
> + rthread_rwlock.c
> .else
> -SRCS+= rthread_sem_compat.c
> +SRCS+= rthread_sem_compat.c \
> + rthread_rwlock_compat.c
> .endif
>
> SRCDIR= ${.CURDIR}/../libpthread
> Index: librthread/rthread.h
> ===
> RCS file: /cvs/src/lib/librthread/rthread.h,v
> retrieving revision 1.63
> diff -u -p -r1.63 rthread.h
> --- librthread/rthread.h 5 Sep 2017 02:40:54 - 1.63
> +++ librthread/rthread.h 23 Jan 2019 17:02:51 -
> @@ -52,13 +52,6 @@ struct stack {
> #define PTHREAD_MAX_PRIORITY31
>
>
> -struct pthread_rwlock {
> - _atomic_lock_t lock;
> - pthread_t owner;
> - struct pthread_queue writers;
> - int readers;
> -};
> -
> struct pthread_rwlockattr {
> int pshared;
> };
> Index: librthread/rthread_rwlock.c
> ===
> RCS file: /cvs/src/lib/librthread/rthread_rwlock.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rthread_rwlock.c
> --- librthread/rthread_rwlock.c 24 Apr 2018 16:28:42 - 1.11
> +++ librthread/rthread_rwlock.c 30 Jan 2019 14:28:01 -
> @@ -1,8 +1,7 @@
> /* $OpenBSD: rthread_rwlock.c,v 1.11 2018/04/24 16:28:42 pirofti Exp $ */
> /*
> - * Copyright (c) 2004,2005 Ted Unangst
> + * Copyright (c) 2019 Martin Pieuchot
> * Copyright (c) 2012 Philip Guenther
> - * All Rights Reserved.
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -16,11 +15,7 @@
> * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
> -/*
> - * rwlocks
> - */
>
> -#include
> #include
> #include
> #include
> @@ -28,
Re: futex(2) based pthread_rwlock*
On Wed, Jan 30, 2019 at 12:29:20PM -0200, Martin Pieuchot wrote:
> On 28/01/19(Mon) 14:57, Martin Pieuchot wrote:
> > Summer is really warm here. No need to make my machines hotter by
> > spinning a lot. So here's a futex(2) based pthread_rwlock*
> > implementation. I should look familiar to those of you that reviewed
> > the mutex & semaphore implementations. It uses amotic cas & inc/dec.
> >
> > I moved the existing implementation to rthread_rwlock_compat.c to match
> > what has been done for semaphores.
> >
> > Tests, benchmarks and reviews are more than welcome!
>
> Newer version with fewer atomics and some barrier corrections as pointed
> out by visa@.
I am trying to review this asap. Currently a bit busy. Hopefully Friday
I will have a bit of time to look this over.
>
> Index: libc/include/thread_private.h
> ===
> RCS file: /cvs/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 thread_private.h
> --- libc/include/thread_private.h 10 Jan 2019 18:45:33 - 1.34
> +++ libc/include/thread_private.h 30 Jan 2019 14:21:16 -
> @@ -298,6 +298,10 @@ struct pthread_cond {
> struct pthread_mutex *mutex;
> };
>
> +struct pthread_rwlock {
> + volatile unsigned int value;
> +};
> +
> #else
>
> struct pthread_mutex {
> @@ -314,6 +318,13 @@ struct pthread_cond {
> struct pthread_queue waiters;
> struct pthread_mutex *mutex;
> clockid_t clock;
> +};
> +
> +struct pthread_rwlock {
> + _atomic_lock_t lock;
> + pthread_t owner;
> + struct pthread_queue writers;
> + int readers;
> };
> #endif /* FUTEX */
>
> Index: librthread/Makefile
> ===
> RCS file: /cvs/src/lib/librthread/Makefile,v
> retrieving revision 1.54
> diff -u -p -r1.54 Makefile
> --- librthread/Makefile 12 Jan 2019 00:16:03 - 1.54
> +++ librthread/Makefile 23 Jan 2019 20:07:29 -
> @@ -27,7 +27,6 @@ SRCS= rthread.c \
> rthread_mutex_prio.c \
> rthread_mutexattr.c \
> rthread_np.c \
> - rthread_rwlock.c \
> rthread_rwlockattr.c \
> rthread_sched.c \
> rthread_stack.c \
> @@ -40,9 +39,12 @@ SRCS= rthread.c \
> ${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "mips64" || \
> ${MACHINE_ARCH} == "mips64el" || ${MACHINE_ARCH} == "powerpc" || \
> ${MACHINE_ARCH} == "sparc64"
> -SRCS+= rthread_sem.c
> +CFLAGS+= -DFUTEX
> +SRCS+= rthread_sem.c \
> + rthread_rwlock.c
> .else
> -SRCS+= rthread_sem_compat.c
> +SRCS+= rthread_sem_compat.c \
> + rthread_rwlock_compat.c
> .endif
>
> SRCDIR= ${.CURDIR}/../libpthread
> Index: librthread/rthread.h
> ===
> RCS file: /cvs/src/lib/librthread/rthread.h,v
> retrieving revision 1.63
> diff -u -p -r1.63 rthread.h
> --- librthread/rthread.h 5 Sep 2017 02:40:54 - 1.63
> +++ librthread/rthread.h 23 Jan 2019 17:02:51 -
> @@ -52,13 +52,6 @@ struct stack {
> #define PTHREAD_MAX_PRIORITY31
>
>
> -struct pthread_rwlock {
> - _atomic_lock_t lock;
> - pthread_t owner;
> - struct pthread_queue writers;
> - int readers;
> -};
> -
> struct pthread_rwlockattr {
> int pshared;
> };
> Index: librthread/rthread_rwlock.c
> ===
> RCS file: /cvs/src/lib/librthread/rthread_rwlock.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rthread_rwlock.c
> --- librthread/rthread_rwlock.c 24 Apr 2018 16:28:42 - 1.11
> +++ librthread/rthread_rwlock.c 30 Jan 2019 14:28:01 -
> @@ -1,8 +1,7 @@
> /* $OpenBSD: rthread_rwlock.c,v 1.11 2018/04/24 16:28:42 pirofti Exp $ */
> /*
> - * Copyright (c) 2004,2005 Ted Unangst
> + * Copyright (c) 2019 Martin Pieuchot
> * Copyright (c) 2012 Philip Guenther
> - * All Rights Reserved.
> *
> * Permission to use, copy, modify, and distribute this software for any
> * purpose with or without fee is hereby granted, provided that the above
> @@ -16,11 +15,7 @@
> * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
> -/*
> - * rwlocks
> - */
>
> -#include
> #include
> #include
> #include
> @@ -28,6 +23,20 @@
> #include
>
> #include "rthread.h"
> +#in
Re: futex(2) based pthread_rwlock*
On 28/01/19(Mon) 14:57, Martin Pieuchot wrote:
> Summer is really warm here. No need to make my machines hotter by
> spinning a lot. So here's a futex(2) based pthread_rwlock*
> implementation. I should look familiar to those of you that reviewed
> the mutex & semaphore implementations. It uses amotic cas & inc/dec.
>
> I moved the existing implementation to rthread_rwlock_compat.c to match
> what has been done for semaphores.
>
> Tests, benchmarks and reviews are more than welcome!
Newer version with fewer atomics and some barrier corrections as pointed
out by visa@.
Index: libc/include/thread_private.h
===
RCS file: /cvs/src/lib/libc/include/thread_private.h,v
retrieving revision 1.34
diff -u -p -r1.34 thread_private.h
--- libc/include/thread_private.h 10 Jan 2019 18:45:33 - 1.34
+++ libc/include/thread_private.h 30 Jan 2019 14:21:16 -
@@ -298,6 +298,10 @@ struct pthread_cond {
struct pthread_mutex *mutex;
};
+struct pthread_rwlock {
+ volatile unsigned int value;
+};
+
#else
struct pthread_mutex {
@@ -314,6 +318,13 @@ struct pthread_cond {
struct pthread_queue waiters;
struct pthread_mutex *mutex;
clockid_t clock;
+};
+
+struct pthread_rwlock {
+ _atomic_lock_t lock;
+ pthread_t owner;
+ struct pthread_queue writers;
+ int readers;
};
#endif /* FUTEX */
Index: librthread/Makefile
===
RCS file: /cvs/src/lib/librthread/Makefile,v
retrieving revision 1.54
diff -u -p -r1.54 Makefile
--- librthread/Makefile 12 Jan 2019 00:16:03 - 1.54
+++ librthread/Makefile 23 Jan 2019 20:07:29 -
@@ -27,7 +27,6 @@ SRCS= rthread.c \
rthread_mutex_prio.c \
rthread_mutexattr.c \
rthread_np.c \
- rthread_rwlock.c \
rthread_rwlockattr.c \
rthread_sched.c \
rthread_stack.c \
@@ -40,9 +39,12 @@ SRCS=rthread.c \
${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "mips64" || \
${MACHINE_ARCH} == "mips64el" || ${MACHINE_ARCH} == "powerpc" || \
${MACHINE_ARCH} == "sparc64"
-SRCS+= rthread_sem.c
+CFLAGS+= -DFUTEX
+SRCS+= rthread_sem.c \
+ rthread_rwlock.c
.else
-SRCS+= rthread_sem_compat.c
+SRCS+= rthread_sem_compat.c \
+ rthread_rwlock_compat.c
.endif
SRCDIR= ${.CURDIR}/../libpthread
Index: librthread/rthread.h
===
RCS file: /cvs/src/lib/librthread/rthread.h,v
retrieving revision 1.63
diff -u -p -r1.63 rthread.h
--- librthread/rthread.h5 Sep 2017 02:40:54 - 1.63
+++ librthread/rthread.h23 Jan 2019 17:02:51 -
@@ -52,13 +52,6 @@ struct stack {
#definePTHREAD_MAX_PRIORITY31
-struct pthread_rwlock {
- _atomic_lock_t lock;
- pthread_t owner;
- struct pthread_queue writers;
- int readers;
-};
-
struct pthread_rwlockattr {
int pshared;
};
Index: librthread/rthread_rwlock.c
===
RCS file: /cvs/src/lib/librthread/rthread_rwlock.c,v
retrieving revision 1.11
diff -u -p -r1.11 rthread_rwlock.c
--- librthread/rthread_rwlock.c 24 Apr 2018 16:28:42 - 1.11
+++ librthread/rthread_rwlock.c 30 Jan 2019 14:28:01 -
@@ -1,8 +1,7 @@
/* $OpenBSD: rthread_rwlock.c,v 1.11 2018/04/24 16:28:42 pirofti Exp $ */
/*
- * Copyright (c) 2004,2005 Ted Unangst
+ * Copyright (c) 2019 Martin Pieuchot
* Copyright (c) 2012 Philip Guenther
- * All Rights Reserved.
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -16,11 +15,7 @@
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-/*
- * rwlocks
- */
-#include
#include
#include
#include
@@ -28,6 +23,20 @@
#include
#include "rthread.h"
+#include "synch.h"
+
+#define UNLOCKED 0
+#define MAXREADER 0x7ffe
+#define WRITER 0x7fff
+#define WAITING0x8000
+#define COUNT(v) ((v) & WRITER)
+
+#define SPIN_COUNT 128
+#if defined(__i386__) || defined(__amd64__)
+#define SPIN_WAIT()asm volatile("pause": : : "memory")
+#else
+#define SPIN_WAIT()do { } while (0)
+#endif
static _atomic_lock_t rwlock_init_lock = _SPINLOCK_UNLOCKED;
@@ -35,15 +44,13 @@ int
pthread_rwlock_init(pthread_rwlock_t *lockp,
const pthread_rwlockattr_t *attrp __unused)
{
- pthread_rwlock_t lock;
+ pthread_rwlock_t rwlock;
- lock = calloc(1, sizeof(*lock));
- if (!lock)
+ rwlock = calloc(1, sizeof(*rwlock));
+ if (!rwlock)
return
futex(2) based pthread_rwlock*
Summer is really warm here. No need to make my machines hotter by
spinning a lot. So here's a futex(2) based pthread_rwlock*
implementation. I should look familiar to those of you that reviewed
the mutex & semaphore implementations. It uses amotic cas & inc/dec.
I moved the existing implementation to rthread_rwlock_compat.c to match
what has been done for semaphores.
Tests, benchmarks and reviews are more than welcome!
Index: libc/include/thread_private.h
===
RCS file: /cvs/src/lib/libc/include/thread_private.h,v
retrieving revision 1.34
diff -u -p -r1.34 thread_private.h
--- libc/include/thread_private.h 10 Jan 2019 18:45:33 - 1.34
+++ libc/include/thread_private.h 23 Jan 2019 19:34:36 -
@@ -298,6 +298,11 @@ struct pthread_cond {
struct pthread_mutex *mutex;
};
+struct pthread_rwlock {
+ volatile unsigned int value;
+ volatile unsigned int waiters;
+};
+
#else
struct pthread_mutex {
@@ -314,6 +319,13 @@ struct pthread_cond {
struct pthread_queue waiters;
struct pthread_mutex *mutex;
clockid_t clock;
+};
+
+struct pthread_rwlock {
+ _atomic_lock_t lock;
+ pthread_t owner;
+ struct pthread_queue writers;
+ int readers;
};
#endif /* FUTEX */
Index: librthread/Makefile
===
RCS file: /cvs/src/lib/librthread/Makefile,v
retrieving revision 1.54
diff -u -p -r1.54 Makefile
--- librthread/Makefile 12 Jan 2019 00:16:03 - 1.54
+++ librthread/Makefile 23 Jan 2019 20:07:29 -
@@ -27,7 +27,6 @@ SRCS= rthread.c \
rthread_mutex_prio.c \
rthread_mutexattr.c \
rthread_np.c \
- rthread_rwlock.c \
rthread_rwlockattr.c \
rthread_sched.c \
rthread_stack.c \
@@ -40,9 +39,12 @@ SRCS=rthread.c \
${MACHINE_ARCH} == "i386" || ${MACHINE_ARCH} == "mips64" || \
${MACHINE_ARCH} == "mips64el" || ${MACHINE_ARCH} == "powerpc" || \
${MACHINE_ARCH} == "sparc64"
-SRCS+= rthread_sem.c
+CFLAGS+= -DFUTEX
+SRCS+= rthread_sem.c \
+ rthread_rwlock.c
.else
-SRCS+= rthread_sem_compat.c
+SRCS+= rthread_sem_compat.c \
+ rthread_rwlock_compat.c
.endif
SRCDIR= ${.CURDIR}/../libpthread
Index: librthread/rthread.h
===
RCS file: /cvs/src/lib/librthread/rthread.h,v
retrieving revision 1.63
diff -u -p -r1.63 rthread.h
--- librthread/rthread.h5 Sep 2017 02:40:54 - 1.63
+++ librthread/rthread.h23 Jan 2019 17:02:51 -
@@ -52,13 +52,6 @@ struct stack {
#definePTHREAD_MAX_PRIORITY31
-struct pthread_rwlock {
- _atomic_lock_t lock;
- pthread_t owner;
- struct pthread_queue writers;
- int readers;
-};
-
struct pthread_rwlockattr {
int pshared;
};
Index: librthread/rthread_rwlock.c
===
RCS file: /cvs/src/lib/librthread/rthread_rwlock.c,v
retrieving revision 1.11
diff -u -p -r1.11 rthread_rwlock.c
--- librthread/rthread_rwlock.c 24 Apr 2018 16:28:42 - 1.11
+++ librthread/rthread_rwlock.c 23 Jan 2019 20:30:22 -
@@ -1,8 +1,7 @@
/* $OpenBSD: rthread_rwlock.c,v 1.11 2018/04/24 16:28:42 pirofti Exp $ */
/*
- * Copyright (c) 2004,2005 Ted Unangst
+ * Copyright (c) 2018 Martin Pieuchot
* Copyright (c) 2012 Philip Guenther
- * All Rights Reserved.
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -16,11 +15,7 @@
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-/*
- * rwlocks
- */
-#include
#include
#include
#include
@@ -28,6 +23,20 @@
#include
#include "rthread.h"
+#include "synch.h"
+
+#define UNLOCKED 0
+#define MAXREADER 0x7ffe
+#define WRITER 0x7fff
+#define WAITING0x8000
+#define COUNT(v) ((v) & WRITER)
+
+#define SPIN_COUNT 128
+#if defined(__i386__) || defined(__amd64__)
+#define SPIN_WAIT()asm volatile("pause": : : "memory")
+#else
+#define SPIN_WAIT()do { } while (0)
+#endif
static _atomic_lock_t rwlock_init_lock = _SPINLOCK_UNLOCKED;
@@ -35,15 +44,13 @@ int
pthread_rwlock_init(pthread_rwlock_t *lockp,
const pthread_rwlockattr_t *attrp __unused)
{
- pthread_rwlock_t lock;
+ pthread_rwlock_t rwlock;
- lock = calloc(1, sizeof(*lock));
- if (!lock)
+ rwlock = calloc(1, sizeof(*rwlock));
+ if (!rwlock)
return (errno);
- lock->lock = _SPINLOCK_UNLOCKED;
- TAILQ_INIT(>writers);
- *lockp = lock;
+ *lockp = rwlock;
re
