Message ID | 1333442297-18932-7-git-send-email-laijs@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
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
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
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"
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