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