On Wed, Jul 13, 2011 at 11:15:50AM +0300, 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
>
ok, so bottom line (for all of these) is:
1. I'll add an async_complete handler in red_dispatcher.c
2. I'll add mutex around the variables that reds.c (qemu iothread) and the
callback (worker thread) both touch.
3. I'll audit to make sure reds only accesses them via red_dispatcher code
that is mutex protected.
> > }
> >
> >-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
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
