Patchwork qemu-coroutine: Add simple work queue support

login
register
mail settings
Submitter Peter A. G. Crosthwaite
Date Aug. 24, 2011, 7:57 a.m.
Message ID <1314172671-13123-1-git-send-email-peter.crosthwaite@petalogix.com>
Download mbox | patch
Permalink /patch/111269/
State New
Headers show

Comments

Peter A. G. Crosthwaite - Aug. 24, 2011, 7:57 a.m.
Add a function co_queue_yield_to_next() which will immediately transfer
control to the coroutine at the head of a co queue. This can be used for
implementing simple work queues where the manager of a co-queue only
needs to restart queued routines one at a time.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 qemu-coroutine-lock.c |   13 +++++++++++++
 qemu-coroutine.h      |    9 +++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)
Stefan Hajnoczi - Aug. 24, 2011, 12:08 p.m.
On Wed, Aug 24, 2011 at 05:57:51PM +1000, Peter A. G. Crosthwaite wrote:
> Add a function co_queue_yield_to_next() which will immediately transfer
> control to the coroutine at the head of a co queue. This can be used for
> implementing simple work queues where the manager of a co-queue only
> needs to restart queued routines one at a time.
> 
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  qemu-coroutine-lock.c |   13 +++++++++++++
>  qemu-coroutine.h      |    9 +++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)

Please share code that uses this function, even if it's not suitable for
merging.  I'd like to understand what the pattern for using this
function is.

> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index a80f437..de2fc21 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -75,6 +75,19 @@ bool qemu_co_queue_next(CoQueue *queue)
>      return (next != NULL);
>  }
> 
> +bool qemu_co_queue_yield_to_next(CoQueue *queue)

This function can only be executed in coroutine context (i.e. it cannot
be executed outside a coroutine).  Therefore please use the coroutine_fn
annotation.  Perhaps we'll hook the annotation up with a checker in the
future to ensure that non-coroutine_fn functions never call coroutine_fn
functions statically at build time.  In the meantime it serves as
documentation.

Stefan
Peter A. G. Crosthwaite - Aug. 25, 2011, 6:46 a.m.
Hi Stefan,

I have ccd you on a RFC containing the work that this patch uses. I have
also changed the name of the function to co_queue_enter_next() as i think
yield was not the appropriate name. The idea is the top level thread which
is managing the work queue can transfer into coroutine context with the
function, so this function is more of an "enter" function than a "yield"
function. I cant see any technical reasons why it needs the coroutine_fn
restriction, as this in not required by qemu_coroutine_enter, which is the
backend of this function.

Regards and thanks for your comments,
Peter

On Wed, Aug 24, 2011 at 10:08 PM, Stefan Hajnoczi <
stefanha@linux.vnet.ibm.com> wrote:

> On Wed, Aug 24, 2011 at 05:57:51PM +1000, Peter A. G. Crosthwaite wrote:
> > Add a function co_queue_yield_to_next() which will immediately transfer
> > control to the coroutine at the head of a co queue. This can be used for
> > implementing simple work queues where the manager of a co-queue only
> > needs to restart queued routines one at a time.
> >
> > Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> > ---
> >  qemu-coroutine-lock.c |   13 +++++++++++++
> >  qemu-coroutine.h      |    9 +++++++++
> >  2 files changed, 22 insertions(+), 0 deletions(-)
>
> Please share code that uses this function, even if it's not suitable for
> merging.  I'd like to understand what the pattern for using this
> function is.
>
> > diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> > index a80f437..de2fc21 100644
> > --- a/qemu-coroutine-lock.c
> > +++ b/qemu-coroutine-lock.c
> > @@ -75,6 +75,19 @@ bool qemu_co_queue_next(CoQueue *queue)
> >      return (next != NULL);
> >  }
> >
> > +bool qemu_co_queue_yield_to_next(CoQueue *queue)
>
> This function can only be executed in coroutine context (i.e. it cannot
> be executed outside a coroutine).  Therefore please use the coroutine_fn
> annotation.  Perhaps we'll hook the annotation up with a checker in the
> future to ensure that non-coroutine_fn functions never call coroutine_fn
> functions statically at build time.  In the meantime it serves as
> documentation.
>
> Stefan
>
Stefan Hajnoczi - Aug. 25, 2011, 8:06 a.m.
On Thu, Aug 25, 2011 at 04:46:59PM +1000, Peter Crosthwaite wrote:
> Hi Stefan,
> 
> I have ccd you on a RFC containing the work that this patch uses. I have
> also changed the name of the function to co_queue_enter_next() as i think
> yield was not the appropriate name. The idea is the top level thread which
> is managing the work queue can transfer into coroutine context with the
> function, so this function is more of an "enter" function than a "yield"
> function. I cant see any technical reasons why it needs the coroutine_fn
> restriction, as this in not required by qemu_coroutine_enter, which is the
> backend of this function.

You are right, I was confused by the "yield" in the name :).  It doesn't
need to be coroutine_fn.

Stefan
Kevin Wolf - Sept. 5, 2011, 1:23 p.m.
Am 24.08.2011 09:57, schrieb Peter A. G. Crosthwaite:
> Add a function co_queue_yield_to_next() which will immediately transfer
> control to the coroutine at the head of a co queue. This can be used for
> implementing simple work queues where the manager of a co-queue only
> needs to restart queued routines one at a time.
> 
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

Is there a difference to qemu_co_queue_next(), except that it doesn't
use a bottom half? Is there a reason why using a BH isn't reasonable in
your use case?

Kevin

Patch

diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index a80f437..de2fc21 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -75,6 +75,19 @@  bool qemu_co_queue_next(CoQueue *queue)
     return (next != NULL);
 }
 
+bool qemu_co_queue_yield_to_next(CoQueue *queue)
+{
+    Coroutine *next;
+
+    next = QTAILQ_FIRST(&queue->entries);
+    if (next) {
+        QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
+        qemu_coroutine_enter(next, NULL);
+    }
+
+    return (next != NULL);
+}
+
 bool qemu_co_queue_empty(CoQueue *queue)
 {
     return (QTAILQ_FIRST(&queue->entries) == NULL);
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 2f2fd95..dbc3610 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -125,6 +125,15 @@  void coroutine_fn qemu_co_queue_wait(CoQueue *queue);
 bool qemu_co_queue_next(CoQueue *queue);
 
 /**
+ * Transfers control to the next coroutine in the CoQueue and removes it from
+ * the queue.
+ *
+ * Returns true once after control transfers back to caller, or false
+ * immediately if the queue is empty.
+ */
+bool qemu_co_queue_yield_to_next(CoQueue *queue)
+
+/**
  * Checks if the CoQueue is empty.
  */
 bool qemu_co_queue_empty(CoQueue *queue);