Re: [PATCH 13/18] worker: move more free_glz_drawable

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

 



On Wed, Nov 18, 2015 at 5:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>
> ---
>  server/dcc-encoders.c    | 55 ++++++++++++++++++++++++++++++++++++++
>  server/dcc-encoders.h    |  7 +++--
>  server/display-channel.c | 22 +++++++++++----
>  server/display-channel.h |  1 +
>  server/red_worker.c      | 69 ------------------------------------------------
>  5 files changed, 78 insertions(+), 76 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 9104405..385b6b6 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -473,6 +473,39 @@ void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
>      }
>  }
>
> +/*
> + * Releases all the instances of the drawable from the dictionary and the display channel client.
> + * The release of the last instance will also release the drawable itself and the qxl drawable
> + * if possible.
> + * NOTE - the caller should prevent encoding using the dictionary during this operation
> + */
> +void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable)
> +{
> +    RingItem *head_instance = ring_get_head(&drawable->instances);
> +    int cont = (head_instance != NULL);
> +
> +    while (cont) {
> +        if (drawable->instances_count == 1) {
> +            /* Last instance: dcc_free_glz_drawable_instance will free the drawable */
> +            cont = FALSE;
> +        }
> +        GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
> +                                                        GlzDrawableInstanceItem,
> +                                                        glz_link);
> +        if (!ring_item_is_linked(&instance->free_link)) {
> +            // the instance didn't get out from window yet
> +            glz_enc_dictionary_remove_image(dcc->glz_dict->dict,
> +                                            instance->context,
> +                                            &dcc->glz_data.usr);
> +        }
> +        dcc_free_glz_drawable_instance(dcc, instance);
> +
> +        if (cont) {
> +            head_instance = ring_get_head(&drawable->instances);
> +        }
> +    }
> +}
> +
>  void dcc_free_glz_drawables_to_free(DisplayChannelClient* dcc)
>  {
>      RingItem *ring_link;
> @@ -489,3 +522,25 @@ void dcc_free_glz_drawables_to_free(DisplayChannelClient* dcc)
>      }
>      pthread_mutex_unlock(&dcc->glz_drawables_inst_to_free_lock);
>  }
> +
> +/* Clear all lz drawables - enforce their removal from the global dictionary.
> +   NOTE - prevents encoding using the dictionary during the operation*/
> +void dcc_free_glz_drawables(DisplayChannelClient *dcc)
> +{
> +    RingItem *ring_link;
> +    GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> +
> +    if (!glz_dict) {
> +        return;
> +    }
> +
> +    // assure no display channel is during global lz encoding
> +    pthread_rwlock_wrlock(&glz_dict->encode_lock);
> +    while ((ring_link = ring_get_head(&dcc->glz_drawables))) {
> +        RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
> +        // no need to lock the to_free list, since we assured no other thread is encoding and
> +        // thus not other thread access the to_free list of the channel
> +        dcc_free_glz_drawable(dcc, drawable);
> +    }
> +    pthread_rwlock_unlock(&glz_dict->encode_lock);
> +}
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 4dc50b1..dd3d528 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -34,10 +34,15 @@
>
>  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);
> +void             dcc_free_glz_drawables                      (DisplayChannelClient *dcc);
>  void             dcc_free_glz_drawables_to_free              (DisplayChannelClient* dcc);
>
>  void             marshaller_add_compressed                   (SpiceMarshaller *m,
> @@ -124,8 +129,6 @@ typedef struct {
>
>  #define MAX_GLZ_DRAWABLE_INSTANCES 2
>
> -typedef struct RedGlzDrawable RedGlzDrawable;
> -
>  /* for each qxl drawable, there may be several instances of lz drawables */
>  /* TODO - reuse this stuff for the top level. I just added a second level of multiplicity
>   * at the Drawable by keeping a ring, so:
> diff --git a/server/display-channel.c b/server/display-channel.c
> index a391c29..6db5aa7 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -850,14 +850,26 @@ void display_channel_flush_all_surfaces(DisplayChannel *display)
>      }
>  }
>
> -static void rcc_free_glz_drawables_to_free(RedChannelClient *rcc)
> +void display_channel_free_glz_drawables_to_free(DisplayChannel *display)
>  {
> -    DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> +    RingItem *link, *next;
> +    DisplayChannelClient *dcc;
>
> -    dcc_free_glz_drawables_to_free(dcc);
> +    spice_return_if_fail(display);

Not exactly sure if we should abort on this case, or ...

> +
> +    DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display)) {
> +        dcc_free_glz_drawables_to_free(dcc);
> +    }
>  }
>
> -void display_channel_free_glz_drawables_to_free(DisplayChannel *display)
> +void display_channel_free_glz_drawables(DisplayChannel *display)
>  {
> -    red_channel_apply_clients(RED_CHANNEL(display), rcc_free_glz_drawables_to_free);
> +    RingItem *link, *next;
> +    DisplayChannelClient *dcc;
> +
> +    spice_return_if_fail(display);


... also in this case ...

> +
> +    DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display)) {
> +        dcc_free_glz_drawables(dcc);
> +    }
>  }
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 684f983..3e67889 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -274,6 +274,7 @@ void                       display_channel_current_flush             (DisplayCha
>  int                        display_channel_wait_for_migrate_data     (DisplayChannel *display);
>  void                       display_channel_flush_all_surfaces        (DisplayChannel *display);
>  void                       display_channel_free_glz_drawables_to_free(DisplayChannel *display);
> +void                       display_channel_free_glz_drawables        (DisplayChannel *display);
>
>  static inline int is_equal_path(SpicePath *path1, SpicePath *path2)
>  {
> diff --git a/server/red_worker.c b/server/red_worker.c
> index fd68753..19d8f43 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -187,7 +187,6 @@ static void red_freeze_glz(DisplayChannelClient *dcc);
>  static void display_channel_push_release(DisplayChannelClient *dcc, uint8_t type, uint64_t id,
>                                           uint64_t* sync_data);
>  static int red_display_free_some_independent_glz_drawables(DisplayChannelClient *dcc);
> -static void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable);
>  static void display_channel_client_release_item_before_push(DisplayChannelClient *dcc,
>                                                              PipeItem *item);
>  static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> @@ -2007,74 +2006,6 @@ static void fill_base(SpiceMarshaller *base_marshaller, Drawable *drawable)
>  }
>
>  /*
> - * Releases all the instances of the drawable from the dictionary and the display channel client.
> - * The release of the last instance will also release the drawable itself and the qxl drawable
> - * if possible.
> - * NOTE - the caller should prevent encoding using the dictionary during this operation
> - */
> -static void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable)
> -{
> -    RingItem *head_instance = ring_get_head(&drawable->instances);
> -    int cont = (head_instance != NULL);
> -
> -    while (cont) {
> -        if (drawable->instances_count == 1) {
> -            /* Last instance: dcc_free_glz_drawable_instance will free the drawable */
> -            cont = FALSE;
> -        }
> -        GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
> -                                                        GlzDrawableInstanceItem,
> -                                                        glz_link);
> -        if (!ring_item_is_linked(&instance->free_link)) {
> -            // the instance didn't get out from window yet
> -            glz_enc_dictionary_remove_image(dcc->glz_dict->dict,
> -                                            instance->context,
> -                                            &dcc->glz_data.usr);
> -        }
> -        dcc_free_glz_drawable_instance(dcc, instance);
> -
> -        if (cont) {
> -            head_instance = ring_get_head(&drawable->instances);
> -        }
> -    }
> -}
> -
> -/* Clear all lz drawables - enforce their removal from the global dictionary.
> -   NOTE - prevents encoding using the dictionary during the operation*/
> -static void dcc_free_glz_drawables(DisplayChannelClient *dcc)
> -{
> -    RingItem *ring_link;
> -    GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> -
> -    if (!glz_dict) {
> -        return;
> -    }
> -
> -    // assure no display channel is during global lz encoding
> -    pthread_rwlock_wrlock(&glz_dict->encode_lock);
> -    while ((ring_link = ring_get_head(&dcc->glz_drawables))) {
> -        RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
> -        // no need to lock the to_free list, since we assured no other thread is encoding and
> -        // thus not other thread access the to_free list of the channel
> -        dcc_free_glz_drawable(dcc, drawable);
> -    }
> -    pthread_rwlock_unlock(&glz_dict->encode_lock);
> -}
> -
> -static void display_channel_free_glz_drawables(DisplayChannel *display_channel)
> -{
> -    RingItem *link, *next;
> -    DisplayChannelClient *dcc;
> -
> -    if (!display_channel) {
> -        return;
> -    }
> -    DCC_FOREACH_SAFE(link, next, dcc, RED_CHANNEL(display_channel)) {
> -        dcc_free_glz_drawables(dcc);
> -    }
> -}
> -
> -/*
>   * 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
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Pacth seems okay. ACK.

-- 
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]