Re: [PATCH 09/10] Change RedCharDevice write_queue to GList

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The use of g_list_last() in this patch makes me think a GQueue would be
more appropriate.

Christophe

On Thu, Sep 08, 2016 at 11:52:59AM -0500, Jonathon Jongsma wrote:
> Change a couple more Rings to GList
> ---
>  server/char-device.c | 88 ++++++++++++++++++++++------------------------------
>  server/char-device.h |  1 -
>  2 files changed, 37 insertions(+), 52 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 957fb26..e01f34c 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -50,8 +50,8 @@ struct RedCharDevicePrivate {
>      int active; /* has read/write been performed since the device was started */
>      int wait_for_migrate_data;
>  
> -    Ring write_queue;
> -    Ring write_bufs_pool;
> +    GList *write_queue;
> +    GList *write_bufs_pool;
>      uint64_t cur_pool_size;
>      RedCharDeviceWriteBuffer *cur_write_buf;
>      uint8_t *cur_write_buf_pos;
> @@ -150,18 +150,6 @@ static void red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
>      free(buf);
>  }
>  
> -static void write_buffers_queue_free(Ring *write_queue)
> -{
> -    while (!ring_is_empty(write_queue)) {
> -        RingItem *item = ring_get_tail(write_queue);
> -        RedCharDeviceWriteBuffer *buf;
> -
> -        ring_remove(item);
> -        buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
> -        red_char_device_write_buffer_free(buf);
> -    }
> -}
> -
>  static void red_char_device_write_buffer_pool_add(RedCharDevice *dev,
>                                                    RedCharDeviceWriteBuffer *buf)
>  {
> @@ -171,7 +159,7 @@ static void red_char_device_write_buffer_pool_add(RedCharDevice *dev,
>          buf->origin = WRITE_BUFFER_ORIGIN_NONE;
>          buf->client = NULL;
>          dev->priv->cur_pool_size += buf->buf_size;
> -        ring_add(&dev->priv->write_bufs_pool, &buf->link);
> +        dev->priv->write_bufs_pool = g_list_prepend(dev->priv->write_bufs_pool, buf);
>          return;
>      }
>  
> @@ -182,7 +170,7 @@ static void red_char_device_write_buffer_pool_add(RedCharDevice *dev,
>  static void red_char_device_client_free(RedCharDevice *dev,
>                                          RedCharDeviceClient *dev_client)
>  {
> -    RingItem *item, *next;
> +    GList *l, *next;
>  
>      if (dev_client->wait_for_tokens_timer) {
>          reds_core_timer_remove(dev->priv->reds, dev_client->wait_for_tokens_timer);
> @@ -192,16 +180,18 @@ static void red_char_device_client_free(RedCharDevice *dev,
>      g_queue_free_full(dev_client->send_queue, (GDestroyNotify)red_pipe_item_unref);
>  
>      /* remove write buffers that are associated with the client */
> -    spice_debug("write_queue_is_empty %d", ring_is_empty(&dev->priv->write_queue) && !dev->priv->cur_write_buf);
> -    RING_FOREACH_SAFE(item, next, &dev->priv->write_queue) {
> -        RedCharDeviceWriteBuffer *write_buf;
> +    spice_debug("write_queue_is_empty %d", (dev->priv->write_queue == NULL) && !dev->priv->cur_write_buf);
> +    l = dev->priv->write_queue;
> +    while (l) {
> +        RedCharDeviceWriteBuffer *write_buf = l->data;
> +        next = l->next;
>  
> -        write_buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
>          if (write_buf->origin == WRITE_BUFFER_ORIGIN_CLIENT &&
>              write_buf->client == dev_client->client) {
> -            ring_remove(item);
> +            dev->priv->write_queue = g_list_delete_link(dev->priv->write_queue, l);
>              red_char_device_write_buffer_pool_add(dev, write_buf);
>          }
> +        l = next;
>      }
>  
>      if (dev->priv->cur_write_buf && dev->priv->cur_write_buf->origin == WRITE_BUFFER_ORIGIN_CLIENT &&
> @@ -483,13 +473,13 @@ static int red_char_device_write_to_device(RedCharDevice *dev)
>          uint32_t write_len;
>  
>          if (!dev->priv->cur_write_buf) {
> -            RingItem *item = ring_get_tail(&dev->priv->write_queue);
> +            GList *item = g_list_last(dev->priv->write_queue);
>              if (!item) {
>                  break;
>              }
> -            dev->priv->cur_write_buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
> +            dev->priv->cur_write_buf = item->data;
>              dev->priv->cur_write_buf_pos = dev->priv->cur_write_buf->buf;
> -            ring_remove(item);
> +            dev->priv->write_queue = g_list_delete_link(dev->priv->write_queue, item);
>          }
>  
>          write_len = dev->priv->cur_write_buf->buf + dev->priv->cur_write_buf->buf_used -
> @@ -519,7 +509,7 @@ static int red_char_device_write_to_device(RedCharDevice *dev)
>                                        CHAR_DEVICE_WRITE_TO_TIMEOUT);
>              }
>          } else {
> -            spice_assert(ring_is_empty(&dev->priv->write_queue));
> +            spice_assert(dev->priv->write_queue == NULL);
>          }
>          dev->priv->active = dev->priv->active || total;
>      }
> @@ -542,16 +532,16 @@ static RedCharDeviceWriteBuffer *__red_char_device_write_buffer_get(
>      RedCharDevice *dev, RedClient *client,
>      int size, int origin, int migrated_data_tokens)
>  {
> -    RingItem *item;
> +    GList *item;
>      RedCharDeviceWriteBuffer *ret;
>  
>      if (origin == WRITE_BUFFER_ORIGIN_SERVER && !dev->priv->num_self_tokens) {
>          return NULL;
>      }
>  
> -    if ((item = ring_get_tail(&dev->priv->write_bufs_pool))) {
> -        ret = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
> -        ring_remove(item);
> +    if ((item = g_list_last(dev->priv->write_bufs_pool))) {
> +        ret = item->data;
> +        dev->priv->write_bufs_pool = g_list_delete_link(dev->priv->write_bufs_pool, item);
>          dev->priv->cur_pool_size -= ret->buf_size;
>      } else {
>          ret = spice_new0(RedCharDeviceWriteBuffer, 1);
> @@ -594,7 +584,7 @@ static RedCharDeviceWriteBuffer *__red_char_device_write_buffer_get(
>      return ret;
>  error:
>      dev->priv->cur_pool_size += ret->buf_size;
> -    ring_add(&dev->priv->write_bufs_pool, &ret->link);
> +    dev->priv->write_bufs_pool = g_list_prepend(dev->priv->write_bufs_pool, ret);
>      return NULL;
>  }
>  
> @@ -643,7 +633,7 @@ void red_char_device_write_buffer_add(RedCharDevice *dev,
>          return;
>      }
>  
> -    ring_add(&dev->priv->write_queue, &write_buf->link);
> +    dev->priv->write_queue = g_list_prepend(dev->priv->write_queue, write_buf);
>      red_char_device_write_to_device(dev);
>  }
>  
> @@ -660,7 +650,6 @@ void red_char_device_write_buffer_release(RedCharDevice *dev,
>      uint32_t buf_token_price = write_buf->token_price;
>      RedClient *client = write_buf->client;
>  
> -    spice_assert(!ring_item_is_linked(&write_buf->link));
>      if (!dev) {
>          spice_printerr("no device. write buffer is freed");
>          red_char_device_write_buffer_free(write_buf);
> @@ -794,7 +783,9 @@ void red_char_device_client_remove(RedCharDevice *dev,
>  
>      if (dev->priv->num_clients == 0) {
>          spice_debug("client removed, memory pool will be freed (%"PRIu64" bytes)", dev->priv->cur_pool_size);
> -        write_buffers_queue_free(&dev->priv->write_bufs_pool);
> +        g_list_free_full(dev->priv->write_bufs_pool,
> +                         (GDestroyNotify) red_char_device_write_buffer_free);
> +        dev->priv->write_bufs_pool = NULL;
>          dev->priv->cur_pool_size = 0;
>      }
>  }
> @@ -832,15 +823,9 @@ void red_char_device_reset(RedCharDevice *dev)
>      red_char_device_stop(dev);
>      dev->priv->wait_for_migrate_data = FALSE;
>      spice_debug("char device %p", dev);
> -    while (!ring_is_empty(&dev->priv->write_queue)) {
> -        RingItem *item = ring_get_tail(&dev->priv->write_queue);
> -        RedCharDeviceWriteBuffer *buf;
> -
> -        ring_remove(item);
> -        buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
> -        /* tracking the tokens */
> -        red_char_device_write_buffer_release(dev, &buf);
> -    }
> +    g_list_free_full(dev->priv->write_queue,
> +                     (GDestroyNotify) red_char_device_write_buffer_release);
> +    dev->priv->write_queue = NULL;
>      red_char_device_write_buffer_release(dev, &dev->priv->cur_write_buf);
>  
>      RING_FOREACH(client_item, &dev->priv->clients) {
> @@ -894,7 +879,7 @@ void red_char_device_migrate_data_marshall(RedCharDevice *dev,
>                                             SpiceMarshaller *m)
>  {
>      RedCharDeviceClient *dev_client;
> -    RingItem *item;
> +    GList *item;
>      uint32_t *write_to_dev_size_ptr;
>      uint32_t *write_to_dev_tokens_ptr;
>      SpiceMarshaller *m2;
> @@ -932,10 +917,9 @@ void red_char_device_migrate_data_marshall(RedCharDevice *dev,
>          }
>      }
>  
> -    RING_FOREACH_REVERSED(item, &dev->priv->write_queue) {
> -        RedCharDeviceWriteBuffer *write_buf;
> +    for (item = g_list_last(dev->priv->write_queue); item != NULL; item = item->prev) {
> +        RedCharDeviceWriteBuffer *write_buf = item->data;
>  
> -        write_buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
>          spice_marshaller_add_ref_full(m2, write_buf->buf, write_buf->buf_used,
>                                        migrate_data_marshaller_write_buffer_free,
>                                        red_char_device_write_buffer_ref(write_buf)
> @@ -966,7 +950,7 @@ int red_char_device_restore(RedCharDevice *dev,
>                      dev, mig_data->version, SPICE_MIGRATE_DATA_CHAR_DEVICE_VERSION);
>          return FALSE;
>      }
> -    spice_assert(!dev->priv->cur_write_buf && ring_is_empty(&dev->priv->write_queue));
> +    spice_assert(!dev->priv->cur_write_buf && (dev->priv->write_queue = NULL));
>      spice_assert(mig_data->connected);
>  
>      client_tokens_window = dev_client->num_client_tokens; /* initial state of tokens */
> @@ -1119,8 +1103,12 @@ red_char_device_finalize(GObject *object)
>          reds_core_timer_remove(self->priv->reds, self->priv->write_to_dev_timer);
>          self->priv->write_to_dev_timer = NULL;
>      }
> -    write_buffers_queue_free(&self->priv->write_queue);
> -    write_buffers_queue_free(&self->priv->write_bufs_pool);
> +    g_list_free_full(self->priv->write_queue,
> +                     (GDestroyNotify) red_char_device_write_buffer_free);
> +    self->priv->write_queue = NULL;
> +    g_list_free_full(self->priv->write_bufs_pool,
> +                     (GDestroyNotify) red_char_device_write_buffer_free);
> +    self->priv->write_bufs_pool = NULL;
>      self->priv->cur_pool_size = 0;
>      red_char_device_write_buffer_free(self->priv->cur_write_buf);
>      self->priv->cur_write_buf = NULL;
> @@ -1195,8 +1183,6 @@ red_char_device_init(RedCharDevice *self)
>  {
>      self->priv = RED_CHAR_DEVICE_PRIVATE(self);
>  
> -    ring_init(&self->priv->write_queue);
> -    ring_init(&self->priv->write_bufs_pool);
>      ring_init(&self->priv->clients);
>  
>      g_signal_connect(self, "notify::sin", G_CALLBACK(red_char_device_on_sin_changed), NULL);
> diff --git a/server/char-device.h b/server/char-device.h
> index 1ada763..44e9504 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -144,7 +144,6 @@ GType red_char_device_get_type(void) G_GNUC_CONST;
>  
>  /* buffer that is used for writing to the device */
>  typedef struct RedCharDeviceWriteBuffer {
> -    RingItem link;
>      int origin;
>      RedClient *client; /* The client that sent the message to the device.
>                            NULL if the server created the message */
> -- 
> 2.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]