Re: [Qemu-devel] [RFC 3/7] iothread: add I/O thread object

[Michael Roth]

Quoting Stefan Hajnoczi (2013-12-12 07:19:40)
> This is a stand-in for Michael Roth's QContext.  I expect this to be
> replaced once QContext is completed.
> 
> The IOThread object is an AioContext event loop thread.  This patch adds
> the concept of multiple event loop threads, allowing users to define
> them.
> 
> When SMP guests run on SMP hosts it makes sense to instantiate multiple
> IOThreads.  This spreads event loop processing across multiple cores.
> Note that additional patches are required to actually bind a device to
> an IOThread.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  Makefile.objs             |   1 +
>  include/sysemu/iothread.h |  31 +++++++++++++
>  iothread.c                | 115 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 147 insertions(+)
>  create mode 100644 include/sysemu/iothread.h
>  create mode 100644 iothread.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 2b6c1fe..a1102a5 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -42,6 +42,7 @@ libcacard-y += libcacard/vcardt.o
> 
>  ifeq ($(CONFIG_SOFTMMU),y)
>  common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/
> +common-obj-y += iothread.o
>  common-obj-y += net/
>  common-obj-y += readline.o
>  common-obj-y += qdev-monitor.o device-hotplug.o
> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> new file mode 100644
> index 0000000..8c49bd6
> --- /dev/null
> +++ b/include/sysemu/iothread.h
> @@ -0,0 +1,31 @@
> +/*
> + * Event loop thread
> + *
> + * Copyright Red Hat Inc., 2013
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef IOTHREAD_H
> +#define IOTHREAD_H
> +
> +#include "block/aio.h"
> +
> +#define TYPE_IOTHREAD "iothread"
> +#define IOTHREADS_PATH "/backends/iothreads"
> +
> +typedef struct IOThread IOThread;
> +
> +#define IOTHREAD(obj) \
> +   OBJECT_CHECK(IOThread, obj, TYPE_IOTHREAD)
> +
> +IOThread *iothread_find(const char *id);
> +char *iothread_get_id(IOThread *iothread);
> +AioContext *iothread_get_aio_context(IOThread *iothread);
> +
> +#endif /* IOTHREAD_H */
> diff --git a/iothread.c b/iothread.c
> new file mode 100644
> index 0000000..dbc6047
> --- /dev/null
> +++ b/iothread.c
> @@ -0,0 +1,115 @@
> +/*
> + * Event loop thread
> + *
> + * Copyright Red Hat Inc., 2013
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qom/object.h"
> +#include "qemu/module.h"
> +#include "qemu/thread.h"
> +#include "block/aio.h"
> +#include "sysemu/iothread.h"
> +
> +typedef ObjectClass IOThreadClass;
> +struct IOThread {
> +    Object parent;
> +    QemuThread thread;
> +    AioContext *ctx;
> +    bool stopping;
> +};
> +
> +#define IOTHREAD_GET_CLASS(obj) \
> +   OBJECT_GET_CLASS(IOThreadClass, obj, TYPE_IOTHREAD)
> +#define IOTHREAD_CLASS(klass) \
> +   OBJECT_CLASS_CHECK(IOThreadClass, klass, TYPE_IOTHREAD)
> +
> +static void *iothread_run(void *opaque)
> +{
> +    IOThread *iothread = opaque;
> +
> +    for (;;) {
> +        /* TODO can we optimize away acquire/release to only happen when
> +         * aio_notify() was called?
> +         */

Perhaps have the AioContext's notifier callback set a flag that can be
checked for afterward to determine whether we should release/re-acquire?
Calls to aio_context_acquire() could reset it upon acquistion, so we could
maybe do something like:

while(!iothread->stopping) {
    aio_context_acquire(iothread->ctx);
    while (!iothread->ctx->notified) {
        aio_poll(iothread->ctx, true);
    }
    aio_context_release(iothread->ctx);
}

> +        aio_context_acquire(iothread->ctx);
> +        if (iothread->stopping) {
> +            aio_context_release(iothread->ctx);
> +            break;
> +        }
> +        aio_poll(iothread->ctx, true);
> +        aio_context_release(iothread->ctx);
> +    }
> +    return NULL;
> +}
> +
> +static void iothread_instance_init(Object *obj)
> +{
> +    IOThread *iothread = IOTHREAD(obj);
> +
> +    iothread->stopping = false;
> +    iothread->ctx = aio_context_new();
> +
> +    /* This assumes .instance_init() is called from a thread with useful CPU
> +     * affinity for us to inherit.
> +     */

Is this assumption necessary/controllable? Couldn't we just expose the thread
id via QOM or some other interface so users/management can set the affinity
later?

> +    qemu_thread_create(&iothread->thread, iothread_run,
> +                       iothread, QEMU_THREAD_JOINABLE);
> +}
> +
> +static void iothread_instance_finalize(Object *obj)
> +{
> +    IOThread *iothread = IOTHREAD(obj);
> +
> +    iothread->stopping = true;
> +    aio_notify(iothread->ctx);
> +    qemu_thread_join(&iothread->thread);
> +    aio_context_unref(iothread->ctx);
> +}
> +
> +static const TypeInfo iothread_info = {
> +    .name = TYPE_IOTHREAD,
> +    .parent = TYPE_OBJECT,
> +    .instance_size = sizeof(IOThread),
> +    .instance_init = iothread_instance_init,
> +    .instance_finalize = iothread_instance_finalize,
> +};
> +
> +static void iothread_register_types(void)
> +{
> +    type_register_static(&iothread_info);
> +}
> +
> +type_init(iothread_register_types)
> +
> +IOThread *iothread_find(const char *id)
> +{
> +    Object *container = container_get(object_get_root(), IOTHREADS_PATH);
> +    Object *child;
> +
> +    child = object_property_get_link(container, id, NULL);
> +    if (!child) {
> +        return NULL;
> +    }
> +    return IOTHREAD(child);
> +}
> +
> +char *iothread_get_id(IOThread *iothread)
> +{
> +    /* The last path component is the identifier */
> +    char *path = object_get_canonical_path(OBJECT(iothread));
> +    char *id = g_strdup(&path[sizeof(IOTHREADS_PATH)]);
> +    g_free(path);
> +    return id;
> +}
> +
> +AioContext *iothread_get_aio_context(IOThread *iothread)
> +{
> +    return iothread->ctx;
> +}
> -- 
> 1.8.4.2