[Spice-devel] [PATCH 13/18] worker: move dcc_release_item

Fabiano Fidêncio fabiano at fidencio.org
Fri Nov 20 08:22:35 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>
>
> ---
>  server/dcc.c             | 124 +++++++++++++++++++++++++++++++++++++
>  server/dcc.h             |   3 +
>  server/display-channel.h |   7 +++
>  server/red_worker.c      | 158 ++++-------------------------------------------
>  4 files changed, 147 insertions(+), 145 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index fbaf386..eccaa77 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -1390,3 +1390,127 @@ int dcc_handle_migrate_data(DisplayChannelClient *dcc, uint32_t size, void *mess
>      red_channel_client_ack_zero_messages_window(RED_CHANNEL_CLIENT(dcc));
>      return TRUE;
>  }
> +
> +static void image_item_unref(ImageItem *item)
> +{
> +    if (--item->refs != 0)
> +        return;
> +
> +    free(item);
> +}
> +
> +static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item)
> +{
> +    if (--item->refs != 0)
> +        return;
> +
> +    display_channel_drawable_unref(display, item->drawable);
> +    free(item->rects);
> +    free(item);
> +}
> +
> +static void release_item_after_push(DisplayChannelClient *dcc, PipeItem *item)
> +{
> +    DisplayChannel *display = DCC_TO_DC(dcc);
> +
> +    switch (item->type) {
> +    case PIPE_ITEM_TYPE_DRAW:
> +        drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
> +        break;
> +    case PIPE_ITEM_TYPE_STREAM_CLIP:
> +        stream_clip_item_unref(dcc, (StreamClipItem *)item);
> +        break;
> +    case PIPE_ITEM_TYPE_UPGRADE:
> +        upgrade_item_unref(display, (UpgradeItem *)item);
> +        break;
> +    case PIPE_ITEM_TYPE_IMAGE:
> +        image_item_unref((ImageItem *)item);
> +        break;
> +    case PIPE_ITEM_TYPE_VERB:
> +        free(item);
> +        break;
> +    case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> +        MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> +                                                             MonitorsConfigItem, pipe_item);
> +        monitors_config_unref(monconf_item->monitors_config);
> +        free(item);
> +        break;
> +    }
> +    default:
> +        spice_critical("invalid item type");
> +    }
> +}
> +
> +// TODO: share code between before/after_push since most of the items need the same
> +// release
> +static void release_item_before_push(DisplayChannelClient *dcc, PipeItem *item)
> +{
> +    DisplayChannel *display = DCC_TO_DC(dcc);
> +
> +    spice_debug("item.type: %d", item->type);
> +    switch (item->type) {
> +    case PIPE_ITEM_TYPE_DRAW: {
> +        DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
> +        ring_remove(&dpi->base);
> +        drawable_pipe_item_unref(dpi);
> +        break;
> +    }
> +    case PIPE_ITEM_TYPE_STREAM_CREATE: {
> +        StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> +        stream_agent_unref(display, agent);
> +        break;
> +    }
> +    case PIPE_ITEM_TYPE_STREAM_CLIP:
> +        stream_clip_item_unref(dcc, (StreamClipItem *)item);
> +        break;
> +    case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> +        StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item);
> +        stream_agent_unref(display, agent);
> +        break;
> +    }
> +    case PIPE_ITEM_TYPE_UPGRADE:
> +        upgrade_item_unref(display, (UpgradeItem *)item);
> +        break;
> +    case PIPE_ITEM_TYPE_IMAGE:
> +        image_item_unref((ImageItem *)item);
> +        break;
> +    case PIPE_ITEM_TYPE_CREATE_SURFACE: {
> +        SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item, SurfaceCreateItem,
> +                                                              pipe_item);
> +        free(surface_create);
> +        break;
> +    }
> +    case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
> +        SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item, SurfaceDestroyItem,
> +                                                                pipe_item);
> +        free(surface_destroy);
> +        break;
> +    }
> +    case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> +        MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> +                                                             MonitorsConfigItem, pipe_item);
> +        monitors_config_unref(monconf_item->monitors_config);
> +        free(item);
> +        break;
> +    }
> +    case PIPE_ITEM_TYPE_INVAL_ONE:
> +    case PIPE_ITEM_TYPE_VERB:
> +    case PIPE_ITEM_TYPE_MIGRATE_DATA:
> +    case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> +    case PIPE_ITEM_TYPE_PIXMAP_RESET:
> +    case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE:
> +    case PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT:
> +        free(item);
> +        break;
> +    default:
> +        spice_critical("invalid item type");
> +    }
> +}
> +
> +void dcc_release_item(DisplayChannelClient *dcc, PipeItem *item, int item_pushed)
> +{
> +    if (item_pushed)
> +        release_item_after_push(dcc, item);
> +    else
> +        release_item_before_push(dcc, item);
> +}
> diff --git a/server/dcc.h b/server/dcc.h
> index 9da85ab..13d8c82 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -194,6 +194,9 @@ void                       dcc_add_drawable                          (DisplayCha
>  void                       dcc_add_drawable_after                    (DisplayChannelClient *dcc,
>                                                                        Drawable *drawable,
>                                                                        PipeItem *pos);
> +void                       dcc_release_item                          (DisplayChannelClient *dcc,
> +                                                                      PipeItem *item,
> +                                                                      int item_pushed);
>
>  typedef struct compress_send_data_t {
>      void*    comp_buf;
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 4a50912..b49cec9 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -242,6 +242,13 @@ typedef struct SurfaceDestroyItem {
>      PipeItem pipe_item;
>  } SurfaceDestroyItem;
>
> +typedef struct UpgradeItem {
> +    PipeItem base;
> +    int refs;
> +    Drawable *drawable;
> +    SpiceClipRects *rects;
> +} UpgradeItem;
> +
>
>  void                       display_channel_set_stream_video          (DisplayChannel *display,
>                                                                        int stream_video);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 10b6c00..3873da4 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -85,13 +85,6 @@ struct SpiceWatch {
>
>  #define MAX_PIPE_SIZE 50
>
> -typedef struct UpgradeItem {
> -    PipeItem base;
> -    int refs;
> -    Drawable *drawable;
> -    SpiceClipRects *rects;
> -} UpgradeItem;
> -
>  struct RedWorker {
>      pthread_t thread;
>      clockid_t clockid;
> @@ -151,10 +144,6 @@ static void red_update_area_till(DisplayChannel *display, const SpiceRect *area,
>                                   Drawable *last);
>  static inline void display_begin_send_message(RedChannelClient *rcc);
>  static int red_display_free_some_independent_glz_drawables(DisplayChannelClient *dcc);
> -static void display_channel_client_release_item_before_push(DisplayChannelClient *dcc,
> -                                                            PipeItem *item);
> -static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> -                                                           PipeItem *item);
>  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);
> @@ -213,23 +202,6 @@ void red_pipes_remove_drawable(Drawable *drawable)
>      }
>  }
>
> -static void release_image_item(ImageItem *item)
> -{
> -    if (!--item->refs) {
> -        free(item);
> -    }
> -}
> -
> -static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item)
> -{
> -    if (--item->refs != 0)
> -        return;
> -
> -    display_channel_drawable_unref(display, item->drawable);
> -    free(item->rects);
> -    free(item);
> -}
> -
>  static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size)
>  {
>      CommonChannel *common = SPICE_CONTAINEROF(rcc->channel, CommonChannel, base);
> @@ -3960,7 +3932,7 @@ static void red_marshall_stream_activate_report(RedChannelClient *rcc,
>      spice_marshall_msg_display_stream_activate_report(base_marshaller, &msg);
>  }
>
> -static void display_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item)
> +static void send_item(RedChannelClient *rcc, PipeItem *pipe_item)

