[Spice-devel] [PATCH server 0.8 3/3] red_dispatcher: support concurrent asyncs

Marc-André Lureau marcandre.lureau at gmail.com
Tue Oct 11 05:33:06 PDT 2011


Hi

On Sun, Oct 9, 2011 at 4:29 PM, Alon Levy <alevy at 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


More information about the Spice-devel mailing list