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(&rwlock->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(&rwlock->value, new, CLOCK_REALTIME,
+                           abs);
+               }
+               if (error == ETIMEDOUT)
+                       break;
-       _spinunlock(&lock->lock);
return (error);
-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?

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

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@

Reply via email to