Re: [Qemu-devel] [PATCH RFC] iothread: Add "spawns" property

[Stefan Hajnoczi]

On Wed, Oct 19, 2016 at 04:04:50PM +0800, Fam Zheng wrote:
> The option specifies how many threads to spawn under the iothread
> object. All threads share the same AioContext so they can safely run
> (contend) together.
> 
> With AioContext going away, the spawns will natually enable the block
> multi-queue work.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> 
> ---
> 
> Based on v2 of Paolo's RFifoLock removal series, with which the
> symmetric contention on the single AioContext is no longer a busy
> preempt loop.
> ---
>  include/sysemu/iothread.h |  19 ++++--
>  iothread.c                | 148 
> ++++++++++++++++++++++++++++++++++------------
>  2 files changed, 125 insertions(+), 42 deletions(-)

I'm not happy with "IOThread" becoming a group of threads.  IOThread
should stay the way it is.  Instead you should add a new object type
that simply groups IOThreads for convenient assignment to devices, e.g.
IOThreadGroup.  Then multiqueue devices can use an IOThreadGroup to work
inside multiple IOThreads.

> @@ -161,22 +225,32 @@ static int query_one_iothread(Object *object, void 
> *opaque)
>      IOThreadInfoList *elem;
>      IOThreadInfo *info;
>      IOThread *iothread;
> +    char *id;
> +    int i;
>  
>      iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
>      if (!iothread) {
>          return 0;
>      }
>  
> -    info = g_new0(IOThreadInfo, 1);
> -    info->id = iothread_get_id(iothread);
> -    info->thread_id = iothread->thread_id;
> +    id = iothread_get_id(iothread);
> +    for (i = 0; i < iothread->nspawns; ++i) {
> +        info = g_new0(IOThreadInfo, 1);
> +        info->thread_id = iothread->spawns[i].thread_id;
> +        if (iothread->nspawns > 1) {
> +            info->id = g_strdup_printf("%s[%d]", id, i);
> +        } else {
> +            info->id = g_strdup(id);
> +        }
>  
> -    elem = g_new0(IOThreadInfoList, 1);
> -    elem->value = info;
> -    elem->next = NULL;
> +        elem = g_new0(IOThreadInfoList, 1);
> +        elem->value = info;
> +        elem->next = NULL;
>  
> -    **prev = elem;
> -    *prev = &elem->next;
> +        **prev = elem;
> +        *prev = &elem->next;
> +    }
> +    g_free(id);

^-- yikes, now we're pretending to the QMP client that there are
multiple IOThread objects but internally we only have one.  Let's not go
down that road.

signature.asc
Description: PGP signature