On Wed, 02/15 12:20, Paolo Bonzini wrote:
>
>
> On 15/02/2017 10:23, Fam Zheng wrote:
> > On Mon, 02/13 19:12, Paolo Bonzini wrote:
> >> This adds a CoMutex around the existing CoQueue. Because the write-side
> >
> > s/CoQueue/CoRwlock/
>
> No, I meant that CoRwlock has a CoQueue, and after this patch it is
> wrapped by a CoMutex too.
OK.
>
>
> >> @@ -375,16 +384,20 @@ void qemu_co_rwlock_unlock(CoRwlock *lock)
> >> qemu_co_queue_next(&lock->queue);
> >> }
> >> }
> >> - self->locks_held--;
> >> + qemu_co_mutex_unlock(&lock->mutex);
> >> }
> >>
> >> void qemu_co_rwlock_wrlock(CoRwlock *lock)
> >> {
> >> - Coroutine *self = qemu_coroutine_self();
> >> -
> >> - while (lock->writer || lock->reader) {
> >> - qemu_co_queue_wait(&lock->queue, NULL);
> >> + qemu_co_mutex_lock(&lock->mutex);
> >> + lock->pending_writer++;
> >> + while (lock->reader) {
> >> + qemu_co_queue_wait(&lock->queue, &lock->mutex);
> >> }
> >> - lock->writer = true;
> >> - self->locks_held++;
> >> + lock->pending_writer--;
> >> +
> >> + /* The rest of the write-side critical section is run with
> >> + * the mutex taken, so that lock->reader remains zero.
> >> + * There is no need to update self->locks_held.
> >> + */
> >
> > But is it still better to update self->locks_held anyway for the
> > 'assert(!co->locks_held)' in qemu_coroutine_enter? Or is the same thing
> > checked
> > elsewhere?
>
> self->locks_held is already incremented by the qemu_co_mutex_lock call
> at the beginning of qemu_co_rwlock_wrlock. It is then decremented in
> qemu_co_rwlock_unlock.
>
> For the read side, rdlock _unlocks_ lock->mutex before returning, so
> self->locks_held must be incremented by rdlock and decremented by unlock.
Makes sense.
Fam