This rename is not relevant for this patch ...

>  {
>      SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> @@ -4041,7 +4013,7 @@ static void display_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item
>          spice_error("invalid pipe item type");
>      }
>
> -    display_channel_client_release_item_before_push(dcc, pipe_item);
> +    dcc_release_item(dcc, pipe_item, FALSE);
>
>      // a message is pending
>      if (red_channel_client_send_message_pending(rcc)) {
> @@ -4059,7 +4031,7 @@ static inline void red_push(RedWorker *worker)
>      }
>  }
>
> -static void display_channel_client_on_disconnect(RedChannelClient *rcc)
> +static void on_disconnect(RedChannelClient *rcc)

Same here ...

>  {
>      DisplayChannel *display;
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> @@ -4335,7 +4307,7 @@ static inline void flush_all_qxl_commands(RedWorker *worker)
>      flush_cursor_commands(worker);
>  }
>
> -static int display_channel_handle_migrate_mark(RedChannelClient *rcc)
> +static int handle_migrate_flush_mark(RedChannelClient *rcc)

Same here ...

>  {
>      DisplayChannel *display_channel = SPICE_CONTAINEROF(rcc->channel, DisplayChannel, common.base);
>      RedChannel *channel = RED_CHANNEL(display_channel);
> @@ -4540,7 +4512,7 @@ RedChannel *red_worker_new_channel(RedWorker *worker, int size,
>      return channel;
>  }
>
> -static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item)
> +static void hold_item(RedChannelClient *rcc, PipeItem *item)

Same here ...

