[Spice-devel] [PATCHv4] server: add async io support

Yonit Halperin yhalperi at redhat.com
Wed Jul 13 01:15:50 PDT 2011


On 07/12/2011 05:03 PM, Alon Levy wrote:
Hi,

> The new _ASYNC io's in qxl_dev listed at the end get six new api
> functions, and an additional callback function "async_complete". When
> the async version of a specific io is used, completion is notified by
> calling async_complete, and no READY message is written or expected by
> the dispatcher.
>
> update_area has been changed to push QXLRects to the worker thread, where
> the conversion to SpiceRect takes place.
>
> A cookie has been added to each async call to QXLWorker, and is passed back via
> async_complete.
>
> Added api:
>
> QXLWorker:
>      update_area_async
>      add_memslot_async
>      destroy_surfaces_async
>      destroy_primary_surface_async
>      create_primary_surface_async
>      destroy_surface_wait_async
>
> QXLInterface:
>      async_complete
> ---
>   server/red_dispatcher.c |  156 ++++++++++++++++++++++++++++++++++++++---------
>   server/red_parse_qxl.c  |    2 +-
>   server/red_parse_qxl.h  |    2 +-
>   server/red_worker.c     |  155 ++++++++++++++++++++++++++++++++--------------
>   server/red_worker.h     |    7 ++
>   server/spice.h          |   11 +++
>   6 files changed, 254 insertions(+), 79 deletions(-)



