Re: [PATCH 3/9] server: remove worker thread creation from dispatcher

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> 
> ---
>  server/dispatcher.c     | 13 +++++++++++++
>  server/dispatcher.h     |  2 ++
>  server/red_dispatcher.c | 21 +++------------------
>  server/red_dispatcher.h |  6 +-----
>  server/red_worker.c     | 33 ++++++++++++++++++++++++++++++---
>  server/red_worker.h     |  5 ++++-
>  6 files changed, 53 insertions(+), 27 deletions(-)
> 
> diff --git a/server/dispatcher.c b/server/dispatcher.c
> index d6c03ca..d334117 100644
> --- a/server/dispatcher.c
> +++ b/server/dispatcher.c
> @@ -32,6 +32,7 @@
>  #include "common/mem.h"
>  #include "common/spice_common.h"
>  #include "dispatcher.h"
> +#include "red_dispatcher.h"
>  
>  //#define DEBUG_DISPATCHER
>  
> @@ -200,6 +201,18 @@ unlock:
>      pthread_mutex_unlock(&dispatcher->lock);
>  }
>  
> +uint32_t dispatcher_read_message(Dispatcher *dispatcher)
> +{
> +    uint32_t message;
> +
> +    spice_return_val_if_fail(dispatcher, 0);
> +    spice_return_val_if_fail(dispatcher->send_fd != -1, 0);
> +
> +    receive_data(dispatcher->send_fd, &message, sizeof(message));
> +
> +    return message;
> +}
> +
>  void dispatcher_register_async_done_callback(
>                                          Dispatcher *dispatcher,
>                                          dispatcher_handle_async_done
>                                          handler)
> diff --git a/server/dispatcher.h b/server/dispatcher.h
> index c3e7c74..8882532 100644
> --- a/server/dispatcher.h
> +++ b/server/dispatcher.h
> @@ -19,6 +19,7 @@
>  #define DISPATCHER_H
>  
>  #include <spice.h>
> +#include "spice_server_utils.h"
>  
>  typedef struct Dispatcher Dispatcher;
>  

This include can be removed

