Re: [Qemu-devel] [PATCH v3 19/29] block: Allow wait_serialising_requests() at any point

2014-01-22 Thread Benoît Canet 
Le Friday 17 Jan 2014 à 15:15:09 (+0100), Kevin Wolf a écrit :
 We can only have a single wait_serialising_requests() call per request
 because otherwise we can run into deadlocks where requests are waiting
 for each other. The same is true when wait_serialising_requests() is not
 at the very beginning of a request, so that other requests can be issued
 between the start of the tracking and wait_serialising_requests().
 
 Fix this by changing wait_serialising_requests() to ignore requests that
 are already (directly or indirectly) waiting for the calling request.
 
 Signed-off-by: Kevin Wolf kw...@redhat.com
 Reviewed-by: Max Reitz mre...@redhat.com
 ---
  block.c   | 13 ++---
  include/block/block_int.h |  2 ++
  2 files changed, 12 insertions(+), 3 deletions(-)
 
 diff --git a/block.c b/block.c
 index e72966a..55e8c69 100644
 --- a/block.c
 +++ b/block.c
 @@ -2148,9 +2148,16 @@ static void coroutine_fn 
 wait_serialising_requests(BdrvTrackedRequest *self)
   */
  assert(qemu_coroutine_self() != req-co);
  
 -qemu_co_queue_wait(req-wait_queue);
 -retry = true;
 -break;
 +/* If the request is already (indirectly) waiting for us, or
 + * will wait for us as soon as it wakes up, then just go on
 + * (instead of producing a deadlock in the former case). */
 +if (!req-waiting_for) {
 +self-waiting_for = req;
 +qemu_co_queue_wait(req-wait_queue);
 +self-waiting_for = NULL;
 +retry = true;
 +break;
 +}
  }
  }
  } while (retry);
 diff --git a/include/block/block_int.h b/include/block/block_int.h
 index ccd2c68..fdf0e0b 100644
 --- a/include/block/block_int.h
 +++ b/include/block/block_int.h
 @@ -68,6 +68,8 @@ typedef struct BdrvTrackedRequest {
  QLIST_ENTRY(BdrvTrackedRequest) list;
  Coroutine *co; /* owner, used for deadlock detection */
  CoQueue wait_queue; /* coroutines blocked on this request */
 +
 +struct BdrvTrackedRequest *waiting_for;
  } BdrvTrackedRequest;
  
  struct BlockDriver {
 -- 
 1.8.1.4
 
 
Reviewed-by: Benoit Canet ben...@irqsave.net

[Qemu-devel] [PATCH v3 19/29] block: Allow wait_serialising_requests() at any point

2014-01-17 Thread Kevin Wolf 
We can only have a single wait_serialising_requests() call per request
because otherwise we can run into deadlocks where requests are waiting
for each other. The same is true when wait_serialising_requests() is not
at the very beginning of a request, so that other requests can be issued
between the start of the tracking and wait_serialising_requests().

Fix this by changing wait_serialising_requests() to ignore requests that
are already (directly or indirectly) waiting for the calling request.

Signed-off-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Max Reitz mre...@redhat.com
---
 block.c   | 13 ++---
 include/block/block_int.h |  2 ++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index e72966a..55e8c69 100644
--- a/block.c
+++ b/block.c
@@ -2148,9 +2148,16 @@ static void coroutine_fn 
wait_serialising_requests(BdrvTrackedRequest *self)
  */
 assert(qemu_coroutine_self() != req-co);
 
-qemu_co_queue_wait(req-wait_queue);
-retry = true;
-break;
+/* If the request is already (indirectly) waiting for us, or
+ * will wait for us as soon as it wakes up, then just go on
+ * (instead of producing a deadlock in the former case). */
+if (!req-waiting_for) {
+self-waiting_for = req;
+qemu_co_queue_wait(req-wait_queue);
+self-waiting_for = NULL;
+retry = true;
+break;
+}
 }
 }
 } while (retry);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ccd2c68..fdf0e0b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -68,6 +68,8 @@ typedef struct BdrvTrackedRequest {
 QLIST_ENTRY(BdrvTrackedRequest) list;
 Coroutine *co; /* owner, used for deadlock detection */
 CoQueue wait_queue; /* coroutines blocked on this request */
+
+struct BdrvTrackedRequest *waiting_for;
 } BdrvTrackedRequest;
 
 struct BlockDriver {
-- 
1.8.1.4