ping
>
> This structure was used to store the cookie for the async
> reply and the message for the generic async callback.
> Most async messages do not require extra action beside
> sending back the cookie for the reply so instead of using
> a switch inline the replies and remove store the cookie
> directory instead of a pointer to AsyncCommand.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/red-qxl.c | 71
> +++++++++++++----------------------------------------
> server/red-qxl.h | 1 +
> server/red-worker.c | 18 ++++++++------
> server/red-worker.h | 5 ++--
> 4 files changed, 31 insertions(+), 64 deletions(-)
>
> Not a regression but this change shows that
> red_qxl_destroy_primary_surface_complete and
> red_qxl_create_primary_surface_complete are called from the worker
> thread. This possibly can be a race condition as they call some
> functions in red-qxl.c and reds.c that possibly use structures that
> should only be used in the main thread.
>
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 4ef293060..5815e4406 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -41,11 +41,6 @@
> #include "red-qxl.h"
>
>
> -struct AsyncCommand {
> - RedWorkerMessage message;
> - uint64_t cookie;
> -};
> -
> struct QXLState {
> QXLWorker qxl_worker;
> QXLInstance *qxl;
> @@ -205,19 +200,6 @@ gboolean red_qxl_client_monitors_config(QXLInstance
> *qxl,
> qxl_get_interface(qxl)->client_monitors_config(qxl,
> monitors_config));
> }
>
> -static AsyncCommand *async_command_alloc(QXLState *qxl_state,
> - RedWorkerMessage message,
> - uint64_t cookie)
> -{
> - AsyncCommand *async_command = spice_new0(AsyncCommand, 1);
> -
> - async_command->cookie = cookie;
> - async_command->message = message;
> -
> - spice_debug("%p", async_command);
> - return async_command;
> -}
> -
> static void red_qxl_update_area_async(QXLState *qxl_state,
> uint32_t surface_id,
> QXLRect *qxl_area,
> @@ -227,7 +209,7 @@ static void red_qxl_update_area_async(QXLState
> *qxl_state,
> RedWorkerMessage message = RED_WORKER_MESSAGE_UPDATE_ASYNC;
> RedWorkerMessageUpdateAsync payload;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> payload.surface_id = surface_id;
> payload.qxl_area = *qxl_area;
> payload.clear_dirty_region = clear_dirty_region;
> @@ -266,7 +248,7 @@ static void red_qxl_add_memslot_async(QXLState
> *qxl_state, QXLDevMemSlot *mem_sl
> RedWorkerMessageAddMemslotAsync payload;
> RedWorkerMessage message = RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> payload.mem_slot = *mem_slot;
> dispatcher_send_message(qxl_state->dispatcher, message, &payload);
> }
> @@ -307,11 +289,11 @@ static void red_qxl_destroy_surfaces_async(QXLState
> *qxl_state, uint64_t cookie)
> RedWorkerMessageDestroySurfacesAsync payload;
> RedWorkerMessage message = RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> dispatcher_send_message(qxl_state->dispatcher, message, &payload);
> }
>
> -static void red_qxl_destroy_primary_surface_complete(QXLState *qxl_state)
> +void red_qxl_destroy_primary_surface_complete(QXLState *qxl_state)
> {
> qxl_state->x_res = 0;
> qxl_state->y_res = 0;
> @@ -340,7 +322,7 @@ red_qxl_destroy_primary_surface_async(QXLState
> *qxl_state,
> RedWorkerMessageDestroyPrimarySurfaceAsync payload;
> RedWorkerMessage message =
> RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> payload.surface_id = surface_id;
> dispatcher_send_message(qxl_state->dispatcher, message, &payload);
> }
> @@ -362,7 +344,7 @@ static void qxl_worker_destroy_primary_surface(QXLWorker
> *qxl_worker, uint32_t s
> red_qxl_destroy_primary_surface(qxl_state, surface_id, 0, 0);
> }
>
> -static void red_qxl_create_primary_surface_complete(QXLState *qxl_state)
> +void red_qxl_create_primary_surface_complete(QXLState *qxl_state)
> {
> QXLDevSurfaceCreate *surface = &qxl_state->surface_create;
>
> @@ -383,7 +365,7 @@ red_qxl_create_primary_surface_async(QXLState *qxl_state,
> uint32_t surface_id,
> RedWorkerMessage message =
> RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC;
>
> qxl_state->surface_create = *surface;
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> payload.surface_id = surface_id;
> payload.surface = *surface;
> dispatcher_send_message(qxl_state->dispatcher, message, &payload);
> @@ -470,7 +452,7 @@ static void red_qxl_destroy_surface_wait_async(QXLState
> *qxl_state,
> RedWorkerMessageDestroySurfaceWaitAsync payload;
> RedWorkerMessage message =
> RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> payload.surface_id = surface_id;
> dispatcher_send_message(qxl_state->dispatcher, message, &payload);
> }
> @@ -574,7 +556,7 @@ static void red_qxl_flush_surfaces_async(QXLState
> *qxl_state, uint64_t cookie)
> RedWorkerMessageFlushSurfacesAsync payload;
> RedWorkerMessage message = RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> dispatcher_send_message(qxl_state->dispatcher, message, &payload);
> }
>
> @@ -586,7 +568,7 @@ static void red_qxl_monitors_config_async(QXLState
> *qxl_state,
> RedWorkerMessageMonitorsConfigAsync payload;
> RedWorkerMessage message = RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC;
>
> - payload.base.cmd = async_command_alloc(qxl_state, message, cookie);
> + payload.base.cookie = cookie;
> payload.monitors_config = monitors_config;
> payload.group_id = group_id;
> payload.max_monitors = qxl_state->max_monitors;
> @@ -891,32 +873,6 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl,
> dispatcher_send_message(qxl_state->dispatcher, message, &draw);
> }
>
> -void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command)
> -{
> - QXLInterface *interface = qxl_get_interface(qxl);
> -
> - spice_debug("%p: cookie %" PRId64, async_command,
> async_command->cookie);
> - switch (async_command->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_DESTROY_SURFACE_WAIT_ASYNC:
> - case RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC:
> - case RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC:
> - break;
> - case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC:
> - red_qxl_create_primary_surface_complete(qxl->st);
> - break;
> - case RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC:
> - red_qxl_destroy_primary_surface_complete(qxl->st);
> - break;
> - default:
> - spice_warning("unexpected message %d", async_command->message);
> - }
> - interface->async_complete(qxl, async_command->cookie);
> - free(async_command);
> -}
> -
> void red_qxl_gl_draw_async_complete(QXLInstance *qxl)
> {
> QXLInterface *interface = qxl_get_interface(qxl);
> @@ -1151,3 +1107,10 @@ void red_qxl_set_client_capabilities(QXLInstance *qxl,
>
> interface->set_client_capabilities(qxl, client_present, caps);
> }
> +
> +void red_qxl_async_complete(QXLInstance *qxl, uint64_t cookie)
> +{
> + QXLInterface *interface = qxl_get_interface(qxl);
> +
> + interface->async_complete(qxl, cookie);
> +}
> diff --git a/server/red-qxl.h b/server/red-qxl.h
> index 5896113e1..51ec14562 100644
> --- a/server/red-qxl.h
> +++ b/server/red-qxl.h
> @@ -51,6 +51,7 @@ int red_qxl_get_cursor_command(QXLInstance *qxl, struct
> QXLCommandExt *cmd);
> int red_qxl_req_cursor_notification(QXLInstance *qxl);
> void red_qxl_notify_update(QXLInstance *qxl, uint32_t update_id);
> int red_qxl_flush_resources(QXLInstance *qxl);
> +void red_qxl_async_complete(QXLInstance *qxl, uint64_t cookie);
> void red_qxl_update_area_complete(QXLInstance *qxl, uint32_t surface_id,
> struct QXLRect *updated_rects,
> uint32_t num_updated_rects);
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 58d9e0add..7238a66e8 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -432,7 +432,7 @@ static void handle_dev_update_async(void *opaque, void
> *payload)
> red_qxl_update_area_complete(worker->qxl, msg->surface_id,
> qxl_dirty_rects, num_dirty_rects);
> free(qxl_dirty_rects);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_update(void *opaque, void *payload)
> @@ -583,7 +583,8 @@ static void handle_dev_destroy_primary_surface_async(void
> *opaque, void *payload
> uint32_t surface_id = msg->surface_id;
>
> destroy_primary_surface(worker, surface_id);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_destroy_primary_surface_complete(worker->qxl->st);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_flush_surfaces_async(void *opaque, void *payload)
> @@ -593,7 +594,7 @@ static void handle_dev_flush_surfaces_async(void *opaque,
> void *payload)
>
> flush_all_qxl_commands(worker);
> display_channel_flush_all_surfaces(worker->display_channel);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_stop(void *opaque, void *payload)
> @@ -693,7 +694,7 @@ static void handle_dev_destroy_surface_wait_async(void
> *opaque, void *payload)
> RedWorker *worker = opaque;
>
> display_channel_destroy_surface_wait(worker->display_channel,
> msg->surface_id);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_destroy_surfaces_async(void *opaque, void *payload)
> @@ -704,7 +705,7 @@ static void handle_dev_destroy_surfaces_async(void
> *opaque, void *payload)
> flush_all_qxl_commands(worker);
> display_channel_destroy_surfaces(worker->display_channel);
> cursor_channel_reset(worker->cursor_channel);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_create_primary_surface_async(void *opaque, void
> *payload)
> @@ -713,7 +714,8 @@ static void handle_dev_create_primary_surface_async(void
> *opaque, void *payload)
> RedWorker *worker = opaque;
>
> dev_create_primary_surface(worker, msg->surface_id, msg->surface);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_create_primary_surface_complete(worker->qxl->st);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_display_connect(void *opaque, void *payload)
> @@ -811,7 +813,7 @@ static void handle_dev_monitors_config_async(void
> *opaque, void *payload)
> MIN(count, msg->max_monitors),
> MIN(max_allowed,
> msg->max_monitors));
> async_complete:
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> /* TODO: special, perhaps use another dispatcher? */
> @@ -938,7 +940,7 @@ static void handle_dev_add_memslot_async(void *opaque,
> void *payload)
> RedWorker *worker = opaque;
>
> dev_add_memslot(worker, msg->mem_slot);
> - red_qxl_async_complete(worker->qxl, msg->base.cmd);
> + red_qxl_async_complete(worker->qxl, msg->base.cookie);
> }
>
> static void handle_dev_reset_memslots(void *opaque, void *payload)
> diff --git a/server/red-worker.h b/server/red-worker.h
> index 3d918575c..f29840c4e 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -36,8 +36,9 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> bool red_worker_run(RedWorker *worker);
> void red_worker_free(RedWorker *worker);
>
> -void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command);
> struct Dispatcher *red_qxl_get_dispatcher(QXLInstance *qxl);
> +void red_qxl_destroy_primary_surface_complete(QXLState *qxl_state);
> +void red_qxl_create_primary_surface_complete(QXLState *qxl_state);
>
> typedef uint32_t RedWorkerMessage;
>
> @@ -137,7 +138,7 @@ typedef struct RedWorkerMessageUpdate {
> } RedWorkerMessageUpdate;
>
> typedef struct RedWorkerMessageAsync {
> - AsyncCommand *cmd;
> + uint64_t cookie;
> } RedWorkerMessageAsync;
>
> typedef struct RedWorkerMessageUpdateAsync {
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel