On Thu, Aug 22, 2019 at 12:12 PM Rick Payne <[email protected]> wrote:
> On Thu, 2019-08-22 at 10:33 +0300, Nadav Har'El wrote:
> > You're right, it seems there's should be a "return" in the recursive
> > case!
> > That being said, I think the spurious wakeup doesn't cause any harm,
> > because the wait code rwlock::writer_wait_lockable() loops, and if a
> > thread
> > is woken while the lock is still taken, it just goes to sleep again.
> > It will just lose it's good spot on the queue :-(
>
> I wasn't sure that was the case. I put an assert in
> writer_wait_lockable() (see below) and I was able to trigger it by
> having 1 thread take the write lock twice, then a second thread attempt
> to take the write lock. When the first thread released, the second
> thread triggers the assert.
>
> void rwlock::writer_wait_lockable()
> {
> while (true) {
> if (write_lockable()) {
> return;
> }
>
> _write_waiters.wait(_mtx);
> assert((_wowner == sched::thread::current()) ||
> (_wowner == nullptr));
>
Yes, *this* assert will trigger, but it doesn't matter for correctness -
because what will happen next is that the loop will continue, and the code
will check write_lockable() again, and only if that is true, it will
return. This is what I said - this will be a spurious wakeup, and a waste
of time, but will not compromise correctness. At least according to my
understanding.
>
> }
> }
>
> Rick
>
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/CANEVyjtC6RG3Z5tGWdprHGo4ZKZiLNzqn28ajj_ez2YUNwFBfA%40mail.gmail.com.
