>
> 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
>
No, Qemu must provide a thread safe release_resource.
Two reasons:
- release_resource is called from RedWorker which is a different
thread than Qemu or other users (it's just created inside spice-server);
- I looked at Qemu code and there is a clear comment on this ;-) .
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel