[Spice-devel] [PATCH spice-server 1/2] red-qxl: Remove AsyncCommand
Frediano Ziglio
fziglio at redhat.com
Mon Sep 11 08:08:54 UTC 2017
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 at redhat.com>
---
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 {
--
2.13.5
More information about the Spice-devel
mailing list