Re: [PATCH 01/30] Do not release too much drawables

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

 



I have to admit that I'm not very familiar with this bit of the code. But it's
clearly a behavior change and it seems like one that could have non-obvious or
hard-to-test consequences. Can you give a bit more justification for the change?
Does it fix an observed issue? It seems that these glz drawables are specific to
each client, so it doesn't seem "wrong" to free the same number for each
client. 

Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>


On Tue, 2016-06-07 at 11:17 +0100, Frediano Ziglio wrote:
> Accumulate counter, do not reset for each client.
> 
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  server/dcc-encoders.c    | 5 ++---
>  server/dcc-encoders.h    | 3 ++-
>  server/display-channel.c | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 5570798..cc235fa 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -532,13 +532,12 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc,
> RedGlzDrawable *drawable)
>   * 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)
> +int dcc_free_some_independent_glz_drawables(DisplayChannelClient *dcc, int n)
>  {
>      RingItem *ring_link;
> -    int n = 0;
>  
>      if (!dcc) {
> -        return 0;
> +        return n;
>      }
>      ring_link = ring_get_head(&dcc->glz_drawables);
>      while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 0d3e96a9..fed8d58 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -40,7 +40,8 @@
> void             dcc_encoders_init                           (DisplayChannelCl
> ie
>  void             dcc_encoders_free                           (DisplayChannelC
> lient *dcc);
>  void             dcc_free_glz_drawable                       (DisplayChannelC
> lient *dcc,
>                                                                RedGlzDrawable
> *drawable);
> -int              dcc_free_some_independent_glz_drawables     (DisplayChannelC
> lient *dcc);
> +int              dcc_free_some_independent_glz_drawables     (DisplayChannelC
> lient *dcc,
> +                                                              int
> release_count);
>  void             dcc_free_glz_drawables                      (DisplayChannelC
> lient *dcc);
>  void             dcc_free_glz_drawables_to_free              (DisplayChannelC
> lient* dcc);
>  void             dcc_freeze_glz                              (DisplayChannelC
> lient *dcc);
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 2888cad..db059b4 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1267,7 +1267,7 @@ void display_channel_free_some(DisplayChannel *display)
>              // 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);
> +            n = dcc_free_some_independent_glz_drawables(dcc, n);
>          }
>      }
>  
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://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]