[Spice-commits] 3 commits - server/char_device.c

Victor Toso de Carvalho victortoso at kemper.freedesktop.org
Thu Nov 12 23:16:40 PST 2015


 server/char_device.c |   49 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)

New commits:
commit a263c651e18266c2f05ca9cfff8da886c1603723
Author: Victor Toso <victortoso at redhat.com>
Date:   Wed Aug 19 12:05:46 2015 +0200

    char-device: free all memory pool when no clients
    
    When no client is connect we should not need to keep the memory pool
    used by char-device. In most situations this is not significant but
    when using webdav this could mean freeing MAX_POOL_SIZE bytes
    
    Related: https://bugs.freedesktop.org/show_bug.cgi?id=91350

diff --git a/server/char_device.c b/server/char_device.c
index 3aa88ef..fe38385 100644
--- a/server/char_device.c
+++ b/server/char_device.c
@@ -826,6 +826,12 @@ void spice_char_device_client_remove(SpiceCharDeviceState *dev,
         dev->wait_for_migrate_data  = FALSE;
         spice_char_device_read_from_device(dev);
     }
+
+    if (dev->num_clients == 0) {
+        spice_debug("client removed, memory pool will be freed (%lu bytes)", dev->cur_pool_size);
+        write_buffers_queue_free(&dev->write_bufs_pool);
+        dev->cur_pool_size = 0;
+    }
 }
 
 int spice_char_device_client_exists(SpiceCharDeviceState *dev,
commit 2832fdf25a9a2097efc58545eb389342393562fc
Author: Victor Toso <victortoso at redhat.com>
Date:   Wed Aug 19 11:26:34 2015 +0200

    char-device: Define a memory pool limit
    
    Otherwise the amount of unused memory could grow while transfering big
    chunks of data. This change only means that once the memory was used it
    will not be stored again after the limit was reached.
    
    Related: https://bugs.freedesktop.org/show_bug.cgi?id=91350

diff --git a/server/char_device.c b/server/char_device.c
index 4381981..3aa88ef 100644
--- a/server/char_device.c
+++ b/server/char_device.c
@@ -27,6 +27,7 @@
 
 #define CHAR_DEVICE_WRITE_TO_TIMEOUT 100
 #define SPICE_CHAR_DEVICE_WAIT_TOKENS_TIMEOUT 30000
+#define MAX_POOL_SIZE (10 * 64 * 1024)
 
 typedef struct SpiceCharDeviceClientState SpiceCharDeviceClientState;
 struct SpiceCharDeviceClientState {
@@ -52,6 +53,7 @@ struct SpiceCharDeviceState {
 
     Ring write_queue;
     Ring write_bufs_pool;
+    uint64_t cur_pool_size;
     SpiceCharDeviceWriteBuffer *cur_write_buf;
     uint8_t *cur_write_buf_pos;
     SpiceTimer *write_to_dev_timer;
@@ -114,10 +116,12 @@ static void write_buffers_queue_free(Ring *write_queue)
 static void spice_char_device_write_buffer_pool_add(SpiceCharDeviceState *dev,
                                                     SpiceCharDeviceWriteBuffer *buf)
 {
-    if (buf->refs == 1) {
+    if (buf->refs == 1 &&
+        dev->cur_pool_size < MAX_POOL_SIZE) {
         buf->buf_used = 0;
         buf->origin = WRITE_BUFFER_ORIGIN_NONE;
         buf->client = NULL;
+        dev->cur_pool_size += buf->buf_size;
         ring_add(&dev->write_bufs_pool, &buf->link);
         return;
     }
@@ -529,6 +533,7 @@ static SpiceCharDeviceWriteBuffer *__spice_char_device_write_buffer_get(
     if ((item = ring_get_tail(&dev->write_bufs_pool))) {
         ret = SPICE_CONTAINEROF(item, SpiceCharDeviceWriteBuffer, link);
         ring_remove(item);
+        dev->cur_pool_size -= ret->buf_size;
     } else {
         ret = spice_new0(SpiceCharDeviceWriteBuffer, 1);
     }
@@ -569,6 +574,7 @@ static SpiceCharDeviceWriteBuffer *__spice_char_device_write_buffer_get(
     ret->refs = 1;
     return ret;
 error:
+    dev->cur_pool_size += ret->buf_size;
     ring_add(&dev->write_bufs_pool, &ret->link);
     return NULL;
 }
@@ -739,6 +745,7 @@ void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
     }
     write_buffers_queue_free(&char_dev->write_queue);
     write_buffers_queue_free(&char_dev->write_bufs_pool);
+    char_dev->cur_pool_size = 0;
     spice_char_device_write_buffer_free(char_dev->cur_write_buf);
 
     while (!ring_is_empty(&char_dev->clients)) {
commit d7bee1bc56e2d3ea6af399ba8479cb4b849d4b15
Author: Victor Toso <victortoso at redhat.com>
Date:   Wed Aug 19 10:53:22 2015 +0200

    char-device: fix usage of free/unref on WriteBuffer
    
    There are places were the could should definetly free the
    SpiceCharDeviceWriteBuffer and places that it should only unref it. The
    current use of spice_char_device_write_buffer_free was missleading.
    
    This patch creates the spice_char_device_write_buffer_unref and properly
    call these two functions.
    
    Related: https://bugs.freedesktop.org/show_bug.cgi?id=91350

diff --git a/server/char_device.c b/server/char_device.c
index 54357f0..4381981 100644
--- a/server/char_device.c
+++ b/server/char_device.c
@@ -81,6 +81,7 @@ enum {
  * destroyed during a callback */
 static void spice_char_device_state_ref(SpiceCharDeviceState *char_dev);
 static void spice_char_device_state_unref(SpiceCharDeviceState *char_dev);
+static void spice_char_device_write_buffer_unref(SpiceCharDeviceWriteBuffer *write_buf);
 
 static void spice_char_dev_write_retry(void *opaque);
 
@@ -91,10 +92,11 @@ typedef struct SpiceCharDeviceMsgToClientItem {
 
 static void spice_char_device_write_buffer_free(SpiceCharDeviceWriteBuffer *buf)
 {
-    if (--buf->refs == 0) {
-        free(buf->buf);
-        free(buf);
-    }
+    if (buf == NULL)
+        return;
+
+    free(buf->buf);
+    free(buf);
 }
 
 static void write_buffers_queue_free(Ring *write_queue)
@@ -117,9 +119,11 @@ static void spice_char_device_write_buffer_pool_add(SpiceCharDeviceState *dev,
         buf->origin = WRITE_BUFFER_ORIGIN_NONE;
         buf->client = NULL;
         ring_add(&dev->write_bufs_pool, &buf->link);
-    } else {
-        --buf->refs;
+        return;
     }
+
+    /* Buffer still being used - just unref for the caller */
+    spice_char_device_write_buffer_unref(buf);
 }
 
 static void spice_char_device_client_send_queue_free(SpiceCharDeviceState *dev,
@@ -593,6 +597,15 @@ static SpiceCharDeviceWriteBuffer *spice_char_device_write_buffer_ref(SpiceCharD
     return write_buf;
 }
 
+static void spice_char_device_write_buffer_unref(SpiceCharDeviceWriteBuffer *write_buf)
+{
+    spice_assert(write_buf);
+
+    write_buf->refs--;
+    if (write_buf->refs == 0)
+        spice_char_device_write_buffer_free(write_buf);
+}
+
 void spice_char_device_write_buffer_add(SpiceCharDeviceState *dev,
                                         SpiceCharDeviceWriteBuffer *write_buf)
 {
@@ -619,8 +632,7 @@ void spice_char_device_write_buffer_release(SpiceCharDeviceState *dev,
     spice_assert(!ring_item_is_linked(&write_buf->link));
     if (!dev) {
         spice_printerr("no device. write buffer is freed");
-        free(write_buf->buf);
-        free(write_buf);
+        spice_char_device_write_buffer_free(write_buf);
         return;
     }
 
@@ -727,9 +739,7 @@ void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
     }
     write_buffers_queue_free(&char_dev->write_queue);
     write_buffers_queue_free(&char_dev->write_bufs_pool);
-    if (char_dev->cur_write_buf) {
-        spice_char_device_write_buffer_free(char_dev->cur_write_buf);
-    }
+    spice_char_device_write_buffer_free(char_dev->cur_write_buf);
 
     while (!ring_is_empty(&char_dev->clients)) {
         RingItem *item = ring_get_tail(&char_dev->clients);
@@ -895,7 +905,7 @@ static void migrate_data_marshaller_write_buffer_free(uint8_t *data, void *opaqu
 {
     SpiceCharDeviceWriteBuffer *write_buf = (SpiceCharDeviceWriteBuffer *)opaque;
 
-    spice_char_device_write_buffer_free(write_buf);
+    spice_char_device_write_buffer_unref(write_buf);
 }
 
 void spice_char_device_state_migrate_data_marshall(SpiceCharDeviceState *dev,


More information about the Spice-commits mailing list