> +    /* in async case this is set before primary is destroyed in worker */
>       dispatcher->x_res = 0;
>       dispatcher->y_res = 0;
>       dispatcher->use_hardware_cursor = FALSE;
> @@ -290,11 +323,27 @@ static void qxl_worker_destroy_primary(QXLWorker *qxl_worker, uint32_t surface_i
>       update_client_mouse_allowed();
I think that async_complete callback should go through the dispatcher. 
This way (1) you can make this updates only after the worker handles the 
request and (2) async_complete will be executed in the vcpu thread context

>   }
>
> -static void qxl_worker_create_primary(QXLWorker *qxl_worker, uint32_t surface_id,
> -                                      QXLDevSurfaceCreate *surface)
> +static void qxl_worker_destroy_primary_surface(QXLWorker *qxl_worker, uint32_t surface_id)
> +{
> +    qxl_worker_destroy_primary_surface_helper(qxl_worker, surface_id, 0, 0);
> +}
> +
> +static void qxl_worker_destroy_primary_surface_async(QXLWorker *qxl_worker, uint32_t surface_id, uint64_t cookie)
> +{
> +    qxl_worker_destroy_primary_surface_helper(qxl_worker, surface_id, 1, cookie);
> +}
> +
> +static void qxl_worker_create_primary_surface_helper(QXLWorker *qxl_worker, uint32_t surface_id,
> +                                      QXLDevSurfaceCreate *surface, int async, uint64_t cookie)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> -    RedWorkerMessage message = RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE;
> +    RedWorkerMessage message;
> +
> +    if (async) {
> +        message = RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC;
> +    } else {
> +        message = RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE;
> +    }
>
>       dispatcher->x_res = surface->width;
>       dispatcher->y_res = surface->height;
> @@ -302,14 +351,31 @@ static void qxl_worker_create_primary(QXLWorker *qxl_worker, uint32_t surface_id
>       dispatcher->primary_active = TRUE;
>
same as the previous comment
>       write_message(dispatcher->channel,&message);
> +    if (async) {
> +        send_data(dispatcher->channel,&cookie, sizeof(cookie));
> +    }
>       send_data(dispatcher->channel,&surface_id, sizeof(uint32_t));
>       send_data(dispatcher->channel, surface, sizeof(QXLDevSurfaceCreate));
> -    read_message(dispatcher->channel,&message);
> -    ASSERT(message == RED_WORKER_MESSAGE_READY);
> +    if (!async) {
> +        read_message(dispatcher->channel,&message);
> +        ASSERT(message == RED_WORKER_MESSAGE_READY);
> +    }
>
>       update_client_mouse_allowed();
same as the previous comment
>   }
>
> +static void qxl_worker_create_primary_surface(QXLWorker *qxl_worker, uint32_t surface_id,
> +                                      QXLDevSurfaceCreate *surface)
> +{
> +    qxl_worker_create_primary_surface_helper(qxl_worker, surface_id, surface, 0, 0);
> +}
> +
> +static void qxl_worker_create_primary_surface_async(QXLWorker *qxl_worker, uint32_t surface_id,
> +                                      QXLDevSurfaceCreate *surface, uint64_t cookie)
> +{
> +    qxl_worker_create_primary_surface_helper(qxl_worker, surface_id, surface, 1, cookie);
> +}
> +
>   static void qxl_worker_reset_image_cache(QXLWorker *qxl_worker)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> @@ -330,17 +396,40 @@ static void qxl_worker_reset_cursor(QXLWorker *qxl_worker)
>       ASSERT(message == RED_WORKER_MESSAGE_READY);
>   }
>
> -static void qxl_worker_destroy_surface_wait(QXLWorker *qxl_worker, uint32_t surface_id)
> +static void qxl_worker_destroy_surface_wait_helper(QXLWorker *qxl_worker, uint32_t surface_id,
> +                                                   int async, uint64_t cookie)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> -    RedWorkerMessage message = RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT;
> +    RedWorkerMessage message;
> +
> +    if (async ) {
> +        message = RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC;
> +    } else {
> +        message = RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT;
> +    }
>
>       write_message(dispatcher->channel,&message);
> +    if (async) {
> +        send_data(dispatcher->channel,&cookie, sizeof(cookie));
> +    }
>       send_data(dispatcher->channel,&surface_id, sizeof(uint32_t));
> +    if (async) {
> +        return;
> +    }
>       read_message(dispatcher->channel,&message);
>       ASSERT(message == RED_WORKER_MESSAGE_READY);
>   }
>
> +static void qxl_worker_destroy_surface_wait(QXLWorker *qxl_worker, uint32_t surface_id)
> +{
> +    qxl_worker_destroy_surface_wait_helper(qxl_worker, surface_id, 0, 0);
> +}
> +
> +static void qxl_worker_destroy_surface_wait_async(QXLWorker *qxl_worker, uint32_t surface_id, uint64_t cookie)
> +{
> +    qxl_worker_destroy_surface_wait_helper(qxl_worker, surface_id, 1, cookie);
> +}
> +
>   static void qxl_worker_reset_memslots(QXLWorker *qxl_worker)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> @@ -363,8 +452,9 @@ static void qxl_worker_wakeup(QXLWorker *qxl_worker)
>   static void qxl_worker_oom(QXLWorker *qxl_worker)
>   {
>       RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> +    RedWorkerMessage message = RED_WORKER_MESSAGE_OOM;
> +
>       if (!test_bit(RED_WORKER_PENDING_OOM, dispatcher->pending)) {
> -        RedWorkerMessage message = RED_WORKER_MESSAGE_OOM;
>           set_bit(RED_WORKER_PENDING_OOM,&dispatcher->pending);
>           write_message(dispatcher->channel,&message);
>       }
> @@ -530,14 +620,22 @@ RedDispatcher *red_dispatcher_init(QXLInstance *qxl)
>       dispatcher->base.del_memslot = qxl_worker_del_memslot;
>       dispatcher->base.reset_memslots = qxl_worker_reset_memslots;
>       dispatcher->base.destroy_surfaces = qxl_worker_destroy_surfaces;
> -    dispatcher->base.create_primary_surface = qxl_worker_create_primary;
> -    dispatcher->base.destroy_primary_surface = qxl_worker_destroy_primary;
> +    dispatcher->base.create_primary_surface = qxl_worker_create_primary_surface;
> +    dispatcher->base.destroy_primary_surface = qxl_worker_destroy_primary_surface;
>
>       dispatcher->base.reset_image_cache = qxl_worker_reset_image_cache;
>       dispatcher->base.reset_cursor = qxl_worker_reset_cursor;
>       dispatcher->base.destroy_surface_wait = qxl_worker_destroy_surface_wait;
>       dispatcher->base.loadvm_commands = qxl_worker_loadvm_commands;
>
> +    dispatcher->base.update_area_async = qxl_worker_update_area_async;
> +    dispatcher->base.add_memslot_async = qxl_worker_add_memslot_async;
> +    dispatcher->base.reset_memslots = qxl_worker_reset_memslots;
> +    dispatcher->base.destroy_surfaces_async = qxl_worker_destroy_surfaces_async;
> +    dispatcher->base.destroy_primary_surface_async = qxl_worker_destroy_primary_surface_async;
> +    dispatcher->base.create_primary_surface_async = qxl_worker_create_primary_surface_async;
> +    dispatcher->base.destroy_surface_wait_async = qxl_worker_destroy_surface_wait_async;
> +
>       qxl->st->qif->get_init_info(qxl,&init_info);
>
>       init_data.memslot_id_bits = init_info.memslot_id_bits;
> diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
> index 258a541..7737c04 100644
> --- a/server/red_parse_qxl.c
> +++ b/server/red_parse_qxl.c
> @@ -148,7 +148,7 @@ static void red_get_point16_ptr(SpicePoint16 *red, QXLPoint16 *qxl)
>       red->y = qxl->y;
>   }
>
> -void red_get_rect_ptr(SpiceRect *red, QXLRect *qxl)
> +void red_get_rect_ptr(SpiceRect *red, const QXLRect *qxl)
>   {
>       red->top    = qxl->top;
>       red->left   = qxl->left;
> diff --git a/server/red_parse_qxl.h b/server/red_parse_qxl.h
> index 5de0325..a713dcf 100644
> --- a/server/red_parse_qxl.h
> +++ b/server/red_parse_qxl.h
> @@ -110,7 +110,7 @@ typedef struct RedCursorCmd {
>       uint8_t *device_data;
>   } RedCursorCmd;
>
> -void red_get_rect_ptr(SpiceRect *red, QXLRect *qxl);
> +void red_get_rect_ptr(SpiceRect *red, const QXLRect *qxl);
>
>   void red_get_drawable(RedMemSlotInfo *slots, int group_id,
>                         RedDrawable *red, QXLPHYSICAL addr, uint32_t flags);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index d8d032d..425933d 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -9415,22 +9415,46 @@ static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item)
>       red_unref_channel(channel);
>   }
>
> +static inline void handle_dev_update_async(RedWorker *worker)
> +{
> +    QXLRect qxl_rect;
> +    SpiceRect rect;
> +    uint32_t surface_id;
> +    uint32_t clear_dirty_region;
> +
> +    receive_data(worker->channel,&surface_id, sizeof(uint32_t));
> +    receive_data(worker->channel,&qxl_rect, sizeof(QXLRect));
> +    receive_data(worker->channel,&clear_dirty_region, sizeof(uint32_t));
> +
> +    red_get_rect_ptr(&rect,&qxl_rect);
> +    flush_display_commands(worker);
> +
> +    ASSERT(worker->running);
> +
> +    validate_surface(worker, surface_id);
> +    red_update_area(worker,&rect, surface_id);
> +}
> +
>   static inline void handle_dev_update(RedWorker *worker)
>   {
> -    RedWorkerMessage message;
> -    const SpiceRect *rect;
> +    const QXLRect *qxl_rect;
> +    SpiceRect *rect = spice_new0(SpiceRect, 1);
> +    QXLRect *qxl_dirty_rects;
>       SpiceRect *dirty_rects;
>       RedSurface *surface;
>       uint32_t num_dirty_rects;
>       uint32_t surface_id;
>       uint32_t clear_dirty_region;
> +    int i;
>
>       receive_data(worker->channel,&surface_id, sizeof(uint32_t));
> -    receive_data(worker->channel,&rect, sizeof(SpiceRect *));
> -    receive_data(worker->channel,&dirty_rects, sizeof(SpiceRect *));
> +    receive_data(worker->channel,&qxl_rect, sizeof(QXLRect *));
> +    receive_data(worker->channel,&qxl_dirty_rects, sizeof(QXLRect *));
>       receive_data(worker->channel,&num_dirty_rects, sizeof(uint32_t));
>       receive_data(worker->channel,&clear_dirty_region, sizeof(uint32_t));
>
> +    dirty_rects = spice_new0(SpiceRect, num_dirty_rects);
> +    red_get_rect_ptr(rect, qxl_rect);
>       flush_display_commands(worker);
>
>       ASSERT(worker->running);
> @@ -9444,15 +9468,16 @@ static inline void handle_dev_update(RedWorker *worker)
>       if (clear_dirty_region) {
>           region_clear(&surface->draw_dirty_region);
>       }
> -
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel,&message);
> +    for (i = 0; i<  num_dirty_rects; i++) {
> +        qxl_dirty_rects[i].top    = dirty_rects[i].top;
> +        qxl_dirty_rects[i].left   = dirty_rects[i].left;
> +        qxl_dirty_rects[i].bottom = dirty_rects[i].bottom;
> +        qxl_dirty_rects[i].right  = dirty_rects[i].right;
> +    }