> @@ -63,6 +64,7 @@ struct Dispatcher {
>   */
>  void dispatcher_send_message(Dispatcher *dispatcher, uint32_t message_type,
>                               void *payload);
> +uint32_t dispatcher_read_message(Dispatcher *dispatcher);
>  
>  /*
>   * dispatcher_init
> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> index 7ad860c..0bc853d 100644
> --- a/server/red_dispatcher.c
> +++ b/server/red_dispatcher.c
> @@ -56,7 +56,6 @@ struct RedDispatcher {
>      QXLWorker base;
>      QXLInstance *qxl;
>      Dispatcher dispatcher;
> -    pthread_t worker_thread;
>      uint32_t pending;
>      int primary_active;
>      int x_res;
> @@ -1064,14 +1063,10 @@ static RedChannel
> *red_dispatcher_cursor_channel_create(RedDispatcher *dispatche
>  void red_dispatcher_init(QXLInstance *qxl)
>  {
>      RedDispatcher *red_dispatcher;
> -    RedWorkerMessage message;
>      WorkerInitData init_data;
>      QXLDevInitInfo init_info;
> -    int r;
>      RedChannel *display_channel;
>      RedChannel *cursor_channel;
> -    sigset_t thread_sig_mask;
> -    sigset_t curr_sig_mask;
>      ClientCbs client_cbs = { NULL, };
>  
>      spice_return_if_fail(qxl->st->dispatcher == NULL);
> @@ -1135,19 +1130,9 @@ void red_dispatcher_init(QXLInstance *qxl)
>  
>      num_active_workers = 1;
>  
> -    sigfillset(&thread_sig_mask);
> -    sigdelset(&thread_sig_mask, SIGILL);
> -    sigdelset(&thread_sig_mask, SIGFPE);
> -    sigdelset(&thread_sig_mask, SIGSEGV);
> -    pthread_sigmask(SIG_SETMASK, &thread_sig_mask, &curr_sig_mask);
> -    if ((r = pthread_create(&red_dispatcher->worker_thread, NULL,
> red_worker_main, &init_data))) {
> -        spice_error("create thread failed %d", r);
> -    }
> -    pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL);
> -
> -    read_message(red_dispatcher->dispatcher.send_fd, &message);
> -    spice_assert(message == RED_WORKER_MESSAGE_READY);
> -
> +    // TODO: reference and free
> +    RedWorker *worker = red_worker_new(&init_data);
> +    red_worker_run(worker);
>      display_channel = red_dispatcher_display_channel_create(red_dispatcher);
>  
>      if (display_channel) {
> diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
> index dbe65d1..2121007 100644
> --- a/server/red_dispatcher.h
> +++ b/server/red_dispatcher.h
> @@ -18,6 +18,7 @@
>  #ifndef _H_RED_DISPATCHER
>  #define _H_RED_DISPATCHER
>  
> +#include <unistd.h>
>  #include <errno.h>
>  #include "spice_server_utils.h"
>  #include "red_channel.h"

Even this include

> @@ -83,11 +84,6 @@ static inline void receive_data(int fd, void *in_buf, int
> n)
>      } while (n);
>  }
>  
> -static inline void read_message(int fd, RedWorkerMessage *message)
> -{
> -    receive_data(fd, message, sizeof(RedWorkerMessage));
> -}
> -
>  enum {
>      RED_WORKER_MESSAGE_NOP,
>      RED_WORKER_MESSAGE_UPDATE,
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 225c272..daa233b 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -868,6 +868,7 @@ typedef struct ItemTrace {
>  #define NUM_CURSORS 100
>  
>  typedef struct RedWorker {
> +    pthread_t thread;
>      DisplayChannel *display_channel;
>      CursorChannel *cursor_channel;
>      QXLInstance *qxl;
> @@ -11849,7 +11850,7 @@ static void handle_dev_input(int fd, int event, void
> *opaque)
>      dispatcher_handle_recv_read(red_dispatcher_get_dispatcher(worker->red_dispatcher));
>  }
>  
> -static RedWorker* red_worker_new(WorkerInitData *init_data)
> +RedWorker* red_worker_new(WorkerInitData *init_data)
>  {
>      RedWorker *worker = spice_new0(RedWorker, 1);
>      RedWorkerMessage message;
> @@ -11954,14 +11955,15 @@ static void
> red_display_cc_free_glz_drawables(RedChannelClient *rcc)
>      red_display_handle_glz_drawables_to_free(dcc);
>  }
>  
> -SPICE_GNUC_NORETURN void *red_worker_main(void *arg)
> +SPICE_GNUC_NORETURN static void *red_worker_main(void *arg)
>  {
> -    RedWorker *worker = red_worker_new(arg);
> +    RedWorker *worker = arg;
>  
>      spice_info("begin");
>      spice_assert(MAX_PIPE_SIZE > WIDE_CLIENT_ACK_WINDOW &&
>             MAX_PIPE_SIZE > NARROW_CLIENT_ACK_WINDOW); //ensure wakeup by ack
>             message
>  
> +    // TODO: call once unconditionnally
>      if (pthread_getcpuclockid(pthread_self(), &clock_id)) {
>          spice_error("pthread_getcpuclockid failed");
>      }
> @@ -12028,3 +12030,28 @@ SPICE_GNUC_NORETURN void *red_worker_main(void *arg)
>  
>      spice_warn_if_reached();
>  }
> +
> +bool red_worker_run(RedWorker *worker)
> +{
> +    uint32_t message;
> +    sigset_t thread_sig_mask;
> +    sigset_t curr_sig_mask;
> +    int r;
> +
> +    spice_return_val_if_fail(worker, FALSE);
> +    spice_return_val_if_fail(!worker->thread, FALSE);
> +
> +    sigfillset(&thread_sig_mask);
> +    sigdelset(&thread_sig_mask, SIGILL);
> +    sigdelset(&thread_sig_mask, SIGFPE);
> +    sigdelset(&thread_sig_mask, SIGSEGV);
> +    pthread_sigmask(SIG_SETMASK, &thread_sig_mask, &curr_sig_mask);
> +    if ((r = pthread_create(&worker->thread, NULL, red_worker_main,
> worker))) {
> +        spice_error("create thread failed %d", r);
> +    }
> +    pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL);
> +
> +    message =
> dispatcher_read_message(red_dispatcher_get_dispatcher(worker->red_dispatcher));
> +
> +    return message == RED_WORKER_MESSAGE_READY;
> +}
> diff --git a/server/red_worker.h b/server/red_worker.h
> index c71e9c8..c935e0a 100644
> --- a/server/red_worker.h
> +++ b/server/red_worker.h
> @@ -37,6 +37,8 @@ enum {
>      RED_RENDERER_LAST
>  };
>  
> +typedef struct RedWorker RedWorker;
> +
>  typedef struct WorkerInitData {
>      struct QXLInstance *qxl;
>      int id;
> @@ -56,6 +58,7 @@ typedef struct WorkerInitData {
>      RedDispatcher *red_dispatcher;
>  } WorkerInitData;
>  
> -void *red_worker_main(void *arg);
> +RedWorker* red_worker_new(WorkerInitData *init_data);
> +bool       red_worker_run(RedWorker *worker);
>  
>  #endif

Beside these small changes I agree with this patch.

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]