On 10/21/2015 03:00 PM, Frediano Ziglio wrote:
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;
+}
+
Hi,
This change can go in a separate patch. (mark [1])
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"
Why do we need this header here ?
typedef struct Dispatcher Dispatcher;
@@ -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);
same as [1]
/*
* 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);
Note, here it's spice_assert. mark [2]
-
+ // TODO: reference and free
+ RedWorker *worker = red_worker_new(&init_data);
+ red_worker_run(worker);
return value is not checked (related to [2])
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"
@@ -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));
-}
-
see [1]
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;
See [2] -- returning false
+}
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
If splitting this patch is not an option we can accept it
and add patches on top of it.
Ack.
- Uri
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel