>From a844bf615a17b26e0a62856c4cabf72ffed705b1 Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Sun, 13 Mar 2011 19:45:45 +0200 Subject: [PATCH 2/6] qxl: fix locking, do not lock from spice server thread instead request iothread to do the operation, and wait for its response. --- hw/qxl.c | 70 +++++++++++++++++++++++++++++++++++++++++++++------ hw/qxl.h | 3 +- ui/spice-display.c | 7 +++-- ui/spice-display.h | 5 +++ 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 61fd1fc..d36c06c 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -28,6 +28,8 @@ #include "qxl.h" +#define ASSERT assert + #undef SPICE_RING_PROD_ITEM #define SPICE_RING_PROD_ITEM(r, ret) { \ typeof(r) start = r; \ @@ -117,6 +119,15 @@ static QXLMode qxl_modes[] = { #endif }; + +/* + * commands/requests from server thread to iothread. + */ +enum { + QXL_SERVER_SET_IRQ = 1, + QXL_SERVER_CREATE_UPDATE, +}; + static PCIQXLDevice *qxl0; static void qxl_send_events(PCIQXLDevice *d, uint32_t events); @@ -344,11 +355,24 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) QXLCommandRing *ring; QXLCommand *cmd; int notify; + int r; + unsigned char req, ack; switch (qxl->mode) { case QXL_MODE_VGA: dprint(qxl, 2, "%s: vga\n", __FUNCTION__); - update = qemu_spice_create_update(&qxl->ssd); + + /* TODO -- 1. check write+read return codes. retry upon e.g. EINTR */ + /* TODO -- 2. change the imlpementation inside qemu_spice_create_update (option rename+add functions) */ + req = QXL_SERVER_CREATE_UPDATE; + r = write(qxl->pipe[1], &req , 1); + ASSERT(r == 1); + ack = 0; + r = read(qxl->pipe_out[0], &ack, 1); + ASSERT(r == 1); + ASSERT(ack == QXL_SERVER_CREATE_UPDATE); // ack==req + update = qxl->ssd.update; + //update = qemu_spice_create_update(&qxl->ssd); if (update == NULL) { return false; } @@ -1058,16 +1082,27 @@ static void qxl_map(PCIDevice *pci, int region_num, static void pipe_read(void *opaque) { PCIQXLDevice *d = opaque; - char dummy; + unsigned char cmd; int len; do { - len = read(d->pipe[0], &dummy, sizeof(dummy)); - } while (len == sizeof(dummy)); - qxl_set_irq(d); + len = read(d->pipe[0], &cmd, sizeof(cmd)); + } while (len == sizeof(cmd)); + switch (cmd) { + case QXL_SERVER_SET_IRQ: + qxl_set_irq(d); + break; + case QXL_SERVER_CREATE_UPDATE: + d->ssd.update = qemu_spice_create_update(&d->ssd); + len = write(d->pipe_out[1], &cmd, 1); + ASSERT(len == 1); + break; + default: + fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd); + abort(); + } } -/* called from spice server thread context only */ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) { uint32_t old_pending; @@ -1079,10 +1114,17 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) return; } if (pthread_self() == d->main) { + /* running in io_thread thread */ qxl_set_irq(d); } else { - if (write(d->pipe[1], d, 1) != 1) { - dprint(d, 1, "%s: write to pipe failed\n", __FUNCTION__); + /* called from spice-server thread */ + int ret; + unsigned char ack = QXL_SERVER_SET_IRQ; + ret = write(d->pipe[1], &ack, 1); + if (ret != 1) { + fprintf(stderr, "%s: write to iothread pipe failed (%d) %m\n", + __FUNCTION__, ret); + perror(""); } } } @@ -1090,19 +1132,29 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) static void init_pipe_signaling(PCIQXLDevice *d) { if (pipe(d->pipe) < 0) { - dprint(d, 1, "%s: pipe creation failed\n", __FUNCTION__); + fprintf(stderr, "%s: pipe creation failed\n", __FUNCTION__); return; } + +#if 0 // why non-blocking ? code does not check that case #ifdef CONFIG_IOTHREAD fcntl(d->pipe[0], F_SETFL, O_NONBLOCK); #else fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */); #endif fcntl(d->pipe[1], F_SETFL, O_NONBLOCK); +#endif //(if 0) + fcntl(d->pipe[0], F_SETOWN, getpid()); d->main = pthread_self(); qemu_set_fd_handler(d->pipe[0], pipe_read, NULL, d); + + + if (pipe(d->pipe_out) < 0) { + fprintf(stderr, "%s: pipe creation failed\n", __FUNCTION__); + return; + } } /* graphics console */ diff --git a/hw/qxl.h b/hw/qxl.h index f6c450d..b153e4b 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -57,7 +57,8 @@ typedef struct PCIQXLDevice { /* thread signaling */ pthread_t main; - int pipe[2]; + int pipe[2]; /* to iothread */ + int pipe_out[2]; /* from iothread */ /* ram pci bar */ QXLRam *ram; diff --git a/ui/spice-display.c b/ui/spice-display.c index 99d9fd6..c7cf3b8 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -79,9 +79,9 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) uint8_t *src, *dst; int by, bw, bh; - qemu_mutex_lock_iothread(); + /* qemu_mutex_lock_iothread(); */ if (qemu_spice_rect_is_empty(&ssd->dirty)) { - qemu_mutex_unlock_iothread(); + /* qemu_mutex_unlock_iothread(); */ return NULL; }; @@ -142,7 +142,7 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) cmd->data = (intptr_t)drawable; memset(&ssd->dirty, 0, sizeof(ssd->dirty)); - qemu_mutex_unlock_iothread(); + /* qemu_mutex_unlock_iothread(); */ return update; } @@ -301,6 +301,7 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) SimpleSpiceUpdate *update; dprint(3, "%s:\n", __FUNCTION__); + assert((const char *)"TODO -- fixme" == NULL); update = qemu_spice_create_update(ssd); if (update == NULL) { return false; diff --git a/ui/spice-display.h b/ui/spice-display.h index aef0464..e1fc398 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -31,6 +31,8 @@ #define NUM_SURFACES 1024 +struct SimpleSpiceUpdate; + typedef struct SimpleSpiceDisplay { DisplayState *ds; void *buf; @@ -43,6 +45,9 @@ typedef struct SimpleSpiceDisplay { QXLRect dirty; int notify; int running; + + /* ssd updates (one request/command at a time) */ + struct SimpleSpiceUpdate *update; } SimpleSpiceDisplay; typedef struct SimpleSpiceUpdate { -- 1.7.4.1