qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
	Adam Litke <agl@us.ibm.com>
Subject: [Qemu-devel] [PATCH 06/15] qed: avoid deadlock on emulated synchronous I/O
Date: Wed, 27 Jul 2011 14:44:46 +0100	[thread overview]
Message-ID: <1311774295-8696-7-git-send-email-stefanha@linux.vnet.ibm.com> (raw)
In-Reply-To: <1311774295-8696-1-git-send-email-stefanha@linux.vnet.ibm.com>

The block layer emulates synchronous bdrv_read()/bdrv_write() for
drivers that only provide the asynchronous interfaces.  The emulation
issues an asynchronous request inside a new "async context" and waits
for that request to complete.  If currently outstanding requests
complete during this time, their completion functions are not invoked
until the async context is popped again.

This can lead to deadlock if an allocating write is being processed when
synchronous I/O emulation starts.  The emulated synchronous write will
be queued because an existing request is being processed.  But the
existing request on cannot complete until the async context is popped.
The result is that qemu_aio_wait() sits in a deadlock.

Address this problem in two ways:

1. Add an assertion so that we instantly know if this corner case is
   hit.  This saves us time by giving a clear failure indication.

2. Ignore the copy-on-read hint for emulated synchronous reads.  This
   allows us to do emulated synchronous reads without hitting the
   deadlock.

Keep this as a separate commit instead of merging with previous QED
patches so it is easy to drop when coroutines are introduced and
eliminate async contexts.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qed.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 6ca57f2..ffdbc2d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1120,6 +1120,14 @@ static bool qed_start_allocating_write(QEDAIOCB *acb)
     }
     if (acb != QSIMPLEQ_FIRST(&s->allocating_write_reqs) ||
         s->allocating_write_reqs_plugged) {
+        /* Queuing an emulated synchronous write causes deadlock since
+         * currently outstanding requests are not in the current async context
+         * and their completion will never be invoked.  Once the block layer
+         * moves to truly asynchronous semantics this failure case will be
+         * eliminated.
+         */
+        assert(get_async_context_id() == 0);
+
         return false;
     }
     return true;
@@ -1246,7 +1254,9 @@ static void qed_aio_read_data(void *opaque, int ret,
     } else if (ret != QED_CLUSTER_FOUND) {
         BlockDriverCompletionFunc *cb = qed_aio_next_io;
 
-        if (bs->backing_hd && (acb->flags & QED_AIOCB_COPY_ON_READ)) {
+        /* See qed_start_allocating_write() for get_async_context_id() hack */
+        if (bs->backing_hd && (acb->flags & QED_AIOCB_COPY_ON_READ) &&
+            get_async_context_id() == 0) {
             if (!qed_start_allocating_write(acb)) {
                 qemu_iovec_reset(&acb->cur_qiov);
                 return; /* wait for current allocating write to complete */
-- 
1.7.5.4

  parent reply	other threads:[~2011-07-27 13:45 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-27 13:44 [Qemu-devel] [RFC v2 00/15] QED image streaming Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 01/15] block: add -drive copy-on-read=on|off Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 02/15] qed: replace is_write with flags field Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 03/15] qed: extract qed_start_allocating_write() Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 04/15] qed: make qed_aio_write_alloc() reusable Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 05/15] qed: add support for copy-on-read Stefan Hajnoczi
2011-07-27 13:44 ` Stefan Hajnoczi [this message]
2011-07-27 13:44 ` [Qemu-devel] [PATCH 07/15] block: add bdrv_aio_copy_backing() Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 08/15] qmp: add block_stream command Stefan Hajnoczi
2011-07-28 15:53   ` Marcelo Tosatti
2011-07-28 15:57     ` Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 09/15] qmp: add block_job_cancel command Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 10/15] qmp: add query-block-jobs command Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 11/15] qmp: add block_job_set_speed command Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 12/15] block: add -drive stream=on|off Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 13/15] qed: intelligent streaming implementation Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 14/15] trace: trace bdrv_aio_readv/writev error paths Stefan Hajnoczi
2011-07-27 13:44 ` [Qemu-devel] [PATCH 15/15] tests: add image streaming QMP interface tests 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=1311774295-8696-7-git-send-email-stefanha@linux.vnet.ibm.com \
    --to=stefanha@linux.vnet.ibm.com \
    --cc=agl@us.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).