[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] Implement basic sleeping locks for gnumach.
From: |
Samuel Thibault |
Subject: |
Re: [PATCH 1/2] Implement basic sleeping locks for gnumach. |
Date: |
Mon, 30 Jan 2017 01:34:27 +0100 |
User-agent: |
Mutt/1.5.21+34 (58baf7c9f32f) (2010-12-30) |
Agustina Arzille, on Sun 29 Jan 2017 21:11:12 -0300, wrote:
> On 2017-01-29 17:44, Samuel Thibault wrote:
> >Agustina Arzille, on Sun 29 Jan 2017 16:46:45 -0300, wrote:
> >
> >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?
>
> Well, the locker thread checks the wait_result value: If it's
> THREAD_AWAKENED,
> it assumes that control of the mutex was transferred to it; otherwise, it
> assumes it was interrupted. I *think* that's good enough.
But what if some other thread, completely unrelated to the mutex, wakes
the locker thread, for whatever reason unrelated to the mutex?
> >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.
> >
>
> Ah, I see what you mean. Yeah, I was trying to get rid of the interlock,
> but I don't think it's possible, since you need to serialize access to
> the sleep queue.
The sleep queue is already protected inside assert_wait with a
simple_lock.
What you however want to serialize together is changing the mutex state
and the wakeup.
> >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?
> >
>
> Yes, I think it could be explained better. What it means is:
>
> - The owner thread believes that there's at least a waiter, since the state
> is KMUTEX_CONTENDED,
Ah, right. Now I see what the comment is coming from :)
> - However, thread_wakeup_one reports that no thread was awakened. This means
> that any waiter(s) that were sleeping on the mutex were interrupted. As
> such,
> it must reset the state and make it available.
Yes, "any" here is important, I'd say, otherwise one could think that
it's only one thread we have missed, while we actually have checked for
all of them.
Samuel