Hi
On Sun, Oct 9, 2011 at 4:29 PM, Alon Levy <al...@redhat.com> wrote:
> ---
> server/red_dispatcher.c | 70 +++++++++++++++++++++++++++++++++-------------
> 1 files changed, 50 insertions(+), 20 deletions(-)
>
> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> index a998238..8289c6b 100644
> --- a/server/red_dispatcher.c
> +++ b/server/red_dispatcher.c
> @@ -40,6 +40,13 @@ static int num_active_workers = 0;
>
> //volatile
>
> +typedef struct AsyncCommand AsyncCommand;
> +struct AsyncCommand {
> + AsyncCommand *next;
> + RedWorkerMessage message;
> + uint64_t cookie;
> +};
> +
> struct RedDispatcher {
> QXLWorker base;
> QXLInstance *qxl;
> @@ -51,7 +58,7 @@ struct RedDispatcher {
> int y_res;
> int use_hardware_cursor;
> RedDispatcher *next;
> - RedWorkerMessage async_message;
> + AsyncCommand* async_commands;
> pthread_mutex_t async_lock;
> QXLDevSurfaceCreate surface_create;
> };
> @@ -224,17 +231,27 @@ static void red_dispatcher_update_area(RedDispatcher
> *dispatcher, uint32_t surfa
> ASSERT(message == RED_WORKER_MESSAGE_READY);
> }
>
> +static AsyncCommand *push_async_command(RedDispatcher *dispatcher,
> + RedWorkerMessage message,
> + uint64_t cookie)
> +{
> + AsyncCommand *async_command = spice_new(AsyncCommand, 1);
> +
> + async_command->cookie = cookie;
> + async_command->message = message;
> + if (dispatcher->async_commands) {
> + async_command->next = dispatcher->async_commands;
> + }
The assignment could be unconditionnal.
Shouldn't the function take the async_lock instead of the caller?
> + dispatcher->async_commands = async_command;
> + return async_command;
> +}
> +
> static RedWorkerMessage red_dispatcher_async_start(RedDispatcher *dispatcher,
> - RedWorkerMessage message)
> + RedWorkerMessage message,
> + uint64_t cookie)
> {
> pthread_mutex_lock(&dispatcher->async_lock);
> - if (dispatcher->async_message != RED_WORKER_MESSAGE_NOP) {
> - red_warn("warning: async clash. have %d, ignoring %d",
> - dispatcher->async_message, message);
> - pthread_mutex_unlock(&dispatcher->async_lock);
> - return RED_WORKER_MESSAGE_NOP;
> - }
> - dispatcher->async_message = message;
> + push_async_command(dispatcher, message, cookie);
> pthread_mutex_unlock(&dispatcher->async_lock);
> return message;
> }
> @@ -246,7 +263,8 @@ static void
> red_dispatcher_update_area_async(RedDispatcher *dispatcher,
> uint64_t cookie)
> {
> RedWorkerMessage message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_UPDATE_ASYNC);
> +
> RED_WORKER_MESSAGE_UPDATE_ASYNC,
> + cookie);
>
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> @@ -265,7 +283,7 @@ static void
> red_dispatcher_update_area_dirty_async(RedDispatcher *dispatcher,
> uint32_t clear_dirty_region, uint64_t
> cookie)
> {
> RedWorkerMessage message = red_dispatcher_async_start(
> - dispatcher, RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC);
> + dispatcher, RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC, cookie);
>
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> @@ -306,7 +324,8 @@ static void qxl_worker_add_memslot(QXLWorker *qxl_worker,
> QXLDevMemSlot *mem_slo
> static void red_dispatcher_add_memslot_async(RedDispatcher *dispatcher,
> QXLDevMemSlot *mem_slot, uint64_t cookie)
> {
> RedWorkerMessage message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC);
> +
> RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC,
> + cookie);
>
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> @@ -347,7 +366,8 @@ static void qxl_worker_destroy_surfaces(QXLWorker
> *qxl_worker)
> static void red_dispatcher_destroy_surfaces_async(RedDispatcher *dispatcher,
> uint64_t cookie)
> {
> RedWorkerMessage message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC);
> +
> RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC,
> + cookie);
>
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> @@ -374,7 +394,8 @@ red_dispatcher_destroy_primary_surface(RedDispatcher
> *dispatcher,
>
> if (async) {
> message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC);
> +
> RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC,
> + cookie);
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> }
> @@ -420,7 +441,8 @@ red_dispatcher_create_primary_surface(RedDispatcher
> *dispatcher, uint32_t surfac
>
> if (async) {
> message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC);
> +
> RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC,
> + cookie);
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> }
> @@ -483,7 +505,8 @@ static void
> red_dispatcher_destroy_surface_wait(RedDispatcher *dispatcher, uint3
>
> if (async ) {
> message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC);
> +
> RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC,
> + cookie);
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> }
> @@ -563,7 +586,8 @@ static void qxl_worker_start(QXLWorker *qxl_worker)
> static void red_dispatcher_flush_surfaces_async(RedDispatcher *dispatcher,
> uint64_t cookie)
> {
> RedWorkerMessage message = red_dispatcher_async_start(dispatcher,
> -
> RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC);
> +
> RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC,
> + cookie);
>
> if (message == RED_WORKER_MESSAGE_NOP) {
> return;
> @@ -836,8 +860,15 @@ void spice_qxl_flush_surfaces_async(QXLInstance
> *instance, uint64_t cookie)
>
> void red_dispatcher_async_complete(struct RedDispatcher *dispatcher,
> uint64_t cookie)
> {
> + AsyncCommand *async_command;
> pthread_mutex_lock(&dispatcher->async_lock);
> - switch (dispatcher->async_message) {
> + for (async_command = dispatcher->async_commands;
> + async_command && async_command->cookie != cookie &&
> async_command->next;
I think the "&& async_command->next" condition is unnecessary.
> + async_command = async_command->next) {};
> + if (!async_command || async_command->cookie != cookie) {
So you can only check if (!async_command)
> + red_warn("unknown cookie");
And should return or goto fail to avoid crash later.
> + }
> + switch (async_command->message) {
> case RED_WORKER_MESSAGE_UPDATE_ASYNC:
> case RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC:
> break;
> @@ -858,7 +889,7 @@ void red_dispatcher_async_complete(struct RedDispatcher
> *dispatcher, uint64_t co
> default:
> red_warn("unexpected message");
> }
> - dispatcher->async_message = RED_WORKER_MESSAGE_NOP;
> + free(async_command);
I might be missing something but where is dispatcher->async_commands updated?
> pthread_mutex_unlock(&dispatcher->async_lock);
> dispatcher->qxl->st->qif->async_complete(dispatcher->qxl, cookie);
> }
> @@ -895,7 +926,6 @@ RedDispatcher *red_dispatcher_init(QXLInstance *qxl)
> init_data.num_renderers = num_renderers;
> memcpy(init_data.renderers, renderers, sizeof(init_data.renderers));
>
> - dispatcher->async_message = RED_WORKER_MESSAGE_NOP;
> pthread_mutex_init(&dispatcher->async_lock, NULL);
> init_data.image_compression = image_compression;
> init_data.jpeg_state = jpeg_state;
> --
> 1.7.6.4
>
>
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
