Agustina Arzille, on Sun 29 Jan 2017 16:46:45 -0300, wrote:
> >T1 atomic_cas(), gets the mutex
> >T2 fails atomic_cas()
> >T2 simple_lock()
> >T1 atomic_cas() to release the mutex
> >T2 reads KMUTEX_AVAIL in locked_swap
> >T3 atomic_cas(), gets the mutex
> >T2 writes KMUTEX_CONTENDED, gets KMUTEX_AVAIL
> >
> >now T2 believes it has the mutex, while it was given to T3.
>
> Ahh, that's a good point. We can use atomic_swap in place of locked_swap.
> Do you think that will be enough?
Yes.
There is just one thing I'm a bit afraid of: the code is assuming that
the only reason why the locker thread is awakened is that it was given
the mutex control. Is that guaranteed?
> >Don't use thread_sleep() as a whole: after failing to acquire simply,
> >assert_wait() the event, then use atomic_cas to try to put
> >KMUTEX_CONTENDED, and on success thread_block().
>
> This I don't see. thread_sleep() calls assert_wait() before unlocking the
> interlock and blocking. I'm not sure how using assert_wait() would improve
> things.
It'd allow to avoid simple_lock() completely on that side. I've been
thinking about avoiding simple_lock() completely, but it's not easy
without modifying thread_block/thread_wakeup_one deeply. So for now
let's keep the simple_lock, and then yes thread_sleep is enough.
> >>+void kmutex_unlock (struct kmutex *mtxp)
> >>+{
> >>+ if (atomic_cas (&mtxp->state, KMUTEX_LOCKED, KMUTEX_AVAIL))
> >>+ /* No waiters - We're done. */
> >>+ return;
> >>+
> >>+ simple_lock (&mtxp->lock);
> >>+
> >>+ if (!thread_wakeup_one ((event_t)mtxp))
> >>+ /* The waiter was interrupted and left - Reset the mutex state. */
> >>+ mtxp->state = KMUTEX_AVAIL;
> >
> >And what about other threads which were waiting on the mutex? We need
> >to make sure that at least one wakes up to get the mutex, otherwise
> >they'll stay asleep indefinitely.
>
> I modified thread_wakeup_prim() so that it returns TRUE if it managed
> to wake up at least a thread. So, if it returns false, we can be assert
> that there were no waiters.
The comment above is misleading then: "The waiter was interrupted and
left". Does it mean that *at best* there were some waiters but they were
interrupted?
Samuel
