[Spice-devel] [PATCH 15/18] worker: move display_channel_free_some
Fabiano Fidêncio
fabiano at fidencio.org
Fri Nov 20 08:44:47 PST 2015
On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> Author: Marc-André Lureau <marcandre.lureau at gmail.com>
> ---
> 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 at lists.freedesktop.org
> 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
More information about the Spice-devel
mailing list