> 
> On 28 Apr 2017, at 11:11, Christophe Fergeau <cferg...@redhat.com> wrote:
> 
> Signed-off-by: Christophe Fergeau <cferg...@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.
 

>     return write_buf;
> }
> 
> @@ -620,8 +621,8 @@ static void 
> red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer *write_b
> {
>     spice_assert(write_buf);
> 
> -    write_buf->refs--;
> -    if (write_buf->refs == 0)
> +    write_buf->priv->refs--;
> +    if (write_buf->priv->refs == 0)
>         red_char_device_write_buffer_free(write_buf);
> }
> 
> diff --git a/server/char-device.h b/server/char-device.h
> index 13e625fe1..f66e2a158 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -151,7 +151,6 @@ typedef struct RedCharDeviceWriteBuffer {
>     uint8_t *buf;
>     uint32_t buf_size;
>     uint32_t buf_used;
> -    uint32_t refs;
> 
>     RedCharDeviceWriteBufferPrivate *priv;
> } RedCharDeviceWriteBuffer;
> -- 
> 2.12.2
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to