free for rect and dirty_rects are missing
>   }
>
> -
>   static inline void handle_dev_add_memslot(RedWorker *worker)
>   {
> -    RedWorkerMessage message;
>       QXLDevMemSlot dev_slot;
>
>       receive_data(worker->channel,&dev_slot, sizeof(QXLDevMemSlot));
> @@ -9460,9 +9485,6 @@ static inline void handle_dev_add_memslot(RedWorker *worker)
>       red_memslot_info_add_slot(&worker->mem_slots, dev_slot.slot_group_id, dev_slot.slot_id,
>                                 dev_slot.addr_delta, dev_slot.virt_start, dev_slot.virt_end,
>                                 dev_slot.generation);
> -
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel,&message);
>   }
>
>   static inline void handle_dev_del_memslot(RedWorker *worker)
> @@ -9498,7 +9520,6 @@ static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
>
>   static inline void handle_dev_destroy_surface_wait(RedWorker *worker)
>   {
> -    RedWorkerMessage message;
>       uint32_t surface_id;
>
>       receive_data(worker->channel,&surface_id, sizeof(uint32_t));
> @@ -9510,9 +9531,6 @@ static inline void handle_dev_destroy_surface_wait(RedWorker *worker)
>       if (worker->surfaces[0].context.canvas) {
>           destroy_surface_wait(worker, 0);
>       }
> -
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel,&message);
>   }
>
>   static inline void red_cursor_reset(RedWorker *worker)
> @@ -9540,7 +9558,6 @@ static inline void red_cursor_reset(RedWorker *worker)
>   static inline void handle_dev_destroy_surfaces(RedWorker *worker)
>   {
>       int i;
> -    RedWorkerMessage message;
>
>       flush_all_qxl_commands(worker);
>       //to handle better
> @@ -9563,14 +9580,10 @@ static inline void handle_dev_destroy_surfaces(RedWorker *worker)
>       red_display_clear_glz_drawables(worker->display_channel);
>
>       red_cursor_reset(worker);
> -
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel,&message);
>   }
>
>   static inline void handle_dev_create_primary_surface(RedWorker *worker)
>   {
> -    RedWorkerMessage message;
>       uint32_t surface_id;
>       QXLDevSurfaceCreate surface;
>       uint8_t *line_0;
> @@ -9600,14 +9613,10 @@ static inline void handle_dev_create_primary_surface(RedWorker *worker)
>       if (worker->cursor_channel) {
>           red_channel_pipe_add_type(&worker->cursor_channel->common.base, PIPE_ITEM_TYPE_CURSOR_INIT);
>       }
> -
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel,&message);
>   }
>
>   static inline void handle_dev_destroy_primary_surface(RedWorker *worker)
>   {
> -    RedWorkerMessage message;
>       uint32_t surface_id;
>
>       receive_data(worker->channel,&surface_id, sizeof(uint32_t));
> @@ -9623,22 +9632,78 @@ static inline void handle_dev_destroy_primary_surface(RedWorker *worker)
>       ASSERT(!worker->surfaces[surface_id].context.canvas);
>
>       red_cursor_reset(worker);
> +}
>
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel,&message);
> +static void handle_dev_stop(RedWorker *worker)
> +{
> +    int x;
> +
> +    ASSERT(worker->running);
> +    worker->running = FALSE;
> +    red_display_clear_glz_drawables(worker->display_channel);
> +    for (x = 0; x<  NUM_SURFACES; ++x) {
> +        if (worker->surfaces[x].context.canvas) {
> +            red_current_flush(worker, x);
> +        }
> +    }
> +    red_wait_outgoing_item((RedChannel *)worker->display_channel);
> +    red_wait_outgoing_item((RedChannel *)worker->cursor_channel);
> +}
> +
> +static void handle_dev_start(RedWorker *worker)
> +{
> +    RedChannel *cursor_red_channel =&worker->cursor_channel->common.base;
> +    RedChannel *display_red_channel =&worker->display_channel->common.base;
> +
> +    ASSERT(!worker->running);
> +    if (worker->cursor_channel) {
> +        cursor_red_channel->migrate = FALSE;
> +    }
> +    if (worker->display_channel) {
> +        display_red_channel->migrate = FALSE;
> +    }
> +    worker->running = TRUE;
>   }
>
>   static void handle_dev_input(EventListener *listener, uint32_t events)
>   {
>       RedWorker *worker = SPICE_CONTAINEROF(listener, RedWorker, dev_listener);
>       RedWorkerMessage message;
> -    RedChannel *cursor_red_channel =&worker->cursor_channel->common.base;
>       RedChannel *display_red_channel =&worker->display_channel->common.base;
>       int ring_is_empty;
> +    int call_async_complete = 0;
> +    int write_ready = 0;
> +    uint64_t cookie;
>
>       read_message(worker->channel,&message);
>
> +    /* for async messages we do the common work in the handler, and
> +     * send a ready or call async_complete from here, hence the added switch. */
> +    switch (message) {
> +    case RED_WORKER_MESSAGE_UPDATE_ASYNC:
> +    case RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC:
> +    case RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC:
> +    case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC:
> +    case RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC:
> +    case RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC:
> +        call_async_complete = 1;
> +        receive_data(worker->channel,&cookie, sizeof(cookie));
> +        break;
> +    case RED_WORKER_MESSAGE_UPDATE:
> +    case RED_WORKER_MESSAGE_ADD_MEMSLOT:
> +    case RED_WORKER_MESSAGE_DESTROY_SURFACES:
> +    case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE:
> +    case RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE:
> +    case RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT:
all synced operations that writes RED_WORKER_MESSAGE_READY can be add here
> +        write_ready = 1;
> +    default:
> +        break;
> +    }
> +
>       switch (message) {
> +    case RED_WORKER_MESSAGE_UPDATE_ASYNC:
> +        handle_dev_update_async(worker);
> +        break;
>       case RED_WORKER_MESSAGE_UPDATE:
>           handle_dev_update(worker);
>           break;
> @@ -9670,15 +9735,19 @@ static void handle_dev_input(EventListener *listener, uint32_t events)
>           message = RED_WORKER_MESSAGE_READY;
>           write_message(worker->channel,&message);
see comment above



Cheers,
Yonit


More information about the Spice-devel mailing list