Re: [PATCH 09/15] worker: don't process drawable if it can't be allocated

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

 



From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>

---
 server/red_worker.c | 136 ++++++++++++++++++++++++++++------------------------
 1 file changed, 73 insertions(+), 63 deletions(-)

diff --git a/server/red_worker.c b/server/red_worker.c
index 9697532..5f1bbaf 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1179,7 +1179,9 @@ static void red_worker_drawable_unref(RedWorker *worker, Drawable *drawable)
         SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->drawable = NULL;
         ring_remove(item);
     }
-    put_red_drawable(worker, drawable->red_drawable, drawable->group_id);
+    if (drawable->red_drawable) {
+        put_red_drawable(worker, drawable->red_drawable, drawable->group_id);
+    }
     free_drawable(worker, drawable);
     worker->drawable_count--;
 }
@@ -3213,15 +3215,16 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
     return TRUE;
 }
 
-static void free_one_drawable(RedWorker *worker, int force_glz_free)
+static bool free_one_drawable(RedWorker *worker, int force_glz_free)
 {
     RingItem *ring_item = ring_get_tail(&worker->current_list);
     Drawable *drawable;
     Container *container;
 
     if (!ring_item) {
-        return;
+        return FALSE;
     }
+
     drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
     if (force_glz_free) {
         RingItem *glz_item, *next_item;
@@ -3235,47 +3238,32 @@ static void free_one_drawable(RedWorker *worker, int force_glz_free)
 
     current_remove_drawable(worker, drawable);
     container_cleanup(worker, container);
+    return TRUE;
 }
 
-static Drawable *get_drawable(RedWorker *worker, uint8_t effect, RedDrawable *red_drawable,
-                              uint32_t group_id)
+static Drawable *drawable_try_new(RedWorker *worker, int group_id)
 {
     Drawable *drawable;
-    int x;
-
-    VALIDATE_SURFACE_RETVAL(worker, red_drawable->surface_id, NULL)
-    if (!validate_drawable_bbox(worker, red_drawable)) {
-        rendering_incorrect(__func__);
-        return NULL;
-    }
-    for (x = 0; x < 3; ++x) {
-        if (red_drawable->surfaces_dest[x] != -1) {
-            VALIDATE_SURFACE_RETVAL(worker, red_drawable->surfaces_dest[x], NULL)
-        }
-    }
 
     while (!(drawable = alloc_drawable(worker))) {
-        free_one_drawable(worker, FALSE);
+        if (!free_one_drawable(worker, FALSE))
+            return NULL;
     }
-    worker->drawable_count++;
-    memset(drawable, 0, sizeof(Drawable));
+
+    bzero(drawable, sizeof(Drawable));
     drawable->refs = 1;
+    worker->drawable_count++;
     drawable->creation_time = red_get_monotonic_time();
     ring_item_init(&drawable->list_link);
     ring_item_init(&drawable->surface_list_link);
     ring_item_init(&drawable->tree_item.base.siblings_link);
     drawable->tree_item.base.type = TREE_ITEM_TYPE_DRAWABLE;
     region_init(&drawable->tree_item.base.rgn);
-    drawable->tree_item.effect = effect;
-    drawable->red_drawable = ref_red_drawable(red_drawable);
-    drawable->group_id = group_id;
-
-    drawable->surface_id = red_drawable->surface_id;
-    memcpy(drawable->surfaces_dest, red_drawable->surfaces_dest, sizeof(drawable->surfaces_dest));
     ring_init(&drawable->pipes);
     ring_init(&drawable->glz_ring);
-
     drawable->process_commands_generation = worker->process_commands_generation;
+    drawable->group_id = group_id;
+
     return drawable;
 }
 
@@ -3351,24 +3339,41 @@ static inline void red_inc_surfaces_drawable_dependencies(RedWorker *worker, Dra
     }
 }
 
-static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable,
-                                    uint32_t group_id)
+static RedDrawable *red_drawable_new(RedWorker *worker)
 {
-    int surface_id;
-    Drawable *drawable = get_drawable(worker, red_drawable->effect, red_drawable, group_id);
+    RedDrawable * red = spice_new0(RedDrawable, 1);
 
-    if (!drawable) {
-        rendering_incorrect("failed to get_drawable");
-        return;
-    }
+    red->refs = 1;
+    worker->red_drawable_count++;
 
-    red_drawable->mm_time = reds_get_mm_time();
-    surface_id = drawable->surface_id;
+    return red;
+}
 
