]>
Commit | Line | Data |
---|---|---|
b45e13fe | 1 | From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
b45e13fe | 2 | From: Paolo Bonzini <pbonzini@redhat.com> |
507c6de3 WB |
3 | Date: Thu, 1 Jun 2017 12:44:56 +0200 |
4 | Subject: [PATCH] nbd: make it thread-safe, fix qcow2 over nbd | |
b45e13fe AD |
5 | |
6 | NBD is not thread safe, because it accesses s->in_flight without | |
7 | a CoMutex. Fixing this will be required for multiqueue. | |
8 | CoQueue doesn't have spurious wakeups but, when another coroutine can | |
9 | run between qemu_co_queue_next's wakeup and qemu_co_queue_wait's | |
10 | re-locking of the mutex, the wait condition can become false and | |
11 | a loop is necessary. | |
12 | ||
13 | In fact, it turns out that the loop is necessary even without this | |
14 | multi-threaded scenario. A particular sequence of coroutine wakeups | |
15 | is happening ~80% of the time when starting a guest with qcow2 image | |
16 | served over NBD (i.e. qemu-nbd --format=raw, and QEMU's -drive option | |
17 | has -format=qcow2). This patch fixes that issue too. | |
18 | ||
19 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | |
b45e13fe AD |
20 | --- |
21 | block/nbd-client.c | 30 +++++++++--------------------- | |
22 | 1 file changed, 9 insertions(+), 21 deletions(-) | |
23 | ||
24 | diff --git a/block/nbd-client.c b/block/nbd-client.c | |
507c6de3 | 25 | index 56eb0e2e16..282679b4f8 100644 |
b45e13fe AD |
26 | --- a/block/nbd-client.c |
27 | +++ b/block/nbd-client.c | |
28 | @@ -114,6 +114,10 @@ static int nbd_co_send_request(BlockDriverState *bs, | |
29 | int rc, ret, i; | |
30 | ||
31 | qemu_co_mutex_lock(&s->send_mutex); | |
32 | + while (s->in_flight == MAX_NBD_REQUESTS) { | |
33 | + qemu_co_queue_wait(&s->free_sema, &s->send_mutex); | |
34 | + } | |
35 | + s->in_flight++; | |
36 | ||
37 | for (i = 0; i < MAX_NBD_REQUESTS; i++) { | |
38 | if (s->recv_coroutine[i] == NULL) { | |
39 | @@ -176,20 +180,6 @@ static void nbd_co_receive_reply(NBDClientSession *s, | |
40 | } | |
41 | } | |
42 | ||
43 | -static void nbd_coroutine_start(NBDClientSession *s, | |
44 | - NBDRequest *request) | |
45 | -{ | |
46 | - /* Poor man semaphore. The free_sema is locked when no other request | |
47 | - * can be accepted, and unlocked after receiving one reply. */ | |
48 | - if (s->in_flight == MAX_NBD_REQUESTS) { | |
49 | - qemu_co_queue_wait(&s->free_sema, NULL); | |
50 | - assert(s->in_flight < MAX_NBD_REQUESTS); | |
51 | - } | |
52 | - s->in_flight++; | |
53 | - | |
54 | - /* s->recv_coroutine[i] is set as soon as we get the send_lock. */ | |
55 | -} | |
56 | - | |
57 | static void nbd_coroutine_end(BlockDriverState *bs, | |
58 | NBDRequest *request) | |
59 | { | |
60 | @@ -197,13 +187,16 @@ static void nbd_coroutine_end(BlockDriverState *bs, | |
61 | int i = HANDLE_TO_INDEX(s, request->handle); | |
62 | ||
63 | s->recv_coroutine[i] = NULL; | |
64 | - s->in_flight--; | |
65 | - qemu_co_queue_next(&s->free_sema); | |
66 | ||
67 | /* Kick the read_reply_co to get the next reply. */ | |
68 | if (s->read_reply_co) { | |
69 | aio_co_wake(s->read_reply_co); | |
70 | } | |
71 | + | |
72 | + qemu_co_mutex_lock(&s->send_mutex); | |
73 | + s->in_flight--; | |
74 | + qemu_co_queue_next(&s->free_sema); | |
75 | + qemu_co_mutex_unlock(&s->send_mutex); | |
76 | } | |
77 | ||
78 | int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset, | |
79 | @@ -221,7 +214,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset, | |
80 | assert(bytes <= NBD_MAX_BUFFER_SIZE); | |
81 | assert(!flags); | |
82 | ||
83 | - nbd_coroutine_start(client, &request); | |
84 | ret = nbd_co_send_request(bs, &request, NULL); | |
85 | if (ret < 0) { | |
86 | reply.error = -ret; | |
87 | @@ -251,7 +243,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset, | |
88 | ||
89 | assert(bytes <= NBD_MAX_BUFFER_SIZE); | |
90 | ||
91 | - nbd_coroutine_start(client, &request); | |
92 | ret = nbd_co_send_request(bs, &request, qiov); | |
93 | if (ret < 0) { | |
94 | reply.error = -ret; | |
95 | @@ -286,7 +277,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, | |
96 | request.flags |= NBD_CMD_FLAG_NO_HOLE; | |
97 | } | |
98 | ||
99 | - nbd_coroutine_start(client, &request); | |
100 | ret = nbd_co_send_request(bs, &request, NULL); | |
101 | if (ret < 0) { | |
102 | reply.error = -ret; | |
103 | @@ -311,7 +301,6 @@ int nbd_client_co_flush(BlockDriverState *bs) | |
104 | request.from = 0; | |
105 | request.len = 0; | |
106 | ||
107 | - nbd_coroutine_start(client, &request); | |
108 | ret = nbd_co_send_request(bs, &request, NULL); | |
109 | if (ret < 0) { | |
110 | reply.error = -ret; | |
111 | @@ -337,7 +326,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count) | |
112 | return 0; | |
113 | } | |
114 | ||
115 | - nbd_coroutine_start(client, &request); | |
116 | ret = nbd_co_send_request(bs, &request, NULL); | |
117 | if (ret < 0) { | |
118 | reply.error = -ret; | |
119 | -- | |
507c6de3 | 120 | 2.11.0 |
b45e13fe | 121 |