All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Edmondson <dme@dme.org>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Stefan Weil <sw@weilnetz.de>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: Re: [RFC PATCH 4/4] coroutine/rwlock: Wake writers in preference to readers
Date: Tue, 09 Mar 2021 11:57:02 +0000	[thread overview]
Message-ID: <m2pn08h75d.fsf@dme.org> (raw)
In-Reply-To: <8de7f2e6-825f-a803-cf12-870559b5e787@redhat.com>

On Tuesday, 2021-03-09 at 12:06:22 +01, Paolo Bonzini wrote:

> On 09/03/21 11:21, David Edmondson wrote:
>> -        /* The critical section started in qemu_co_rwlock_wrlock.  */
>> -        qemu_co_queue_restart_all(&lock->queue);
>> +        /* The critical section started in qemu_co_rwlock_wrlock or
>> +         * qemu_co_rwlock_upgrade.
>> +         */
>> +        qemu_co_queue_restart_all(&lock->wqueue);
>> +        qemu_co_queue_restart_all(&lock->rqueue);
>
> Hmm, the devil is in the details---this is a thundering herd waiting to 
> happen.  But fortunately this can be fixed while making the unlock 
> primitive even simpler:
>
>      if (lock->reader) {
>           self->locks_held--;
>
>           /* Read-side critical sections do not keep lock->mutex.  */
>           qemu_co_mutex_lock(&lock->mutex);
>           lock->reader--;
>           assert(lock->reader >= 0);
>       }
>
>       /* If there are no remaining readers wake one waiting writer
>        * or all waiting readers.
>        */
>       if (!lock->reader && !qemu_co_queue_next(&lock->wqueue)) {
>           assert(!lock->pending_writer);
>           qemu_co_queue_restart_all(&lock->rqueue);
>       }

That's a nice improvement, I agree. I'll roll it into another revision
and work on a test.

dme.
-- 
But uh oh, I love her because, she moves in her own way.


      reply	other threads:[~2021-03-09 12:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-09 10:21 [RFC PATCH 0/4] coroutine rwlock downgrade fix, minor VDI changes David Edmondson
2021-03-09 10:21 ` [RFC PATCH 1/4] block/vdi: When writing new bmap entry fails, don't leak the buffer David Edmondson
2021-03-09 11:09   ` Philippe Mathieu-Daudé
2021-03-09 11:58     ` David Edmondson
2021-03-09 12:06       ` Philippe Mathieu-Daudé
2021-03-09 13:07         ` Paolo Bonzini
2021-03-09 10:21 ` [RFC PATCH 2/4] block/vdi: Don't assume that blocks are larger than VdiHeader David Edmondson
2021-03-09 10:21 ` [RFC PATCH 3/4] coroutine/mutex: Store the coroutine in the CoWaitRecord only once David Edmondson
2021-03-09 10:49   ` Paolo Bonzini
2021-03-09 11:11   ` Philippe Mathieu-Daudé
2021-03-09 10:21 ` [RFC PATCH 4/4] coroutine/rwlock: Wake writers in preference to readers David Edmondson
2021-03-09 10:59   ` Paolo Bonzini
2021-03-09 11:06   ` Paolo Bonzini
2021-03-09 11:57     ` David Edmondson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2pn08h75d.fsf@dme.org \
    --to=dme@dme.org \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.