[Spice-devel] [spice-server v2 5/6] Make RedCharDeviceWriteBuffer::refs private

[Christophe Fergeau]

On Thu, May 11, 2017 at 11:24:28AM +0200, Christophe de Dinechin wrote:
> > 
> > On 28 Apr 2017, at 11:11, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> > ---
> > server/char-device.c | 11 ++++++-----
> > server/char-device.h |  1 -
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/server/char-device.c b/server/char-device.c
> > index 7e77b465b..481ea2a13 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -36,6 +36,7 @@ struct RedCharDeviceWriteBufferPrivate {
> >     RedClient *client; /* The client that sent the message to the device.
> >                           NULL if the server created the message */
> >     uint32_t token_price;
> > +    uint32_t refs;
> > };
> > 
> > typedef struct RedCharDeviceClient RedCharDeviceClient;
> > @@ -165,7 +166,7 @@ static void write_buffers_queue_free(GQueue *write_queue)
> > static void red_char_device_write_buffer_pool_add(RedCharDevice *dev,
> >                                                   RedCharDeviceWriteBuffer *buf)
> > {
> > -    if (buf->refs == 1 &&
> > +    if (buf->priv->refs == 1 &&
> >         dev->priv->cur_pool_size < MAX_POOL_SIZE) {
> >         buf->buf_used = 0;
> >         buf->priv->origin = WRITE_BUFFER_ORIGIN_NONE;
> > @@ -584,7 +585,7 @@ static RedCharDeviceWriteBuffer *__red_char_device_write_buffer_get(
> >     }
> > 
> >     ret->priv->token_price = migrated_data_tokens ? migrated_data_tokens : 1;
> > -    ret->refs = 1;
> > +    ret->priv->refs = 1;
> >     return ret;
> > error:
> >     dev->priv->cur_pool_size += ret->buf_size;
> > @@ -612,7 +613,7 @@ static RedCharDeviceWriteBuffer *red_char_device_write_buffer_ref(RedCharDeviceW
> > {
> >     spice_assert(write_buf);
> > 
> > -    write_buf->refs++;
> > +    write_buf->priv->refs++;
> 
> Increasing / decreasing refs is done in a non-atomic way. I assume
> this means that we know we cannot enter
> red_char_device_write_buffer_ref from different threads simultaneously
> for the same buffer. Just for my education, could someone explain why?
> My own mental model is that marshalling is a very sequential operation
> anyway, so it’s hard to think of a valid scenario where we’d
> parallelize it. But that makes me wonder if there is some kind of
> convention allowing us to easily identify that structures that are
> possibly shared between threads.

There are few threads being used, roughly most of the spice code runs in
the main QEMU thread, except for the display channel code (RedWorker),
each display channel has its own thread. The main thread and the worker
thread communicate through messages (dispatcher or reddispatcher, I
always mix these 2). There should be very few data structures which are
going to be shared between threads.

Christophe

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170511/39995242/attachment-0001.sig>