On Thu, Aug 22, 2019 at 12:12 PM Rick Payne <ri...@rossfell.co.uk> 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 osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/CANEVyjtC6RG3Z5tGWdprHGo4ZKZiLNzqn28ajj_ez2YUNwFBfA%40mail.gmail.com.
