* [Qemu-devel] [PATCH 02/10] coroutine: rename unlock_bh_queue to co_runnable_queue
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:30 ` Paolo Bonzini
2012-04-03 8:38 ` [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable() Lai Jiangshan
` (8 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
It stands for runnable Coroutines.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine-lock.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 26ad76b..10e8dbb 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -29,17 +29,17 @@
#include "main-loop.h"
#include "trace.h"
-static QTAILQ_HEAD(, Coroutine) unlock_bh_queue =
- QTAILQ_HEAD_INITIALIZER(unlock_bh_queue);
-static QEMUBH* unlock_bh;
+static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
+ QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
+static QEMUBH* co_runnable_bh;
static void qemu_co_queue_next_bh(void *opaque)
{
Coroutine *next;
trace_qemu_co_queue_next_bh();
- while ((next = QTAILQ_FIRST(&unlock_bh_queue))) {
- QTAILQ_REMOVE(&unlock_bh_queue, next, co_queue_next);
+ while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
+ QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
qemu_coroutine_enter(next, NULL);
}
}
@@ -48,8 +48,8 @@ void qemu_co_queue_init(CoQueue *queue)
{
QTAILQ_INIT(&queue->entries);
- if (!unlock_bh) {
- unlock_bh = qemu_bh_new(qemu_co_queue_next_bh, NULL);
+ if (!co_runnable_bh) {
+ co_runnable_bh = qemu_bh_new(qemu_co_queue_next_bh, NULL);
}
}
@@ -76,9 +76,9 @@ bool qemu_co_queue_next(CoQueue *queue)
next = QTAILQ_FIRST(&queue->entries);
if (next) {
QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
- QTAILQ_INSERT_TAIL(&unlock_bh_queue, next, co_queue_next);
+ QTAILQ_INSERT_TAIL(&co_runnable_queue, next, co_queue_next);
trace_qemu_co_queue_next(next);
- qemu_bh_schedule(unlock_bh);
+ qemu_bh_schedule(co_runnable_bh);
}
return (next != NULL);
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 02/10] coroutine: rename unlock_bh_queue to co_runnable_queue
2012-04-03 8:38 ` [Qemu-devel] [PATCH 02/10] coroutine: rename unlock_bh_queue to co_runnable_queue Lai Jiangshan
@ 2012-04-03 11:30 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:30 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> It stands for runnable Coroutines.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-lock.c | 18 +++++++++---------
> 1 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 26ad76b..10e8dbb 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -29,17 +29,17 @@
> #include "main-loop.h"
> #include "trace.h"
>
> -static QTAILQ_HEAD(, Coroutine) unlock_bh_queue =
> - QTAILQ_HEAD_INITIALIZER(unlock_bh_queue);
> -static QEMUBH* unlock_bh;
> +static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
> + QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
> +static QEMUBH* co_runnable_bh;
>
> static void qemu_co_queue_next_bh(void *opaque)
> {
> Coroutine *next;
>
> trace_qemu_co_queue_next_bh();
> - while ((next = QTAILQ_FIRST(&unlock_bh_queue))) {
> - QTAILQ_REMOVE(&unlock_bh_queue, next, co_queue_next);
> + while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
> + QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
> qemu_coroutine_enter(next, NULL);
> }
> }
> @@ -48,8 +48,8 @@ void qemu_co_queue_init(CoQueue *queue)
> {
> QTAILQ_INIT(&queue->entries);
>
> - if (!unlock_bh) {
> - unlock_bh = qemu_bh_new(qemu_co_queue_next_bh, NULL);
> + if (!co_runnable_bh) {
> + co_runnable_bh = qemu_bh_new(qemu_co_queue_next_bh, NULL);
> }
> }
>
> @@ -76,9 +76,9 @@ bool qemu_co_queue_next(CoQueue *queue)
> next = QTAILQ_FIRST(&queue->entries);
> if (next) {
> QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
> - QTAILQ_INSERT_TAIL(&unlock_bh_queue, next, co_queue_next);
> + QTAILQ_INSERT_TAIL(&co_runnable_queue, next, co_queue_next);
> trace_qemu_co_queue_next(next);
> - qemu_bh_schedule(unlock_bh);
> + qemu_bh_schedule(co_runnable_bh);
> }
>
> return (next != NULL);
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable()
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
2012-04-03 8:38 ` [Qemu-devel] [PATCH 02/10] coroutine: rename unlock_bh_queue to co_runnable_queue Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:31 ` Paolo Bonzini
2012-04-03 11:43 ` Kevin Wolf
2012-04-03 8:38 ` [Qemu-devel] [PATCH 04/10] coroutine: init co_runnable_bh during boot Lai Jiangshan
` (7 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine-lock.c | 6 +++---
trace-events | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 10e8dbb..90141cd 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -33,11 +33,11 @@ static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
static QEMUBH* co_runnable_bh;
-static void qemu_co_queue_next_bh(void *opaque)
+static void qemu_co_process_runnable(void *opaque)
{
Coroutine *next;
- trace_qemu_co_queue_next_bh();
+ trace_qemu_co_process_runnable();
while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
qemu_coroutine_enter(next, NULL);
@@ -49,7 +49,7 @@ void qemu_co_queue_init(CoQueue *queue)
QTAILQ_INIT(&queue->entries);
if (!co_runnable_bh) {
- co_runnable_bh = qemu_bh_new(qemu_co_queue_next_bh, NULL);
+ co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
}
}
diff --git a/trace-events b/trace-events
index 70f059d..a1737c2 100644
--- a/trace-events
+++ b/trace-events
@@ -561,7 +561,7 @@ qemu_coroutine_yield(void *from, void *to) "from %p to %p"
qemu_coroutine_terminate(void *co) "self %p"
# qemu-coroutine-lock.c
-qemu_co_queue_next_bh(void) ""
+qemu_co_process_runnable(void) ""
qemu_co_queue_next(void *next) "next %p"
qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable()
2012-04-03 8:38 ` [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable() Lai Jiangshan
@ 2012-04-03 11:31 ` Paolo Bonzini
2012-04-03 11:43 ` Kevin Wolf
1 sibling, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:31 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-lock.c | 6 +++---
> trace-events | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 10e8dbb..90141cd 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -33,11 +33,11 @@ static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
> QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
> static QEMUBH* co_runnable_bh;
>
> -static void qemu_co_queue_next_bh(void *opaque)
> +static void qemu_co_process_runnable(void *opaque)
> {
> Coroutine *next;
>
> - trace_qemu_co_queue_next_bh();
> + trace_qemu_co_process_runnable();
> while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
> QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
> qemu_coroutine_enter(next, NULL);
> @@ -49,7 +49,7 @@ void qemu_co_queue_init(CoQueue *queue)
> QTAILQ_INIT(&queue->entries);
>
> if (!co_runnable_bh) {
> - co_runnable_bh = qemu_bh_new(qemu_co_queue_next_bh, NULL);
> + co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
> }
> }
>
> diff --git a/trace-events b/trace-events
> index 70f059d..a1737c2 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -561,7 +561,7 @@ qemu_coroutine_yield(void *from, void *to) "from %p to %p"
> qemu_coroutine_terminate(void *co) "self %p"
>
> # qemu-coroutine-lock.c
> -qemu_co_queue_next_bh(void) ""
> +qemu_co_process_runnable(void) ""
> qemu_co_queue_next(void *next) "next %p"
> qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
> qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable()
2012-04-03 8:38 ` [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable() Lai Jiangshan
2012-04-03 11:31 ` Paolo Bonzini
@ 2012-04-03 11:43 ` Kevin Wolf
1 sibling, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2012-04-03 11:43 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Anthony Liguori, Stefan Hajnoczi, qemu-devel
Am 03.04.2012 10:38, schrieb Lai Jiangshan:
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-lock.c | 6 +++---
> trace-events | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 10e8dbb..90141cd 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -33,11 +33,11 @@ static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
> QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
> static QEMUBH* co_runnable_bh;
>
> -static void qemu_co_queue_next_bh(void *opaque)
> +static void qemu_co_process_runnable(void *opaque)
Please keep the _bh suffix there, so that it's obvious that this is used
as a bottom half.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 04/10] coroutine: init co_runnable_bh during boot
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
2012-04-03 8:38 ` [Qemu-devel] [PATCH 02/10] coroutine: rename unlock_bh_queue to co_runnable_queue Lai Jiangshan
2012-04-03 8:38 ` [Qemu-devel] [PATCH 03/10] coroutine: rename qemu_co_queue_next_bh() to qemu_co_process_runnable() Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:31 ` Paolo Bonzini
2012-04-03 8:38 ` [Qemu-devel] [PATCH 05/10] coroutine: add qemu_co_runnable_schedule() Lai Jiangshan
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
use __attribute__((constructor)) to do the initialization.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine-lock.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 90141cd..7c29bc4 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -44,13 +44,14 @@ static void qemu_co_process_runnable(void *opaque)
}
}
+static void __attribute__((constructor)) co_runnable_bh_init(void)
+{
+ co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
+}
+
void qemu_co_queue_init(CoQueue *queue)
{
QTAILQ_INIT(&queue->entries);
-
- if (!co_runnable_bh) {
- co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
- }
}
void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 04/10] coroutine: init co_runnable_bh during boot
2012-04-03 8:38 ` [Qemu-devel] [PATCH 04/10] coroutine: init co_runnable_bh during boot Lai Jiangshan
@ 2012-04-03 11:31 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:31 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> use __attribute__((constructor)) to do the initialization.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-lock.c | 9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 90141cd..7c29bc4 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -44,13 +44,14 @@ static void qemu_co_process_runnable(void *opaque)
> }
> }
>
> +static void __attribute__((constructor)) co_runnable_bh_init(void)
> +{
> + co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
> +}
> +
> void qemu_co_queue_init(CoQueue *queue)
> {
> QTAILQ_INIT(&queue->entries);
> -
> - if (!co_runnable_bh) {
> - co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
> - }
> }
>
> void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 05/10] coroutine: add qemu_co_runnable_schedule()
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (2 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 04/10] coroutine: init co_runnable_bh during boot Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:31 ` Paolo Bonzini
2012-04-03 8:38 ` [Qemu-devel] [PATCH 06/10] coroutine: move runnale coroutine code to qemu-coroutine.c Lai Jiangshan
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
split qemu_co_queue_next() as two parts:
the first part is dequeuing next from the wait queue.
the second part is schedule it to the runnable queue(qemu_co_runnable_schedule())
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine-lock.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 7c29bc4..6f47685 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -44,6 +44,12 @@ static void qemu_co_process_runnable(void *opaque)
}
}
+static void qemu_co_runnable_schedule(Coroutine *co)
+{
+ QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
+ qemu_bh_schedule(co_runnable_bh);
+}
+
static void __attribute__((constructor)) co_runnable_bh_init(void)
{
co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
@@ -77,9 +83,8 @@ bool qemu_co_queue_next(CoQueue *queue)
next = QTAILQ_FIRST(&queue->entries);
if (next) {
QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
- QTAILQ_INSERT_TAIL(&co_runnable_queue, next, co_queue_next);
trace_qemu_co_queue_next(next);
- qemu_bh_schedule(co_runnable_bh);
+ qemu_co_runnable_schedule(next);
}
return (next != NULL);
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 05/10] coroutine: add qemu_co_runnable_schedule()
2012-04-03 8:38 ` [Qemu-devel] [PATCH 05/10] coroutine: add qemu_co_runnable_schedule() Lai Jiangshan
@ 2012-04-03 11:31 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:31 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> split qemu_co_queue_next() as two parts:
> the first part is dequeuing next from the wait queue.
> the second part is schedule it to the runnable queue(qemu_co_runnable_schedule())
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-lock.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 7c29bc4..6f47685 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -44,6 +44,12 @@ static void qemu_co_process_runnable(void *opaque)
> }
> }
>
> +static void qemu_co_runnable_schedule(Coroutine *co)
> +{
> + QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
> + qemu_bh_schedule(co_runnable_bh);
> +}
> +
> static void __attribute__((constructor)) co_runnable_bh_init(void)
> {
> co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
> @@ -77,9 +83,8 @@ bool qemu_co_queue_next(CoQueue *queue)
> next = QTAILQ_FIRST(&queue->entries);
> if (next) {
> QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
> - QTAILQ_INSERT_TAIL(&co_runnable_queue, next, co_queue_next);
> trace_qemu_co_queue_next(next);
> - qemu_bh_schedule(co_runnable_bh);
> + qemu_co_runnable_schedule(next);
> }
>
> return (next != NULL);
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 06/10] coroutine: move runnale coroutine code to qemu-coroutine.c
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (3 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 05/10] coroutine: add qemu_co_runnable_schedule() Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:32 ` Paolo Bonzini
2012-04-03 8:38 ` [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c Lai Jiangshan
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
runnable coroutine queue is the core mangement of the coroutine.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine-lock.c | 27 ---------------------------
qemu-coroutine.c | 27 +++++++++++++++++++++++++++
qemu-coroutine.h | 7 +++++++
trace-events | 2 +-
4 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 6f47685..159d66d 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -26,35 +26,8 @@
#include "qemu-coroutine.h"
#include "qemu-coroutine-int.h"
#include "qemu-queue.h"
-#include "main-loop.h"
#include "trace.h"
-static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
- QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
-static QEMUBH* co_runnable_bh;
-
-static void qemu_co_process_runnable(void *opaque)
-{
- Coroutine *next;
-
- trace_qemu_co_process_runnable();
- while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
- QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
- qemu_coroutine_enter(next, NULL);
- }
-}
-
-static void qemu_co_runnable_schedule(Coroutine *co)
-{
- QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
- qemu_bh_schedule(co_runnable_bh);
-}
-
-static void __attribute__((constructor)) co_runnable_bh_init(void)
-{
- co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
-}
-
void qemu_co_queue_init(CoQueue *queue)
{
QTAILQ_INIT(&queue->entries);
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index c01252e..c67d557 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -16,6 +16,33 @@
#include "qemu-common.h"
#include "qemu-coroutine.h"
#include "qemu-coroutine-int.h"
+#include "main-loop.h"
+
+static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
+ QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
+static QEMUBH* co_runnable_bh;
+
+static void qemu_co_process_runnable(void *opaque)
+{
+ Coroutine *next;
+
+ trace_qemu_co_process_runnable();
+ while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
+ QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
+ qemu_coroutine_enter(next, NULL);
+ }
+}
+
+void qemu_co_runnable_schedule(Coroutine *co)
+{
+ QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
+ qemu_bh_schedule(co_runnable_bh);
+}
+
+static void __attribute__((constructor)) co_runnable_bh_init(void)
+{
+ co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
+}
Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
{
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 34c15d4..82a7e5c 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -82,6 +82,13 @@ void qemu_coroutine_enter(Coroutine *coroutine, void *opaque);
void coroutine_fn qemu_coroutine_yield(void);
/**
+ * Add a coroutine to runnable to runnable queue
+ *
+ * The coroutine will be processed later.
+ */
+void qemu_co_runnable_schedule(Coroutine *co);
+
+/**
* Get the currently executing coroutine
*/
Coroutine *coroutine_fn qemu_coroutine_self(void);
diff --git a/trace-events b/trace-events
index a1737c2..39b9c84 100644
--- a/trace-events
+++ b/trace-events
@@ -556,12 +556,12 @@ qemu_put_ram_ptr(void* addr) "%p"
xen_platform_log(char *s) "xen platform: %s"
# qemu-coroutine.c
+qemu_co_process_runnable(void) ""
qemu_coroutine_enter(void *from, void *to, void *opaque) "from %p to %p opaque %p"
qemu_coroutine_yield(void *from, void *to) "from %p to %p"
qemu_coroutine_terminate(void *co) "self %p"
# qemu-coroutine-lock.c
-qemu_co_process_runnable(void) ""
qemu_co_queue_next(void *next) "next %p"
qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 06/10] coroutine: move runnale coroutine code to qemu-coroutine.c
2012-04-03 8:38 ` [Qemu-devel] [PATCH 06/10] coroutine: move runnale coroutine code to qemu-coroutine.c Lai Jiangshan
@ 2012-04-03 11:32 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:32 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> runnable coroutine queue is the core mangement of the coroutine.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-lock.c | 27 ---------------------------
> qemu-coroutine.c | 27 +++++++++++++++++++++++++++
> qemu-coroutine.h | 7 +++++++
> trace-events | 2 +-
> 4 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 6f47685..159d66d 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -26,35 +26,8 @@
> #include "qemu-coroutine.h"
> #include "qemu-coroutine-int.h"
> #include "qemu-queue.h"
> -#include "main-loop.h"
> #include "trace.h"
>
> -static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
> - QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
> -static QEMUBH* co_runnable_bh;
> -
> -static void qemu_co_process_runnable(void *opaque)
> -{
> - Coroutine *next;
> -
> - trace_qemu_co_process_runnable();
> - while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
> - QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
> - qemu_coroutine_enter(next, NULL);
> - }
> -}
> -
> -static void qemu_co_runnable_schedule(Coroutine *co)
> -{
> - QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
> - qemu_bh_schedule(co_runnable_bh);
> -}
> -
> -static void __attribute__((constructor)) co_runnable_bh_init(void)
> -{
> - co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
> -}
> -
> void qemu_co_queue_init(CoQueue *queue)
> {
> QTAILQ_INIT(&queue->entries);
> diff --git a/qemu-coroutine.c b/qemu-coroutine.c
> index c01252e..c67d557 100644
> --- a/qemu-coroutine.c
> +++ b/qemu-coroutine.c
> @@ -16,6 +16,33 @@
> #include "qemu-common.h"
> #include "qemu-coroutine.h"
> #include "qemu-coroutine-int.h"
> +#include "main-loop.h"
> +
> +static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
> + QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
> +static QEMUBH* co_runnable_bh;
> +
> +static void qemu_co_process_runnable(void *opaque)
> +{
> + Coroutine *next;
> +
> + trace_qemu_co_process_runnable();
> + while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
> + QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
> + qemu_coroutine_enter(next, NULL);
> + }
> +}
> +
> +void qemu_co_runnable_schedule(Coroutine *co)
> +{
> + QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
> + qemu_bh_schedule(co_runnable_bh);
> +}
> +
> +static void __attribute__((constructor)) co_runnable_bh_init(void)
> +{
> + co_runnable_bh = qemu_bh_new(qemu_co_process_runnable, NULL);
> +}
>
> Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
> {
> diff --git a/qemu-coroutine.h b/qemu-coroutine.h
> index 34c15d4..82a7e5c 100644
> --- a/qemu-coroutine.h
> +++ b/qemu-coroutine.h
> @@ -82,6 +82,13 @@ void qemu_coroutine_enter(Coroutine *coroutine, void *opaque);
> void coroutine_fn qemu_coroutine_yield(void);
>
> /**
> + * Add a coroutine to runnable to runnable queue
> + *
> + * The coroutine will be processed later.
> + */
> +void qemu_co_runnable_schedule(Coroutine *co);
> +
> +/**
> * Get the currently executing coroutine
> */
> Coroutine *coroutine_fn qemu_coroutine_self(void);
> diff --git a/trace-events b/trace-events
> index a1737c2..39b9c84 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -556,12 +556,12 @@ qemu_put_ram_ptr(void* addr) "%p"
> xen_platform_log(char *s) "xen platform: %s"
>
> # qemu-coroutine.c
> +qemu_co_process_runnable(void) ""
> qemu_coroutine_enter(void *from, void *to, void *opaque) "from %p to %p opaque %p"
> qemu_coroutine_yield(void *from, void *to) "from %p to %p"
> qemu_coroutine_terminate(void *co) "self %p"
>
> # qemu-coroutine-lock.c
> -qemu_co_process_runnable(void) ""
> qemu_co_queue_next(void *next) "next %p"
> qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
> qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
I think the point here was to have qemu-coroutine.c not depend on the
QEMU main loop, so this is not ok.
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (4 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 06/10] coroutine: move runnale coroutine code to qemu-coroutine.c Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:32 ` Paolo Bonzini
2012-04-03 11:47 ` Kevin Wolf
2012-04-03 8:38 ` [Qemu-devel] [PATCH 08/10] coroutine: process the coroutines woken by child when child yield Lai Jiangshan
` (3 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
queues are not just internal things for locks, split them.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
Makefile.objs | 2 +-
qemu-coroutine-lock.c | 49 +------------------------------
qemu-coroutine-queue.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
trace-events | 4 ++-
4 files changed, 81 insertions(+), 50 deletions(-)
create mode 100644 qemu-coroutine-queue.c
diff --git a/Makefile.objs b/Makefile.objs
index 226b01d..1f0cc31 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -27,7 +27,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
#######################################################################
# coroutines
coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
-coroutine-obj-y += qemu-coroutine-sleep.o
+coroutine-obj-y += qemu-coroutine-sleep.o qemu-coroutine-queue.o
ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
else
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 159d66d..2bfbd41 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -1,5 +1,5 @@
/*
- * coroutine queues and locks
+ * coroutine locks
*
* Copyright (c) 2011 Kevin Wolf <kwolf@redhat.com>
*
@@ -28,53 +28,6 @@
#include "qemu-queue.h"
#include "trace.h"
-void qemu_co_queue_init(CoQueue *queue)
-{
- QTAILQ_INIT(&queue->entries);
-}
-
-void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
-{
- Coroutine *self = qemu_coroutine_self();
- QTAILQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
- qemu_coroutine_yield();
- assert(qemu_in_coroutine());
-}
-
-void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
-{
- Coroutine *self = qemu_coroutine_self();
- QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
- qemu_coroutine_yield();
- assert(qemu_in_coroutine());
-}
-
-bool qemu_co_queue_next(CoQueue *queue)
-{
- Coroutine *next;
-
- next = QTAILQ_FIRST(&queue->entries);
- if (next) {
- QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
- trace_qemu_co_queue_next(next);
- qemu_co_runnable_schedule(next);
- }
-
- return (next != NULL);
-}
-
-void qemu_co_queue_restart_all(CoQueue *queue)
-{
- while (qemu_co_queue_next(queue)) {
- /* Do nothing */
- }
-}
-
-bool qemu_co_queue_empty(CoQueue *queue)
-{
- return (QTAILQ_FIRST(&queue->entries) == NULL);
-}
-
void qemu_co_mutex_init(CoMutex *mutex)
{
memset(mutex, 0, sizeof(*mutex));
diff --git a/qemu-coroutine-queue.c b/qemu-coroutine-queue.c
new file mode 100644
index 0000000..b90818d
--- /dev/null
+++ b/qemu-coroutine-queue.c
@@ -0,0 +1,76 @@
+/*
+ * coroutine queues
+ *
+ * Copyright (c) 2011 Kevin Wolf <kwolf@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu-common.h"
+#include "qemu-coroutine.h"
+#include "qemu-coroutine-int.h"
+#include "qemu-queue.h"
+#include "trace.h"
+
+void qemu_co_queue_init(CoQueue *queue)
+{
+ QTAILQ_INIT(&queue->entries);
+}
+
+void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
+{
+ Coroutine *self = qemu_coroutine_self();
+ QTAILQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
+ qemu_coroutine_yield();
+ assert(qemu_in_coroutine());
+}
+
+void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
+{
+ Coroutine *self = qemu_coroutine_self();
+ QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
+ qemu_coroutine_yield();
+ assert(qemu_in_coroutine());
+}
+
+bool qemu_co_queue_next(CoQueue *queue)
+{
+ Coroutine *next;
+
+ next = QTAILQ_FIRST(&queue->entries);
+ if (next) {
+ QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
+ trace_qemu_co_queue_next(next);
+ qemu_co_runnable_schedule(next);
+ }
+
+ return (next != NULL);
+}
+
+void qemu_co_queue_restart_all(CoQueue *queue)
+{
+ while (qemu_co_queue_next(queue)) {
+ /* Do nothing */
+ }
+}
+
+bool qemu_co_queue_empty(CoQueue *queue)
+{
+ return (QTAILQ_FIRST(&queue->entries) == NULL);
+}
diff --git a/trace-events b/trace-events
index 39b9c84..b35e8de 100644
--- a/trace-events
+++ b/trace-events
@@ -561,8 +561,10 @@ qemu_coroutine_enter(void *from, void *to, void *opaque) "from %p to %p opaque %
qemu_coroutine_yield(void *from, void *to) "from %p to %p"
qemu_coroutine_terminate(void *co) "self %p"
-# qemu-coroutine-lock.c
+# qemu-coroutine-queue.c
qemu_co_queue_next(void *next) "next %p"
+
+# qemu-coroutine-lock.c
qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
qemu_co_mutex_unlock_entry(void *mutex, void *self) "mutex %p self %p"
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c
2012-04-03 8:38 ` [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c Lai Jiangshan
@ 2012-04-03 11:32 ` Paolo Bonzini
2012-04-03 11:47 ` Kevin Wolf
1 sibling, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:32 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> queues are not just internal things for locks, split them.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> Makefile.objs | 2 +-
> qemu-coroutine-lock.c | 49 +------------------------------
> qemu-coroutine-queue.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
> trace-events | 4 ++-
> 4 files changed, 81 insertions(+), 50 deletions(-)
> create mode 100644 qemu-coroutine-queue.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 226b01d..1f0cc31 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -27,7 +27,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> #######################################################################
> # coroutines
> coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> -coroutine-obj-y += qemu-coroutine-sleep.o
> +coroutine-obj-y += qemu-coroutine-sleep.o qemu-coroutine-queue.o
> ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
> coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
> else
> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 159d66d..2bfbd41 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -1,5 +1,5 @@
> /*
> - * coroutine queues and locks
> + * coroutine locks
> *
> * Copyright (c) 2011 Kevin Wolf <kwolf@redhat.com>
> *
> @@ -28,53 +28,6 @@
> #include "qemu-queue.h"
> #include "trace.h"
>
> -void qemu_co_queue_init(CoQueue *queue)
> -{
> - QTAILQ_INIT(&queue->entries);
> -}
> -
> -void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
> -{
> - Coroutine *self = qemu_coroutine_self();
> - QTAILQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
> - qemu_coroutine_yield();
> - assert(qemu_in_coroutine());
> -}
> -
> -void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
> -{
> - Coroutine *self = qemu_coroutine_self();
> - QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
> - qemu_coroutine_yield();
> - assert(qemu_in_coroutine());
> -}
> -
> -bool qemu_co_queue_next(CoQueue *queue)
> -{
> - Coroutine *next;
> -
> - next = QTAILQ_FIRST(&queue->entries);
> - if (next) {
> - QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
> - trace_qemu_co_queue_next(next);
> - qemu_co_runnable_schedule(next);
> - }
> -
> - return (next != NULL);
> -}
> -
> -void qemu_co_queue_restart_all(CoQueue *queue)
> -{
> - while (qemu_co_queue_next(queue)) {
> - /* Do nothing */
> - }
> -}
> -
> -bool qemu_co_queue_empty(CoQueue *queue)
> -{
> - return (QTAILQ_FIRST(&queue->entries) == NULL);
> -}
> -
> void qemu_co_mutex_init(CoMutex *mutex)
> {
> memset(mutex, 0, sizeof(*mutex));
> diff --git a/qemu-coroutine-queue.c b/qemu-coroutine-queue.c
> new file mode 100644
> index 0000000..b90818d
> --- /dev/null
> +++ b/qemu-coroutine-queue.c
> @@ -0,0 +1,76 @@
> +/*
> + * coroutine queues
> + *
> + * Copyright (c) 2011 Kevin Wolf <kwolf@redhat.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu-common.h"
> +#include "qemu-coroutine.h"
> +#include "qemu-coroutine-int.h"
> +#include "qemu-queue.h"
> +#include "trace.h"
> +
> +void qemu_co_queue_init(CoQueue *queue)
> +{
> + QTAILQ_INIT(&queue->entries);
> +}
> +
> +void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
> +{
> + Coroutine *self = qemu_coroutine_self();
> + QTAILQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
> + qemu_coroutine_yield();
> + assert(qemu_in_coroutine());
> +}
> +
> +void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
> +{
> + Coroutine *self = qemu_coroutine_self();
> + QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
> + qemu_coroutine_yield();
> + assert(qemu_in_coroutine());
> +}
> +
> +bool qemu_co_queue_next(CoQueue *queue)
> +{
> + Coroutine *next;
> +
> + next = QTAILQ_FIRST(&queue->entries);
> + if (next) {
> + QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
> + trace_qemu_co_queue_next(next);
> + qemu_co_runnable_schedule(next);
> + }
> +
> + return (next != NULL);
> +}
> +
> +void qemu_co_queue_restart_all(CoQueue *queue)
> +{
> + while (qemu_co_queue_next(queue)) {
> + /* Do nothing */
> + }
> +}
> +
> +bool qemu_co_queue_empty(CoQueue *queue)
> +{
> + return (QTAILQ_FIRST(&queue->entries) == NULL);
> +}
> diff --git a/trace-events b/trace-events
> index 39b9c84..b35e8de 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -561,8 +561,10 @@ qemu_coroutine_enter(void *from, void *to, void *opaque) "from %p to %p opaque %
> qemu_coroutine_yield(void *from, void *to) "from %p to %p"
> qemu_coroutine_terminate(void *co) "self %p"
>
> -# qemu-coroutine-lock.c
> +# qemu-coroutine-queue.c
> qemu_co_queue_next(void *next) "next %p"
> +
> +# qemu-coroutine-lock.c
> qemu_co_mutex_lock_entry(void *mutex, void *self) "mutex %p self %p"
> qemu_co_mutex_lock_return(void *mutex, void *self) "mutex %p self %p"
> qemu_co_mutex_unlock_entry(void *mutex, void *self) "mutex %p self %p"
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
... and you could move the runnable bits to the new file too.
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c
2012-04-03 8:38 ` [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c Lai Jiangshan
2012-04-03 11:32 ` Paolo Bonzini
@ 2012-04-03 11:47 ` Kevin Wolf
1 sibling, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2012-04-03 11:47 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Anthony Liguori, Stefan Hajnoczi, qemu-devel
Am 03.04.2012 10:38, schrieb Lai Jiangshan:
> queues are not just internal things for locks, split them.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> Makefile.objs | 2 +-
> qemu-coroutine-lock.c | 49 +------------------------------
> qemu-coroutine-queue.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
> trace-events | 4 ++-
> 4 files changed, 81 insertions(+), 50 deletions(-)
What's the point? qemu-coroutine-lock.c is already a small file (171
lines) and all functions in it are doing related things (they
synchronise coroutines). Splitting a small cohesive file into two tiny
halves isn't an improvement, IMO.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 08/10] coroutine: process the coroutines woken by child when child yield
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (5 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 07/10] coroutine: split qemu-coroutine-lock.c Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-05 12:43 ` Kevin Wolf
2012-04-03 8:38 ` [Qemu-devel] [PATCH 09/10] coroutine: schedule timeout coroutine instead process it directly Lai Jiangshan
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
If the child wake up someone, process them.
It would the child complete its works if woken coroutine release the locks
that the child needs.
It may help for the cache, if the child wake up some someone, they are
probably accessing the same data.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine.c | 29 +++++++++++++++++++++++------
1 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index c67d557..ce6b3af 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -22,17 +22,25 @@ static QTAILQ_HEAD(, Coroutine) co_runnable_queue =
QTAILQ_HEAD_INITIALIZER(co_runnable_queue);
static QEMUBH* co_runnable_bh;
-static void qemu_co_process_runnable(void *opaque)
+static void coroutine_enter(Coroutine *self, Coroutine *co, void *opaque);
+static void process_runnable(Coroutine *self)
{
Coroutine *next;
- trace_qemu_co_process_runnable();
while ((next = QTAILQ_FIRST(&co_runnable_queue))) {
QTAILQ_REMOVE(&co_runnable_queue, next, co_queue_next);
- qemu_coroutine_enter(next, NULL);
+ coroutine_enter(self, next, NULL);
}
}
+static void qemu_co_process_runnable(void *opaque)
+{
+ Coroutine *self = qemu_coroutine_self();
+
+ trace_qemu_co_process_runnable();
+ process_runnable(self);
+}
+
void qemu_co_runnable_schedule(Coroutine *co)
{
QTAILQ_INSERT_TAIL(&co_runnable_queue, co, co_queue_next);
@@ -69,10 +77,8 @@ static void coroutine_swap(Coroutine *from, Coroutine *to)
}
}
-void qemu_coroutine_enter(Coroutine *co, void *opaque)
+static void coroutine_enter(Coroutine *self, Coroutine *co, void *opaque)
{
- Coroutine *self = qemu_coroutine_self();
-
trace_qemu_coroutine_enter(self, co, opaque);
if (co->caller) {
@@ -85,6 +91,17 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque)
coroutine_swap(self, co);
}
+void qemu_coroutine_enter(Coroutine *co, void *opaque)
+{
+ Coroutine *self = qemu_coroutine_self();
+ typeof(co_runnable_queue) snap = co_runnable_queue;
+
+ QTAILQ_INIT(&co_runnable_queue);
+ coroutine_enter(self, co, opaque);
+ process_runnable(self);
+ co_runnable_queue = snap;
+}
+
void coroutine_fn qemu_coroutine_yield(void)
{
Coroutine *self = qemu_coroutine_self();
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 08/10] coroutine: process the coroutines woken by child when child yield
2012-04-03 8:38 ` [Qemu-devel] [PATCH 08/10] coroutine: process the coroutines woken by child when child yield Lai Jiangshan
@ 2012-04-05 12:43 ` Kevin Wolf
0 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2012-04-05 12:43 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Anthony Liguori, Stefan Hajnoczi, qemu-devel
Am 03.04.2012 10:38, schrieb Lai Jiangshan:
> If the child wake up someone, process them.
>
> It would the child complete its works if woken coroutine release the locks
> that the child needs.
>
> It may help for the cache, if the child wake up some someone, they are
> probably accessing the same data.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Are you very sure that this is correct, particularly with respect to
reentrancy of nested coroutines? This is why this has become a bottom
half in the first place (instead of directly reentering the unlocked
coroutine in qemu_co_mutex_unlock), and it's not completely clear to me
if the problem doesn't exist when you do it after a coroutine has exited
instead of in an BH.
I'd be very careful with such changes.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 09/10] coroutine: schedule timeout coroutine instead process it directly
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (6 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 08/10] coroutine: process the coroutines woken by child when child yield Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:26 ` Paolo Bonzini
2012-04-03 8:38 ` [Qemu-devel] [PATCH 10/10] coroutine: add qemu_coroutine_run() wrapper Lai Jiangshan
2012-04-03 11:30 ` [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Paolo Bonzini
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
Avoid a timer callback spends too much time.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
qemu-coroutine-sleep.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-coroutine-sleep.c b/qemu-coroutine-sleep.c
index fd65274..df9254a 100644
--- a/qemu-coroutine-sleep.c
+++ b/qemu-coroutine-sleep.c
@@ -24,7 +24,7 @@ static void co_sleep_cb(void *opaque)
CoSleepCB *sleep_cb = opaque;
qemu_free_timer(sleep_cb->ts);
- qemu_coroutine_enter(sleep_cb->co, NULL);
+ qemu_co_runnable_schedule(sleep_cb->co);
}
void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 09/10] coroutine: schedule timeout coroutine instead process it directly
2012-04-03 8:38 ` [Qemu-devel] [PATCH 09/10] coroutine: schedule timeout coroutine instead process it directly Lai Jiangshan
@ 2012-04-03 11:26 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:26 UTC (permalink / raw)
To: qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> Avoid a timer callback spends too much time.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine-sleep.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-coroutine-sleep.c b/qemu-coroutine-sleep.c
> index fd65274..df9254a 100644
> --- a/qemu-coroutine-sleep.c
> +++ b/qemu-coroutine-sleep.c
> @@ -24,7 +24,7 @@ static void co_sleep_cb(void *opaque)
> CoSleepCB *sleep_cb = opaque;
>
> qemu_free_timer(sleep_cb->ts);
> - qemu_coroutine_enter(sleep_cb->co, NULL);
> + qemu_co_runnable_schedule(sleep_cb->co);
> }
>
> void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns)
Why is this important?
Also, why should it be different for timers, fd_handlers (used for
example by NBD) and bottom halves (used for AIO callbacks)?
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 10/10] coroutine: add qemu_coroutine_run() wrapper
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (7 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 09/10] coroutine: schedule timeout coroutine instead process it directly Lai Jiangshan
@ 2012-04-03 8:38 ` Lai Jiangshan
2012-04-03 11:33 ` Paolo Bonzini
2012-04-03 11:30 ` [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Paolo Bonzini
9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2012-04-03 8:38 UTC (permalink / raw)
To: Kevin Wolf, Stefan Hajnoczi, Anthony Liguori; +Cc: qemu-devel, Lai Jiangshan
Wrapper for qemu_coroutine_create()+qemu_coroutine_enter()
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
block.c | 28 +++++++---------------------
hw/9pfs/virtio-9p.c | 4 +---
nbd.c | 2 +-
qemu-coroutine.h | 12 ++++++++++++
qemu-io.c | 4 +---
5 files changed, 22 insertions(+), 28 deletions(-)
diff --git a/block.c b/block.c
index b88ee90..adf2010 100644
--- a/block.c
+++ b/block.c
@@ -1451,7 +1451,6 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
.iov_base = (void *)buf,
.iov_len = nb_sectors * BDRV_SECTOR_SIZE,
};
- Coroutine *co;
RwCo rwco = {
.bs = bs,
.sector_num = sector_num,
@@ -1467,8 +1466,7 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
/* Fast-path if already in coroutine context */
bdrv_rw_co_entry(&rwco);
} else {
- co = qemu_coroutine_create(bdrv_rw_co_entry);
- qemu_coroutine_enter(co, &rwco);
+ qemu_coroutine_run(bdrv_rw_co_entry, &rwco);
while (rwco.ret == NOT_DONE) {
qemu_aio_wait();
}
@@ -2414,7 +2412,6 @@ static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum)
{
- Coroutine *co;
BdrvCoIsAllocatedData data = {
.bs = bs,
.sector_num = sector_num,
@@ -2423,8 +2420,7 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
.done = false,
};
- co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
- qemu_coroutine_enter(co, &data);
+ qemu_coroutine_run(bdrv_is_allocated_co_entry, &data);
while (!data.done) {
qemu_aio_wait();
}
@@ -3348,7 +3344,6 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
void *opaque,
bool is_write)
{
- Coroutine *co;
BlockDriverAIOCBCoroutine *acb;
acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
@@ -3357,8 +3352,7 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
acb->req.qiov = qiov;
acb->is_write = is_write;
- co = qemu_coroutine_create(bdrv_co_do_rw);
- qemu_coroutine_enter(co, acb);
+ qemu_coroutine_run(bdrv_co_do_rw, acb);
return &acb->common;
}
@@ -3378,12 +3372,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
{
trace_bdrv_aio_flush(bs, opaque);
- Coroutine *co;
BlockDriverAIOCBCoroutine *acb;
acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
- co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
- qemu_coroutine_enter(co, acb);
+ qemu_coroutine_run(bdrv_aio_flush_co_entry, acb);
return &acb->common;
}
@@ -3402,7 +3394,6 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
int64_t sector_num, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque)
{
- Coroutine *co;
BlockDriverAIOCBCoroutine *acb;
trace_bdrv_aio_discard(bs, sector_num, nb_sectors, opaque);
@@ -3410,8 +3401,7 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
acb->req.sector = sector_num;
acb->req.nb_sectors = nb_sectors;
- co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
- qemu_coroutine_enter(co, acb);
+ qemu_coroutine_run(bdrv_aio_discard_co_entry, acb);
return &acb->common;
}
@@ -3586,7 +3576,6 @@ void bdrv_invalidate_cache_all(void)
int bdrv_flush(BlockDriverState *bs)
{
- Coroutine *co;
RwCo rwco = {
.bs = bs,
.ret = NOT_DONE,
@@ -3596,8 +3585,7 @@ int bdrv_flush(BlockDriverState *bs)
/* Fast-path if already in coroutine context */
bdrv_flush_co_entry(&rwco);
} else {
- co = qemu_coroutine_create(bdrv_flush_co_entry);
- qemu_coroutine_enter(co, &rwco);
+ qemu_coroutine_run(bdrv_flush_co_entry, &rwco);
while (rwco.ret == NOT_DONE) {
qemu_aio_wait();
}
@@ -3645,7 +3633,6 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
{
- Coroutine *co;
RwCo rwco = {
.bs = bs,
.sector_num = sector_num,
@@ -3657,8 +3644,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
/* Fast-path if already in coroutine context */
bdrv_discard_co_entry(&rwco);
} else {
- co = qemu_coroutine_create(bdrv_discard_co_entry);
- qemu_coroutine_enter(co, &rwco);
+ qemu_coroutine_run(bdrv_discard_co_entry, &rwco);
while (rwco.ret == NOT_DONE) {
qemu_aio_wait();
}
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index c633fb9..e2f384e 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3230,7 +3230,6 @@ static inline bool is_read_only_op(V9fsPDU *pdu)
static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
{
- Coroutine *co;
CoroutineEntry *handler;
if (pdu->id >= ARRAY_SIZE(pdu_co_handlers) ||
@@ -3243,8 +3242,7 @@ static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
if (is_ro_export(&s->ctx) && !is_read_only_op(pdu)) {
handler = v9fs_fs_ro;
}
- co = qemu_coroutine_create(handler);
- qemu_coroutine_enter(co, pdu);
+ qemu_coroutine_run(handler, pdu);
}
void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/nbd.c b/nbd.c
index 567e94e..14ede03 100644
--- a/nbd.c
+++ b/nbd.c
@@ -926,7 +926,7 @@ static void nbd_read(void *opaque)
if (client->recv_coroutine) {
qemu_coroutine_enter(client->recv_coroutine, NULL);
} else {
- qemu_coroutine_enter(qemu_coroutine_create(nbd_trip), client);
+ qemu_coroutine_run(nbd_trip, client);
}
}
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 82a7e5c..572013a 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -74,6 +74,18 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry);
void qemu_coroutine_enter(Coroutine *coroutine, void *opaque);
/**
+ * Create a new coroutine and transfer control to it
+ *
+ * Wrapper for qemu_coroutine_create()+qemu_coroutine_enter()
+ */
+static inline void qemu_coroutine_run(CoroutineEntry *entry, void *opaque)
+{
+ Coroutine *co = qemu_coroutine_create(entry);
+
+ qemu_coroutine_enter(co, opaque);
+}
+
+/**
* Transfer control back to a coroutine's caller
*
* This function does not return until the coroutine is re-entered using
diff --git a/qemu-io.c b/qemu-io.c
index 3189530..75e9ac5 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -243,7 +243,6 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
static int do_co_write_zeroes(int64_t offset, int count, int *total)
{
- Coroutine *co;
CoWriteZeroes data = {
.offset = offset,
.count = count,
@@ -251,8 +250,7 @@ static int do_co_write_zeroes(int64_t offset, int count, int *total)
.done = false,
};
- co = qemu_coroutine_create(co_write_zeroes_entry);
- qemu_coroutine_enter(co, &data);
+ qemu_coroutine_run(co_write_zeroes_entry, &data);
while (!data.done) {
qemu_aio_wait();
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 10/10] coroutine: add qemu_coroutine_run() wrapper
2012-04-03 8:38 ` [Qemu-devel] [PATCH 10/10] coroutine: add qemu_coroutine_run() wrapper Lai Jiangshan
@ 2012-04-03 11:33 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:33 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> Wrapper for qemu_coroutine_create()+qemu_coroutine_enter()
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> block.c | 28 +++++++---------------------
> hw/9pfs/virtio-9p.c | 4 +---
> nbd.c | 2 +-
> qemu-coroutine.h | 12 ++++++++++++
> qemu-io.c | 4 +---
> 5 files changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/block.c b/block.c
> index b88ee90..adf2010 100644
> --- a/block.c
> +++ b/block.c
> @@ -1451,7 +1451,6 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
> .iov_base = (void *)buf,
> .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
> };
> - Coroutine *co;
> RwCo rwco = {
> .bs = bs,
> .sector_num = sector_num,
> @@ -1467,8 +1466,7 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
> /* Fast-path if already in coroutine context */
> bdrv_rw_co_entry(&rwco);
> } else {
> - co = qemu_coroutine_create(bdrv_rw_co_entry);
> - qemu_coroutine_enter(co, &rwco);
> + qemu_coroutine_run(bdrv_rw_co_entry, &rwco);
> while (rwco.ret == NOT_DONE) {
> qemu_aio_wait();
> }
> @@ -2414,7 +2412,6 @@ static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
> int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
> int *pnum)
> {
> - Coroutine *co;
> BdrvCoIsAllocatedData data = {
> .bs = bs,
> .sector_num = sector_num,
> @@ -2423,8 +2420,7 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
> .done = false,
> };
>
> - co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
> - qemu_coroutine_enter(co, &data);
> + qemu_coroutine_run(bdrv_is_allocated_co_entry, &data);
> while (!data.done) {
> qemu_aio_wait();
> }
> @@ -3348,7 +3344,6 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
> void *opaque,
> bool is_write)
> {
> - Coroutine *co;
> BlockDriverAIOCBCoroutine *acb;
>
> acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
> @@ -3357,8 +3352,7 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
> acb->req.qiov = qiov;
> acb->is_write = is_write;
>
> - co = qemu_coroutine_create(bdrv_co_do_rw);
> - qemu_coroutine_enter(co, acb);
> + qemu_coroutine_run(bdrv_co_do_rw, acb);
>
> return &acb->common;
> }
> @@ -3378,12 +3372,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
> {
> trace_bdrv_aio_flush(bs, opaque);
>
> - Coroutine *co;
> BlockDriverAIOCBCoroutine *acb;
>
> acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
> - co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
> - qemu_coroutine_enter(co, acb);
> + qemu_coroutine_run(bdrv_aio_flush_co_entry, acb);
>
> return &acb->common;
> }
> @@ -3402,7 +3394,6 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
> int64_t sector_num, int nb_sectors,
> BlockDriverCompletionFunc *cb, void *opaque)
> {
> - Coroutine *co;
> BlockDriverAIOCBCoroutine *acb;
>
> trace_bdrv_aio_discard(bs, sector_num, nb_sectors, opaque);
> @@ -3410,8 +3401,7 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
> acb = qemu_aio_get(&bdrv_em_co_aio_pool, bs, cb, opaque);
> acb->req.sector = sector_num;
> acb->req.nb_sectors = nb_sectors;
> - co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
> - qemu_coroutine_enter(co, acb);
> + qemu_coroutine_run(bdrv_aio_discard_co_entry, acb);
>
> return &acb->common;
> }
> @@ -3586,7 +3576,6 @@ void bdrv_invalidate_cache_all(void)
>
> int bdrv_flush(BlockDriverState *bs)
> {
> - Coroutine *co;
> RwCo rwco = {
> .bs = bs,
> .ret = NOT_DONE,
> @@ -3596,8 +3585,7 @@ int bdrv_flush(BlockDriverState *bs)
> /* Fast-path if already in coroutine context */
> bdrv_flush_co_entry(&rwco);
> } else {
> - co = qemu_coroutine_create(bdrv_flush_co_entry);
> - qemu_coroutine_enter(co, &rwco);
> + qemu_coroutine_run(bdrv_flush_co_entry, &rwco);
> while (rwco.ret == NOT_DONE) {
> qemu_aio_wait();
> }
> @@ -3645,7 +3633,6 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
>
> int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
> {
> - Coroutine *co;
> RwCo rwco = {
> .bs = bs,
> .sector_num = sector_num,
> @@ -3657,8 +3644,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
> /* Fast-path if already in coroutine context */
> bdrv_discard_co_entry(&rwco);
> } else {
> - co = qemu_coroutine_create(bdrv_discard_co_entry);
> - qemu_coroutine_enter(co, &rwco);
> + qemu_coroutine_run(bdrv_discard_co_entry, &rwco);
> while (rwco.ret == NOT_DONE) {
> qemu_aio_wait();
> }
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index c633fb9..e2f384e 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -3230,7 +3230,6 @@ static inline bool is_read_only_op(V9fsPDU *pdu)
>
> static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
> {
> - Coroutine *co;
> CoroutineEntry *handler;
>
> if (pdu->id >= ARRAY_SIZE(pdu_co_handlers) ||
> @@ -3243,8 +3242,7 @@ static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
> if (is_ro_export(&s->ctx) && !is_read_only_op(pdu)) {
> handler = v9fs_fs_ro;
> }
> - co = qemu_coroutine_create(handler);
> - qemu_coroutine_enter(co, pdu);
> + qemu_coroutine_run(handler, pdu);
> }
>
> void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
> diff --git a/nbd.c b/nbd.c
> index 567e94e..14ede03 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -926,7 +926,7 @@ static void nbd_read(void *opaque)
> if (client->recv_coroutine) {
> qemu_coroutine_enter(client->recv_coroutine, NULL);
> } else {
> - qemu_coroutine_enter(qemu_coroutine_create(nbd_trip), client);
> + qemu_coroutine_run(nbd_trip, client);
> }
> }
>
> diff --git a/qemu-coroutine.h b/qemu-coroutine.h
> index 82a7e5c..572013a 100644
> --- a/qemu-coroutine.h
> +++ b/qemu-coroutine.h
> @@ -74,6 +74,18 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry);
> void qemu_coroutine_enter(Coroutine *coroutine, void *opaque);
>
> /**
> + * Create a new coroutine and transfer control to it
> + *
> + * Wrapper for qemu_coroutine_create()+qemu_coroutine_enter()
> + */
> +static inline void qemu_coroutine_run(CoroutineEntry *entry, void *opaque)
> +{
> + Coroutine *co = qemu_coroutine_create(entry);
> +
> + qemu_coroutine_enter(co, opaque);
> +}
> +
> +/**
> * Transfer control back to a coroutine's caller
> *
> * This function does not return until the coroutine is re-entered using
> diff --git a/qemu-io.c b/qemu-io.c
> index 3189530..75e9ac5 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -243,7 +243,6 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
>
> static int do_co_write_zeroes(int64_t offset, int count, int *total)
> {
> - Coroutine *co;
> CoWriteZeroes data = {
> .offset = offset,
> .count = count,
> @@ -251,8 +250,7 @@ static int do_co_write_zeroes(int64_t offset, int count, int *total)
> .done = false,
> };
>
> - co = qemu_coroutine_create(co_write_zeroes_entry);
> - qemu_coroutine_enter(co, &data);
> + qemu_coroutine_run(co_write_zeroes_entry, &data);
> while (!data.done) {
> qemu_aio_wait();
> }
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly
2012-04-03 8:38 [Qemu-devel] [PATCH 01/10] coroutine: use qemu_coroutine_switch() directly Lai Jiangshan
` (8 preceding siblings ...)
2012-04-03 8:38 ` [Qemu-devel] [PATCH 10/10] coroutine: add qemu_coroutine_run() wrapper Lai Jiangshan
@ 2012-04-03 11:30 ` Paolo Bonzini
9 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2012-04-03 11:30 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel
Il 03/04/2012 10:38, Lai Jiangshan ha scritto:
> When qemu_coroutine_switch() in the qemu_coroutine_yield() returns,
> It must be someone calls qemu_coroutine_enter() for it, so the @to
> is active, the tests in coroutine_swap() are unneeded, so we use
> qemu_coroutine_switch() directly in qemu_coroutine_yield().
The patches are very different. Some of them are just cleanups which I
like (patches 2 to 7, more or less). The others seem to be a (micro?)
optimization whose usecase and result you didn't document because there
was no cover letter. This patch is an example, is one or two "if"s so
expensive that it matters?
Paolo
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> qemu-coroutine.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-coroutine.c b/qemu-coroutine.c
> index 600be26..c01252e 100644
> --- a/qemu-coroutine.c
> +++ b/qemu-coroutine.c
> @@ -71,5 +71,5 @@ void coroutine_fn qemu_coroutine_yield(void)
> }
>
> self->caller = NULL;
> - coroutine_swap(self, to);
> + qemu_coroutine_switch(self, to, COROUTINE_YIELD);
> }
^ permalink raw reply [flat|nested] 22+ messages in thread