-    worker->surfaces[surface_id].refs++;
+static gboolean red_process_draw(RedWorker *worker, QXLCommandExt *ext_cmd)
+{
+    RedDrawable *red_drawable = NULL;
+    Drawable *drawable = NULL;
+    int surface_id, x;
+    gboolean success = FALSE;
 
-    region_add(&drawable->tree_item.base.rgn, &red_drawable->bbox);
+    drawable = drawable_try_new(worker, ext_cmd->group_id);
+    if (!drawable)
+        goto end;
+
+    red_drawable = red_drawable_new(worker);
+    if (red_get_drawable(&worker->mem_slots, ext_cmd->group_id,
+                         red_drawable, ext_cmd->cmd.data, ext_cmd->flags) != 0)
+        goto end;
+
+    if (!validate_drawable_bbox(worker, red_drawable)) {
+        rendering_incorrect(__func__);
+        goto end;
+    }
 
+    drawable->tree_item.effect = red_drawable->effect;
+    drawable->red_drawable = ref_red_drawable(red_drawable);
+    drawable->surface_id = red_drawable->surface_id;
+    region_add(&drawable->tree_item.base.rgn, &red_drawable->bbox);
     if (red_drawable->clip.type == SPICE_CLIP_TYPE_RECTS) {
         QRegion rgn;
 
@@ -3377,6 +3382,20 @@ static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable
         region_and(&drawable->tree_item.base.rgn, &rgn);
         region_destroy(&rgn);
     }
+
+    if (!validate_surface(worker, drawable->surface_id))
+        goto end;
+    for (x = 0; x < 3; ++x) {
+        drawable->surfaces_dest[x] = red_drawable->surfaces_dest[x];
+        if (drawable->surfaces_dest[x] != -1
+            && !validate_surface(worker, drawable->surfaces_dest[x]))
+            goto end;
+    }
+
+    red_drawable->mm_time = reds_get_mm_time();
+    surface_id = drawable->surface_id;
+    worker->surfaces[surface_id].refs++;
+
     /*
         surface->refs is affected by a drawable (that is
         dependent on the surface) as long as the drawable is alive.
@@ -3386,19 +3405,19 @@ static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable
     red_inc_surfaces_drawable_dependencies(worker, drawable);
 
     if (region_is_empty(&drawable->tree_item.base.rgn)) {
-        goto cleanup;
+        goto end;
     }
 
     if (!red_handle_self_bitmap(worker, drawable)) {
-        goto cleanup;
+        goto end;
     }
 
     if (!red_handle_depends_on_target_surface(worker, surface_id)) {
-        goto cleanup;
+        goto end;
     }
 
     if (!red_handle_surfaces_dependencies(worker, drawable)) {
-        goto cleanup;
+        goto end;
     }
 
     if (red_current_add_qxl(worker, &worker->surfaces[surface_id].current, drawable,
@@ -3408,8 +3427,15 @@ static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable
         }
         red_pipes_add_drawable(worker, drawable);
     }
-cleanup:
-    red_worker_drawable_unref(worker, drawable);
+
+    success = TRUE;
+
+end:
+    if (drawable != NULL)
+        red_worker_drawable_unref(worker, drawable);
+    if (red_drawable != NULL)
+        put_red_drawable(worker, red_drawable, ext_cmd->group_id);
+    return success;
 }
 
 static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uint32_t width,
@@ -3900,16 +3926,6 @@ static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size, int *ri
     return n;
 }
 
-static RedDrawable *red_drawable_new(RedWorker *worker)
-{
-    RedDrawable * red = spice_new0(RedDrawable, 1);
-
-    red->refs = 1;
-    worker->red_drawable_count++;
-
-    return red;
-}
-
 static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *ring_is_empty)
 {
     QXLCommandExt ext_cmd;
@@ -3949,14 +3965,8 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
         worker->display_poll_tries = 0;
         switch (ext_cmd.cmd.type) {
         case QXL_CMD_DRAW: {
-            RedDrawable *red_drawable = red_drawable_new(worker); // returns with 1 ref
-
-            if (!red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
-                                 red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
-                red_process_draw(worker, red_drawable, ext_cmd.group_id);
-            }
-            // release the red_drawable
-            put_red_drawable(worker, red_drawable, ext_cmd.group_id);
+            if (!red_process_draw(worker, &ext_cmd))
+                break;
             break;
         }
         case QXL_CMD_UPDATE: {
-- 
2.4.3
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://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]