From patchwork Wed May 11 13:28:00 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerd Hoffmann X-Patchwork-Id: 95144 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 8963D1007D7 for ; Wed, 11 May 2011 23:29:54 +1000 (EST) Received: from localhost ([::1]:46048 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QK9U7-0007Ji-Ps for incoming@patchwork.ozlabs.org; Wed, 11 May 2011 09:29:51 -0400 Received: from eggs.gnu.org ([140.186.70.92]:36148) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QK9SW-0004e7-1g for qemu-devel@nongnu.org; Wed, 11 May 2011 09:28:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QK9SQ-0007bW-8f for qemu-devel@nongnu.org; Wed, 11 May 2011 09:28:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6309) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QK9SP-0007b6-W1 for qemu-devel@nongnu.org; Wed, 11 May 2011 09:28:06 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p4BDS4Hf002257 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 11 May 2011 09:28:04 -0400 Received: from rincewind.home.kraxel.org (vpn1-7-108.ams2.redhat.com [10.36.7.108]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p4BDS3aM023339; Wed, 11 May 2011 09:28:03 -0400 Received: by rincewind.home.kraxel.org (Postfix, from userid 500) id 147E24296B; Wed, 11 May 2011 15:28:03 +0200 (CEST) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Wed, 11 May 2011 15:28:00 +0200 Message-Id: <1305120482-8207-4-git-send-email-kraxel@redhat.com> In-Reply-To: <1305120482-8207-1-git-send-email-kraxel@redhat.com> References: <1305120482-8207-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: Gerd Hoffmann Subject: [Qemu-devel] [PATCH 3/5] spice: don't create updates in spice server context. X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org This patch moves the creation of spice screen updates from the spice server context to qemu iothread context (display refresh timer to be exact). This way we avoid accessing qemu internals (display surface) from spice thread context which in turn allows us to simplify locking. Signed-off-by: Gerd Hoffmann (cherry picked from commit e0c64d08d11736dcea7c5a6373e3e7f62db51d9e) --- hw/qxl.c | 17 +++++++++++------ ui/spice-display.c | 43 +++++++++++++++++++++++++++---------------- ui/spice-display.h | 21 ++++++++++++++++----- 3 files changed, 54 insertions(+), 27 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index fe4212b..bd250db 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -343,18 +343,22 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) SimpleSpiceUpdate *update; QXLCommandRing *ring; QXLCommand *cmd; - int notify; + int notify, ret; switch (qxl->mode) { case QXL_MODE_VGA: dprint(qxl, 2, "%s: vga\n", __FUNCTION__); - update = qemu_spice_create_update(&qxl->ssd); - if (update == NULL) { - return false; + ret = false; + qemu_mutex_lock(&qxl->ssd.lock); + if (qxl->ssd.update != NULL) { + update = qxl->ssd.update; + qxl->ssd.update = NULL; + *ext = update->ext; + ret = true; } - *ext = update->ext; + qemu_mutex_unlock(&qxl->ssd.lock); qxl_log_command(qxl, "vga", ext); - return true; + return ret; case QXL_MODE_COMPAT: case QXL_MODE_NATIVE: case QXL_MODE_UNDEFINED: @@ -1304,6 +1308,7 @@ static int qxl_init_primary(PCIDevice *dev) vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate, qxl_hw_screen_dump, qxl_hw_text_update, qxl); qxl->ssd.ds = vga->ds; + qemu_mutex_init(&qxl->ssd.lock); qxl->ssd.bufsize = (16 * 1024 * 1024); qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize); diff --git a/ui/spice-display.c b/ui/spice-display.c index 020b423..d56dcfc 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -62,14 +62,7 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r) dest->right = MAX(dest->right, r->right); } -/* - * Called from spice server thread context (via interface_get_command). - * - * We must aquire the global qemu mutex here to make sure the - * DisplayState (+DisplaySurface) we are accessing doesn't change - * underneath us. - */ -SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) +static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) { SimpleSpiceUpdate *update; QXLDrawable *drawable; @@ -78,9 +71,7 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) uint8_t *src, *dst; int by, bw, bh; - qemu_mutex_lock_iothread(); if (qemu_spice_rect_is_empty(&ssd->dirty)) { - qemu_mutex_unlock_iothread(); return NULL; }; @@ -141,7 +132,6 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) cmd->data = (intptr_t)drawable; memset(&ssd->dirty, 0, sizeof(ssd->dirty)); - qemu_mutex_unlock_iothread(); return update; } @@ -241,6 +231,12 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd) qemu_pf_conv_put(ssd->conv); ssd->conv = NULL; + qemu_mutex_lock(&ssd->lock); + if (ssd->update != NULL) { + qemu_spice_destroy_update(ssd, ssd->update); + ssd->update = NULL; + } + qemu_mutex_unlock(&ssd->lock); qemu_spice_destroy_host_primary(ssd); qemu_spice_create_host_primary(ssd); @@ -252,6 +248,14 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) { dprint(3, "%s:\n", __FUNCTION__); vga_hw_update(); + + qemu_mutex_lock(&ssd->lock); + if (ssd->update == NULL) { + ssd->update = qemu_spice_create_update(ssd); + ssd->notify++; + } + qemu_mutex_unlock(&ssd->lock); + if (ssd->notify) { ssd->notify = 0; ssd->worker->wakeup(ssd->worker); @@ -298,14 +302,20 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) { SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl); SimpleSpiceUpdate *update; + int ret = false; dprint(3, "%s:\n", __FUNCTION__); - update = qemu_spice_create_update(ssd); - if (update == NULL) { - return false; + + qemu_mutex_lock(&ssd->lock); + if (ssd->update != NULL) { + update = ssd->update; + ssd->update = NULL; + *ext = update->ext; + ret = true; } - *ext = update->ext; - return true; + qemu_mutex_unlock(&ssd->lock); + + return ret; } static int interface_req_cmd_notification(QXLInstance *sin) @@ -398,6 +408,7 @@ void qemu_spice_display_init(DisplayState *ds) { assert(sdpy.ds == NULL); sdpy.ds = ds; + qemu_mutex_init(&sdpy.lock); sdpy.bufsize = (16 * 1024 * 1024); sdpy.buf = qemu_malloc(sdpy.bufsize); register_displaychangelistener(ds, &display_listener); diff --git a/ui/spice-display.h b/ui/spice-display.h index aef0464..e0cc46e 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -19,6 +19,7 @@ #include #include +#include "qemu-thread.h" #include "pflib.h" #define NUM_MEMSLOTS 8 @@ -31,7 +32,10 @@ #define NUM_SURFACES 1024 -typedef struct SimpleSpiceDisplay { +typedef struct SimpleSpiceDisplay SimpleSpiceDisplay; +typedef struct SimpleSpiceUpdate SimpleSpiceUpdate; + +struct SimpleSpiceDisplay { DisplayState *ds; void *buf; int bufsize; @@ -43,19 +47,26 @@ typedef struct SimpleSpiceDisplay { QXLRect dirty; int notify; int running; -} SimpleSpiceDisplay; -typedef struct SimpleSpiceUpdate { + /* + * All struct members below this comment can be accessed from + * both spice server and qemu (iothread) context and any access + * to them must be protected by the lock. + */ + QemuMutex lock; + SimpleSpiceUpdate *update; +}; + +struct SimpleSpiceUpdate { QXLDrawable drawable; QXLImage image; QXLCommandExt ext; uint8_t *bitmap; -} SimpleSpiceUpdate; +}; int qemu_spice_rect_is_empty(const QXLRect* r); void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r); -SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *sdpy); void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate *update); void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd); void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd);