All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Greg Kurz <groug@kaod.org>
Cc: qemu-devel@nongnu.org, Vivek Goyal <vgoyal@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH] virtiofsd: vu_dispatch locking should never fail
Date: Wed, 3 Feb 2021 14:57:23 +0000	[thread overview]
Message-ID: <20210203145723.GF74271@stefanha-x1.localdomain> (raw)
In-Reply-To: <20210129155312.595980-1-groug@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

On Fri, Jan 29, 2021 at 04:53:12PM +0100, Greg Kurz wrote:
> pthread_rwlock_rdlock() and pthread_rwlock_wrlock() can fail if a
> deadlock condition is detected or the current thread already owns
> the lock. They can also fail, like pthread_rwlock_unlock(), if the
> mutex wasn't properly initialized. None of these are ever expected
> to happen with fv_VuDev::vu_dispatch_rwlock.
> 
> Some users already check the return value and assert, some others
> don't. Introduce rdlock/wrlock/unlock wrappers that just do the
> former and use them everywhere.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

What is the purpose of this commit:
1. Code cleanup/consistency?
2. Helps debug an issue that doesn't occur in production but you hit
   during development?
3. Needed to diagnose a real-world issue? How do you reproduce it?

I wanted to check to make sure I'm not missing an issue you found with
production workloads.

> @@ -645,8 +662,7 @@ static void *fv_queue_thread(void *opaque)
>              break;
>          }
>          /* Mutual exclusion with virtio_loop() */
> -        ret = pthread_rwlock_rdlock(&qi->virtio_dev->vu_dispatch_rwlock);
> -        assert(ret == 0); /* there is no possible error case */
> +        vu_dispatch_wrlock(qi->virtio_dev);

s/vu_dispatch_wrlock/vu_dispatch_rdlock/ ?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2021-02-03 14:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-29 15:53 [PATCH] virtiofsd: vu_dispatch locking should never fail Greg Kurz
2021-02-03 14:57 ` Stefan Hajnoczi [this message]
2021-02-03 15:35   ` Greg Kurz
2021-02-03 15:59 ` Vivek Goyal
2021-02-03 16:08   ` Greg Kurz
2021-02-03 16:29     ` Vivek Goyal
2021-02-03 16:53       ` Stefan Hajnoczi

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=20210203145723.GF74271@stefanha-x1.localdomain \
    --to=stefanha@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vgoyal@redhat.com \
    /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.