Re: [PATCH 15/18] worker: move display_channel_free_some

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

 



On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>
> Author:    Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> ---
>  server/dcc-encoders.c    |  25 +++++++++
>  server/dcc-encoders.h    |   3 +-
>  server/display-channel.c | 108 ++++++++++++++++++++++++++++++++++++++
>  server/display-channel.h |   6 +++
>  server/red_worker.c      | 134 +++--------------------------------------------
>  5 files changed, 147 insertions(+), 129 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 4d8eab1..ea975e7 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -506,6 +506,31 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable)
>      }
>  }
>
> +/*
> + * Remove from the global lz dictionary some glz_drawables that have no reference to
> + * Drawable (their qxl drawables are released too).
> + * NOTE - the caller should prevent encoding using the dictionary during the operation
> + */
> +int dcc_free_some_independent_glz_drawables(DisplayChannelClient *dcc)
> +{
> +    RingItem *ring_link;
> +    int n = 0;
> +
> +    if (!dcc) {
> +        return 0;
> +    }
> +    ring_link = ring_get_head(&dcc->glz_drawables);
> +    while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
> +        RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
> +        ring_link = ring_next(&dcc->glz_drawables, ring_link);
> +        if (!glz_drawable->drawable) {
> +            dcc_free_glz_drawable(dcc, glz_drawable);
> +            n++;
> +        }
> +    }
> +    return n;
> +}
> +
>  void dcc_free_glz_drawables_to_free(DisplayChannelClient* dcc)
>  {
>      RingItem *ring_link;
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 231d37f..b0306e8 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -36,12 +36,12 @@ typedef struct RedCompressBuf RedCompressBuf;
>  typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
>  typedef struct RedGlzDrawable RedGlzDrawable;
>
> -
>  void             dcc_encoders_init                           (DisplayChannelClient *dcc);
>  void             dcc_free_glz_drawable_instance              (DisplayChannelClient *dcc,
>                                                                GlzDrawableInstanceItem *item);
>  void             dcc_free_glz_drawable                       (DisplayChannelClient *dcc,
>                                                                RedGlzDrawable *drawable);
> +int              dcc_free_some_independent_glz_drawables     (DisplayChannelClient *dcc);
>  void             dcc_free_glz_drawables                      (DisplayChannelClient *dcc);
>  void             dcc_free_glz_drawables_to_free              (DisplayChannelClient* dcc);
>  void             dcc_freeze_glz                              (DisplayChannelClient *dcc);
> @@ -163,5 +163,6 @@ struct RedGlzDrawable {
>      DisplayChannelClient *dcc;
>  };
>
> +#define RED_RELEASE_BUNCH_SIZE 64
>
>  #endif /* DCC_ENCODERS_H_ */
> diff --git a/server/display-channel.c b/server/display-channel.c
> index ecf6f7d..affd2dd 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -918,3 +918,111 @@ void display_channel_free_glz_drawables(DisplayChannel *display)
>          dcc_free_glz_drawables(dcc);
>      }
>  }
> +
> +static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
> +{
> +    RingItem *ring_item = ring_get_tail(&display->current_list);
> +    Drawable *drawable;
> +    Container *container;
> +
> +    if (!ring_item) {
> +        return FALSE;
> +    }
> +
> +    drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
> +    if (force_glz_free) {
> +        RingItem *glz_item, *next_item;
> +        RedGlzDrawable *glz;
> +        DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> +            dcc_free_glz_drawable(glz->dcc, glz);
> +        }
> +    }
> +    drawable_draw(display, drawable);
> +    container = drawable->tree_item.base.container;
> +
> +    current_remove_drawable(display, drawable);
> +    container_cleanup(container);
> +    return TRUE;
> +}
> +
> +void display_channel_current_flush(DisplayChannel *display, int surface_id)
> +{
> +    while (!ring_is_empty(&display->surfaces[surface_id].current_list)) {
> +        free_one_drawable(display, FALSE);
> +    }
> +    current_remove_all(display, surface_id);
> +}
> +
> +void display_channel_free_some(DisplayChannel *display)
> +{
> +    int n = 0;
> +    DisplayChannelClient *dcc;
> +    RingItem *item, *next;
> +
> +#if FIXME
> +    spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d", display->drawable_count,
> +                worker->red_drawable_count, worker->glz_drawable_count);
> +#endif
> +    FOREACH_DCC(display, item, next, dcc) {
> +        GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> +
> +        if (glz_dict) {
> +            // encoding using the dictionary is prevented since the following operations might
> +            // change the dictionary
> +            pthread_rwlock_wrlock(&glz_dict->encode_lock);
> +            n = dcc_free_some_independent_glz_drawables(dcc);
> +        }
> +    }
> +
> +    while (!ring_is_empty(&display->current_list) && n++ < RED_RELEASE_BUNCH_SIZE) {
> +        free_one_drawable(display, TRUE);
> +    }
> +
> +    FOREACH_DCC(display, item, next, dcc) {
> +        GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> +
> +        if (glz_dict) {
> +            pthread_rwlock_unlock(&glz_dict->encode_lock);
> +        }
> +    }
> +}
> +
> +static Drawable* drawable_try_new(DisplayChannel *display)
> +{
> +    Drawable *drawable;
> +
> +    if (!display->free_drawables)
> +        return NULL;
> +
> +    drawable = &display->free_drawables->u.drawable;
> +    display->free_drawables = display->free_drawables->u.next;
> +    display->drawable_count++;
> +
> +    return drawable;
> +}
> +
> +Drawable *display_channel_drawable_try_new(DisplayChannel *display,
> +                                           int group_id, int process_commands_generation)
> +{
> +    Drawable *drawable;
> +
> +    while (!(drawable = drawable_try_new(display))) {
> +        if (!free_one_drawable(display, FALSE))
> +            return NULL;
> +    }
> +
> +    bzero(drawable, sizeof(Drawable));
> +    drawable->refs = 1;
> +    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);
> +    ring_init(&drawable->pipes);
> +    ring_init(&drawable->glz_ring);
> +    drawable->process_commands_generation = process_commands_generation;
> +    drawable->group_id = group_id;
> +
> +    return drawable;
> +}
> diff --git a/server/display-channel.h b/server/display-channel.h
> index b49cec9..f6f7878 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -250,11 +250,15 @@ typedef struct UpgradeItem {
>  } UpgradeItem;
>
>
> +void                       display_channel_free_some                 (DisplayChannel *display);
>  void                       display_channel_set_stream_video          (DisplayChannel *display,
>                                                                        int stream_video);
>  int                        display_channel_get_streams_timeout       (DisplayChannel *display);
>  void                       display_channel_compress_stats_print      (const DisplayChannel *display);
>  void                       display_channel_compress_stats_reset      (DisplayChannel *display);
> +Drawable *                 display_channel_drawable_try_new          (DisplayChannel *display,
> +                                                                      int group_id,
> +                                                                      int process_commands_generation);
>  void                       display_channel_drawable_unref            (DisplayChannel *display, Drawable *drawable);
>  void                       display_channel_surface_unref             (DisplayChannel *display,
>                                                                        uint32_t surface_id);
> @@ -388,5 +392,7 @@ void current_remove_drawable(DisplayChannel *display, Drawable *item);
>  void red_pipes_remove_drawable(Drawable *drawable);
>  void current_remove(DisplayChannel *display, TreeItem *item);
>  void detach_streams_behind(DisplayChannel *display, QRegion *region, Drawable *drawable);
> +void drawable_draw(DisplayChannel *display, Drawable *item);
> +void current_remove_all(DisplayChannel *display, int surface_id);
>
>  #endif /* DISPLAY_CHANNEL_H_ */
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 3873da4..f9b54ad 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -138,12 +138,10 @@ typedef struct BitmapData {
>
>  static inline int validate_surface(DisplayChannel *display, uint32_t surface_id);
>
> -static void drawable_draw(DisplayChannel *display, Drawable *item);
>  static void red_update_area(DisplayChannel *display, const SpiceRect *area, int surface_id);
>  static void red_update_area_till(DisplayChannel *display, const SpiceRect *area, int surface_id,
>                                   Drawable *last);
>  static inline void display_begin_send_message(RedChannelClient *rcc);
> -static int red_display_free_some_independent_glz_drawables(DisplayChannelClient *dcc);
>  static void red_create_surface(DisplayChannel *display, uint32_t surface_id, uint32_t width,
>                                 uint32_t height, int32_t stride, uint32_t format,
>                                 void *line_0, int data_is_valid, int send_client);
> @@ -170,7 +168,6 @@ static inline int validate_surface(DisplayChannel *display, uint32_t surface_id)
>      return 1;
>  }
>
> -
>  static int display_is_connected(RedWorker *worker)
>  {
>      return (worker->display_channel && red_channel_is_connected(
> @@ -227,20 +224,6 @@ static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32
>  }
>
>
> -static Drawable* drawable_try_new(DisplayChannel *display)
> -{
> -    Drawable *drawable;
> -
> -    if (!display->free_drawables)
> -        return NULL;
> -
> -    drawable = &display->free_drawables->u.drawable;
> -    display->free_drawables = display->free_drawables->u.next;
> -    display->drawable_count++;
> -
> -    return drawable;
> -}
> -
>  static void drawable_free(DisplayChannel *display, Drawable *drawable)
>  {
>      ((_Drawable *)drawable)->u.next = display->free_drawables;
> @@ -432,7 +415,7 @@ void current_remove(DisplayChannel *display, TreeItem *item)
>      }
>  }
>
> -static void current_remove_all(DisplayChannel *display, int surface_id)
> +void current_remove_all(DisplayChannel *display, int surface_id)
>  {
>      Ring *ring = &display->surfaces[surface_id].current;
>      RingItem *ring_item;
> @@ -790,31 +773,6 @@ static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
>      return TRUE;
>  }
>
> -static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
> -{
> -    RingItem *ring_item = ring_get_tail(&display->current_list);
> -    Drawable *drawable;
> -    Container *container;
> -
> -    if (!ring_item) {
> -        return FALSE;
> -    }
> -    drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
> -    if (force_glz_free) {
> -        RingItem *glz_item, *next_item;
> -        RedGlzDrawable *glz;
> -        DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> -            dcc_free_glz_drawable(glz->dcc, glz);
> -        }
> -    }
> -    drawable_draw(display, drawable);
> -    container = drawable->tree_item.base.container;
> -
> -    current_remove_drawable(display, drawable);
> -    container_cleanup(container);
> -    return TRUE;
> -}
> -
>  static Drawable *get_drawable(RedWorker *worker, uint8_t effect, RedDrawable *red_drawable,
>                                uint32_t group_id)
>  {
> @@ -832,29 +790,17 @@ static Drawable *get_drawable(RedWorker *worker, uint8_t effect, RedDrawable *re
>          }
>      }
>
> -    while (!(drawable = drawable_try_new(display))) {
> -        if (!free_one_drawable(display, FALSE))
> -            return NULL;
> +    drawable = display_channel_drawable_try_new(display, group_id, worker->process_commands_generation);
> +    if (!drawable) {
> +        return NULL;
>      }
>
> -    bzero(drawable, sizeof(Drawable));
> -    drawable->refs = 1;
> -    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 = red_drawable_ref(red_drawable);
> -    drawable->group_id = group_id;
>
>      drawable->surface_id = red_drawable->surface_id;
>      memcpy(drawable->surface_deps, red_drawable->surface_deps, sizeof(drawable->surface_deps));
> -    ring_init(&drawable->pipes);
> -    ring_init(&drawable->glz_ring);
>
> -    drawable->process_commands_generation = worker->process_commands_generation;
>      return drawable;
>  }
>
> @@ -1056,7 +1002,7 @@ static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces, uint32_t su
>      return display->surfaces[surface_id].context.canvas;
>  }
>
> -static void drawable_draw(DisplayChannel *display, Drawable *drawable)
> +void drawable_draw(DisplayChannel *display, Drawable *drawable)
>  {
>      RedSurface *surface;
>      SpiceCanvas *canvas;
> @@ -1524,49 +1470,6 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
>      return n;
>  }
>
> -#define RED_RELEASE_BUNCH_SIZE 64
> -
> -static void red_free_some(RedWorker *worker)
> -{
> -    DisplayChannel *display = worker->display_channel;
> -    int n = 0;
> -    DisplayChannelClient *dcc;
> -    RingItem *item, *next;
> -
> -    spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d", display->drawable_count,
> -                worker->red_drawable_count, display->glz_drawable_count);
> -    FOREACH_DCC(worker->display_channel, item, next, dcc) {
> -        GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> -
> -        if (glz_dict) {
> -            // encoding using the dictionary is prevented since the following operations might
> -            // change the dictionary
> -            pthread_rwlock_wrlock(&glz_dict->encode_lock);
> -            n = red_display_free_some_independent_glz_drawables(dcc);
> -        }
> -    }
> -
> -    while (!ring_is_empty(&display->current_list) && n++ < RED_RELEASE_BUNCH_SIZE) {
> -        free_one_drawable(display, TRUE);
> -    }
> -
> -    FOREACH_DCC(worker->display_channel, item, next, dcc) {
> -        GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> -
> -        if (glz_dict) {
> -            pthread_rwlock_unlock(&glz_dict->encode_lock);
> -        }
> -    }
> -}
> -
> -void display_channel_current_flush(DisplayChannel *display, int surface_id)
> -{
> -    while (!ring_is_empty(&display->surfaces[surface_id].current_list)) {
> -        free_one_drawable(display, FALSE);
> -    }
> -    current_remove_all(display, surface_id);
> -}
> -
>  static void fill_base(SpiceMarshaller *base_marshaller, Drawable *drawable)
>  {
>      SpiceMsgDisplayBase base;
> @@ -1578,31 +1481,6 @@ static void fill_base(SpiceMarshaller *base_marshaller, Drawable *drawable)
>      spice_marshall_DisplayBase(base_marshaller, &base);
>  }
>
> -/*
> - * Remove from the global lz dictionary some glz_drawables that have no reference to
> - * Drawable (their qxl drawables are released too).
> - * NOTE - the caller should prevent encoding using the dictionary during the operation
> - */
> -static int red_display_free_some_independent_glz_drawables(DisplayChannelClient *dcc)
> -{
> -    RingItem *ring_link;
> -    int n = 0;
> -
> -    if (!dcc) {
> -        return 0;
> -    }
> -    ring_link = ring_get_head(&dcc->glz_drawables);
> -    while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
> -        RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
> -        ring_link = ring_next(&dcc->glz_drawables, ring_link);
> -        if (!glz_drawable->drawable) {
> -            dcc_free_glz_drawable(dcc, glz_drawable);
> -            n++;
> -        }
> -    }
> -    return n;
> -}
> -
>  static inline void red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
>                                                           SpiceImage *image, SpiceImage *io_image,
>                                                           int is_lossy)
> @@ -5057,7 +4935,7 @@ static void handle_dev_oom(void *opaque, void *payload)
>          red_channel_push(&worker->display_channel->common.base);
>      }
>      if (worker->qxl->st->qif->flush_resources(worker->qxl) == 0) {
> -        red_free_some(worker);
> +        display_channel_free_some(worker->display_channel);
>          worker->qxl->st->qif->flush_resources(worker->qxl);
>      }
>      spice_debug("OOM2 #draw=%u, #red_draw=%u, #glz_draw=%u current %u pipes %u",
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

I have the feeling this patch could be split in a few other patches.
If anyone takes the bullet till Monday, I will :-)

-- 
Fabiano Fidêncio
_______________________________________________
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]