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

Wed Jul 13 01:47:48 PDT 2011

On 07/13/2011 11:15 AM, Yonit Halperin wrote:
> 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
>
Well, I made a mistake about the thread context, since the dispatcher 
doesn't wait for messages from the worker.
>> }
>>
>> -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