1
The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def:
1
The following changes since commit b0fbe46ad82982b289a44ee2495b59b0bad8a842:
2
2
3
Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700)
3
Update version for v2.11.0-rc0 release (2017-11-07 16:05:28 +0000)
4
4
5
are available in the Git repository at:
5
are available in the git repository at:
6
6
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
7
git://github.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 1cc7eada97914f090125e588497986f6f7900514:
9
for you to fetch changes up to ef6dada8b44e1e7c4bec5c1115903af9af415b50:
10
10
11
iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100)
11
util/async: use atomic_mb_set in qemu_bh_cancel (2017-11-08 19:09:15 +0000)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
v2:
17
* v1 emails 2/3 and 3/3 weren't sent due to an email failure
18
* Included Sergio's updated wording in the commit description
19
16
----------------------------------------------------------------
20
----------------------------------------------------------------
17
21
18
Stefano Garzarella (2):
22
Sergio Lopez (1):
19
iothread: rename PollParamInfo to IOThreadParamInfo
23
util/async: use atomic_mb_set in qemu_bh_cancel
20
iothread: use IOThreadParamInfo in iothread_[set|get]_param()
21
24
22
iothread.c | 28 +++++++++++++++-------------
25
Stefan Hajnoczi (1):
23
1 file changed, 15 insertions(+), 13 deletions(-)
26
tests-aio-multithread: fix /aio/multi/schedule race condition
27
28
tests/test-aio-multithread.c | 5 ++---
29
util/async.c | 2 +-
30
2 files changed, 3 insertions(+), 4 deletions(-)
24
31
25
--
32
--
26
2.31.1
33
2.13.6
27
34
28
35
29
diff view generated by jsdifflib
1
From: Stefano Garzarella <sgarzare@redhat.com>
1
test_multi_co_schedule_entry() set to_schedule[id] in the final loop
2
iteration before terminating the coroutine. There is a race condition
3
where the main thread attempts to enter the terminating or terminated
4
coroutine when signalling coroutines to stop:
2
5
3
Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
6
atomic_mb_set(&now_stopping, true);
4
a new parameter (aio-max-batch) to IOThread and used PollParamInfo
7
for (i = 0; i < NUM_CONTEXTS; i++) {
5
structure to handle it.
8
ctx_run(i, finish_cb, NULL); <--- enters dead coroutine!
9
to_schedule[i] = NULL;
10
}
6
11
7
Since it is not a parameter of the polling mechanism, we rename the
12
Make sure only to set to_schedule[id] if this coroutine really needs to
8
structure to a more generic IOThreadParamInfo.
13
be scheduled!
9
14
10
Suggested-by: Kevin Wolf <kwolf@redhat.com>
15
Reported-by: "R.Nageswara Sastry" <nasastry@in.ibm.com>
11
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
16
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
12
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
17
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
13
Message-id: 20210727145936.147032-2-sgarzare@redhat.com
18
Message-id: 20171106190233.1175-1-stefanha@redhat.com
14
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
19
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
15
---
20
---
16
iothread.c | 14 +++++++-------
21
tests/test-aio-multithread.c | 5 ++---
17
1 file changed, 7 insertions(+), 7 deletions(-)
22
1 file changed, 2 insertions(+), 3 deletions(-)
18
23
19
diff --git a/iothread.c b/iothread.c
24
diff --git a/tests/test-aio-multithread.c b/tests/test-aio-multithread.c
20
index XXXXXXX..XXXXXXX 100644
25
index XXXXXXX..XXXXXXX 100644
21
--- a/iothread.c
26
--- a/tests/test-aio-multithread.c
22
+++ b/iothread.c
27
+++ b/tests/test-aio-multithread.c
23
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
28
@@ -XXX,XX +XXX,XX @@ static void finish_cb(void *opaque)
24
typedef struct {
29
static coroutine_fn void test_multi_co_schedule_entry(void *opaque)
25
const char *name;
26
ptrdiff_t offset; /* field's byte offset in IOThread struct */
27
-} PollParamInfo;
28
+} IOThreadParamInfo;
29
30
-static PollParamInfo poll_max_ns_info = {
31
+static IOThreadParamInfo poll_max_ns_info = {
32
"poll-max-ns", offsetof(IOThread, poll_max_ns),
33
};
34
-static PollParamInfo poll_grow_info = {
35
+static IOThreadParamInfo poll_grow_info = {
36
"poll-grow", offsetof(IOThread, poll_grow),
37
};
38
-static PollParamInfo poll_shrink_info = {
39
+static IOThreadParamInfo poll_shrink_info = {
40
"poll-shrink", offsetof(IOThread, poll_shrink),
41
};
42
-static PollParamInfo aio_max_batch_info = {
43
+static IOThreadParamInfo aio_max_batch_info = {
44
"aio-max-batch", offsetof(IOThread, aio_max_batch),
45
};
46
47
@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
48
const char *name, void *opaque, Error **errp)
49
{
30
{
50
IOThread *iothread = IOTHREAD(obj);
31
g_assert(to_schedule[id] == NULL);
51
- PollParamInfo *info = opaque;
32
- atomic_mb_set(&to_schedule[id], qemu_coroutine_self());
52
+ IOThreadParamInfo *info = opaque;
33
53
int64_t *field = (void *)iothread + info->offset;
34
while (!atomic_mb_read(&now_stopping)) {
54
35
int n;
55
visit_type_int64(v, name, field, errp);
36
56
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
37
n = g_test_rand_int_range(0, NUM_CONTEXTS);
57
const char *name, void *opaque, Error **errp)
38
schedule_next(n);
58
{
39
+
59
IOThread *iothread = IOTHREAD(obj);
40
+ atomic_mb_set(&to_schedule[id], qemu_coroutine_self());
60
- PollParamInfo *info = opaque;
41
qemu_coroutine_yield();
61
+ IOThreadParamInfo *info = opaque;
42
-
62
int64_t *field = (void *)iothread + info->offset;
43
g_assert(to_schedule[id] == NULL);
63
int64_t value;
44
- atomic_mb_set(&to_schedule[id], qemu_coroutine_self());
45
}
46
}
64
47
65
--
48
--
66
2.31.1
49
2.13.6
67
50
68
51
diff view generated by jsdifflib
1
From: Stefano Garzarella <sgarzare@redhat.com>
1
From: Sergio Lopez <slp@redhat.com>
2
2
3
Commit 0445409d74 ("iothread: generalize
3
Commit b7a745d added a qemu_bh_cancel call to the completion function
4
iothread_set_param/iothread_get_param") moved common code to set and
4
as an optimization to prevent it from unnecessarily rescheduling itself.
5
get IOThread parameters in two new functions.
6
5
7
These functions are called inside callbacks, so we don't need to use an
6
This completion function is scheduled from worker_thread, after setting
8
opaque pointer. Let's replace `void *opaque` parameter with
7
the state of a ThreadPoolElement to THREAD_DONE.
9
`IOThreadParamInfo *info`.
10
8
11
Suggested-by: Kevin Wolf <kwolf@redhat.com>
9
This was considered to be safe, as the completion function restarts the
12
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
10
loop just after the call to qemu_bh_cancel. But, as this loop lacks a HW
13
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
11
memory barrier, the read of req->state may actually happen _before_ the
14
Message-id: 20210727145936.147032-3-sgarzare@redhat.com
12
call, seeing it still as THREAD_QUEUED, and ending the completion
13
function without having processed a pending TPE linked at pool->head:
14
15
worker thread | I/O thread
16
------------------------------------------------------------------------
17
| speculatively read req->state
18
req->state = THREAD_DONE; |
19
qemu_bh_schedule(p->completion_bh) |
20
bh->scheduled = 1; |
21
| qemu_bh_cancel(p->completion_bh)
22
| bh->scheduled = 0;
23
| if (req->state == THREAD_DONE)
24
| // sees THREAD_QUEUED
25
26
The source of the misunderstanding was that qemu_bh_cancel is now being
27
used by the _consumer_ rather than the producer, and therefore now needs
28
to have acquire semantics just like e.g. aio_bh_poll.
29
30
In some situations, if there are no other independent requests in the
31
same aio context that could eventually trigger the scheduling of the
32
completion function, the omitted TPE and all operations pending on it
33
will get stuck forever.
34
35
[Added Sergio's updated wording about the HW memory barrier.
36
--Stefan]
37
38
Signed-off-by: Sergio Lopez <slp@redhat.com>
39
Message-id: 20171108063447.2842-1-slp@redhat.com
15
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
40
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
16
---
41
---
17
iothread.c | 18 ++++++++++--------
42
util/async.c | 2 +-
18
1 file changed, 10 insertions(+), 8 deletions(-)
43
1 file changed, 1 insertion(+), 1 deletion(-)
19
44
20
diff --git a/iothread.c b/iothread.c
45
diff --git a/util/async.c b/util/async.c
21
index XXXXXXX..XXXXXXX 100644
46
index XXXXXXX..XXXXXXX 100644
22
--- a/iothread.c
47
--- a/util/async.c
23
+++ b/iothread.c
48
+++ b/util/async.c
24
@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
49
@@ -XXX,XX +XXX,XX @@ void qemu_bh_schedule(QEMUBH *bh)
25
};
50
*/
26
51
void qemu_bh_cancel(QEMUBH *bh)
27
static void iothread_get_param(Object *obj, Visitor *v,
28
- const char *name, void *opaque, Error **errp)
29
+ const char *name, IOThreadParamInfo *info, Error **errp)
30
{
52
{
31
IOThread *iothread = IOTHREAD(obj);
53
- bh->scheduled = 0;
32
- IOThreadParamInfo *info = opaque;
54
+ atomic_mb_set(&bh->scheduled, 0);
33
int64_t *field = (void *)iothread + info->offset;
34
35
visit_type_int64(v, name, field, errp);
36
}
55
}
37
56
38
static bool iothread_set_param(Object *obj, Visitor *v,
57
/* This func is async.The bottom half will do the delete action at the finial
39
- const char *name, void *opaque, Error **errp)
40
+ const char *name, IOThreadParamInfo *info, Error **errp)
41
{
42
IOThread *iothread = IOTHREAD(obj);
43
- IOThreadParamInfo *info = opaque;
44
int64_t *field = (void *)iothread + info->offset;
45
int64_t value;
46
47
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
48
static void iothread_get_poll_param(Object *obj, Visitor *v,
49
const char *name, void *opaque, Error **errp)
50
{
51
+ IOThreadParamInfo *info = opaque;
52
53
- iothread_get_param(obj, v, name, opaque, errp);
54
+ iothread_get_param(obj, v, name, info, errp);
55
}
56
57
static void iothread_set_poll_param(Object *obj, Visitor *v,
58
const char *name, void *opaque, Error **errp)
59
{
60
IOThread *iothread = IOTHREAD(obj);
61
+ IOThreadParamInfo *info = opaque;
62
63
- if (!iothread_set_param(obj, v, name, opaque, errp)) {
64
+ if (!iothread_set_param(obj, v, name, info, errp)) {
65
return;
66
}
67
68
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
69
static void iothread_get_aio_param(Object *obj, Visitor *v,
70
const char *name, void *opaque, Error **errp)
71
{
72
+ IOThreadParamInfo *info = opaque;
73
74
- iothread_get_param(obj, v, name, opaque, errp);
75
+ iothread_get_param(obj, v, name, info, errp);
76
}
77
78
static void iothread_set_aio_param(Object *obj, Visitor *v,
79
const char *name, void *opaque, Error **errp)
80
{
81
IOThread *iothread = IOTHREAD(obj);
82
+ IOThreadParamInfo *info = opaque;
83
84
- if (!iothread_set_param(obj, v, name, opaque, errp)) {
85
+ if (!iothread_set_param(obj, v, name, info, errp)) {
86
return;
87
}
88
89
--
58
--
90
2.31.1
59
2.13.6
91
60
92
61
diff view generated by jsdifflib