>  {
>      spice_assert(item);
>      switch (item->type) {
> @@ -4561,116 +4533,12 @@ static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item
>      }
>  }
>
> -static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> -                                                           PipeItem *item)
> -{
> -    DisplayChannel *display = DCC_TO_DC(dcc);
> -
> -    switch (item->type) {
> -    case PIPE_ITEM_TYPE_DRAW:
> -        drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
> -        break;
> -    case PIPE_ITEM_TYPE_STREAM_CLIP:
> -        stream_clip_item_unref(dcc, (StreamClipItem *)item);
> -        break;
> -    case PIPE_ITEM_TYPE_UPGRADE:
> -        upgrade_item_unref(display, (UpgradeItem *)item);
> -        break;
> -    case PIPE_ITEM_TYPE_IMAGE:
> -        release_image_item((ImageItem *)item);
> -        break;
> -    case PIPE_ITEM_TYPE_VERB:
> -        free(item);
> -        break;
> -    case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> -        MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> -                                                             MonitorsConfigItem, pipe_item);
> -        monitors_config_unref(monconf_item->monitors_config);
> -        free(item);
> -        break;
> -    }
> -    default:
> -        spice_critical("invalid item type");
> -    }
> -}
> -
> -// TODO: share code between before/after_push since most of the items need the same
> -// release
> -static void display_channel_client_release_item_before_push(DisplayChannelClient *dcc,
> -                                                            PipeItem *item)
> -{
> -    DisplayChannel *display = DCC_TO_DC(dcc);
> -
> -    switch (item->type) {
> -    case PIPE_ITEM_TYPE_DRAW: {
> -        DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
> -        ring_remove(&dpi->base);
> -        drawable_pipe_item_unref(dpi);
> -        break;
> -    }
> -    case PIPE_ITEM_TYPE_STREAM_CREATE: {
> -        StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> -        stream_agent_unref(display, agent);
> -        break;
> -    }
> -    case PIPE_ITEM_TYPE_STREAM_CLIP:
> -        stream_clip_item_unref(dcc, (StreamClipItem *)item);
> -        break;
> -    case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> -        StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item);
> -        stream_agent_unref(display, agent);
> -        break;
> -    }
> -    case PIPE_ITEM_TYPE_UPGRADE:
> -        upgrade_item_unref(display, (UpgradeItem *)item);
> -        break;
> -    case PIPE_ITEM_TYPE_IMAGE:
> -        release_image_item((ImageItem *)item);
> -        break;
> -    case PIPE_ITEM_TYPE_CREATE_SURFACE: {
> -        SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item, SurfaceCreateItem,
> -                                                              pipe_item);
> -        free(surface_create);
> -        break;
> -    }
> -    case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
> -        SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item, SurfaceDestroyItem,
> -                                                                pipe_item);
> -        free(surface_destroy);
> -        break;
> -    }
> -    case PIPE_ITEM_TYPE_MONITORS_CONFIG: {
> -        MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item,
> -                                                             MonitorsConfigItem, pipe_item);
> -        monitors_config_unref(monconf_item->monitors_config);
> -        free(item);
> -        break;
> -    }
> -    case PIPE_ITEM_TYPE_INVAL_ONE:
> -    case PIPE_ITEM_TYPE_VERB:
> -    case PIPE_ITEM_TYPE_MIGRATE_DATA:
> -    case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> -    case PIPE_ITEM_TYPE_PIXMAP_RESET:
> -    case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE:
> -    case PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT:
> -        free(item);
> -        break;
> -    default:
> -        spice_critical("invalid item type");
> -    }
> -}
> -
> -static void display_channel_release_item(RedChannelClient *rcc, PipeItem *item, int item_pushed)
> +static void release_item(RedChannelClient *rcc, PipeItem *item, int item_pushed)

Same here ...

>  {
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>
> -    spice_assert(item);
> -    if (item_pushed) {
> -        display_channel_client_release_item_after_push(dcc, item);
> -    } else {
> -        spice_debug("not pushed (%d)", item->type);
> -        display_channel_client_release_item_before_push(dcc, item);
> -    }
> +    spice_return_if_fail(item != NULL);
> +    dcc_release_item(dcc, item, item_pushed);
>  }
>
>  static void image_surface_init(DisplayChannel *display)
> @@ -4687,11 +4555,11 @@ static void display_channel_create(RedWorker *worker, int migrate, int stream_vi
>  {
>      DisplayChannel *display_channel;
>      ChannelCbs cbs = {
> -        .on_disconnect = display_channel_client_on_disconnect,
> -        .send_item = display_channel_send_item,
> -        .hold_item = display_channel_hold_pipe_item,
> -        .release_item = display_channel_release_item,
> -        .handle_migrate_flush_mark = display_channel_handle_migrate_mark,
> +        .on_disconnect = on_disconnect,
> +        .send_item = send_item,
> +        .hold_item = hold_item,
> +        .release_item = release_item,
> +        .handle_migrate_flush_mark = handle_migrate_flush_mark,

Same here ...

>          .handle_migrate_data = handle_migrate_data,
>          .handle_migrate_data_get_serial = handle_migrate_data_get_serial
>      };
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

ACK with these changes ...

-- 
Fabiano Fidêncio


More information about the Spice-devel mailing list