On Tue, Mar 01, 2016 at 04:53:10PM +0100, Francois Gouget wrote:
> Signed-off-by: Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
> ---
>
> In theory this could be needed by the next patch.
>
> server/red-parse-qxl.h | 4 ++--
> server/red-worker.c | 7 ++++++-
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
> index 9c30572..220a096 100644
> --- a/server/red-parse-qxl.h
> +++ b/server/red-parse-qxl.h
> @@ -24,7 +24,7 @@
> #include "memslot.h"
>
> typedef struct RedDrawable {
> - int refs;
> + gint refs;
> QXLInstance *qxl;
> QXLReleaseInfoExt release_info_ext;
> uint32_t surface_id;
> @@ -60,7 +60,7 @@ typedef struct RedDrawable {
>
> static inline RedDrawable *red_drawable_ref(RedDrawable *drawable)
> {
> - drawable->refs++;
> + g_atomic_int_inc(&drawable->refs);
> return drawable;
> }
>
> diff --git a/server/red-worker.c b/server/red-worker.c
> index fd8eefb..f45cc3b 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -124,9 +124,14 @@ static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32
>
> void red_drawable_unref(RedDrawable *red_drawable)
> {
> - if (--red_drawable->refs) {
> + gint old_refs;
> + do {
> + old_refs = red_drawable->refs;
> + } while (!g_atomic_int_compare_and_exchange(&red_drawable->refs, old_refs, old_refs - 1));
> + if (old_refs > 1) {
> return;
> }
> +
> red_drawable->qxl->st->qif->release_resource(red_drawable->qxl,
> red_drawable->release_info_ext);
> red_put_drawable(red_drawable);
Making the refcounting thread safe is one thing, but then the code
freeing the drawable needs to be thread-safe too. red_put_drawable might
be fine (only looked briefly), but release_resource which is a vfunc
provided by QEMU seems much less obvious. I think the drawable refcount
is changed in the display channel thread, so I would try to queue an
idle on its main_context, and do the unref from there. Hopefully it will
not be too messy... (and I'll mention again
g_main_context_push_thread_default which could be useful there ;) or
some thread-local storage variable).
By "queueing an idle", I mean something like
GSource *source = g_idle_source_new();
g_source_set_callback(..);
g_source_attach(source, worker->core.main_context);
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel