Re: [PATCH 13/18] worker: move dcc_release_item

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

 



On Mon, Nov 23, 2015 at 2:36 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>>
>> On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>> >
>> > ---
>> >  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@xxxxxxxxxxxxxxxxxxxxx
>> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>> ACK with these changes ...
>>
>> --
>> Fabiano Fidêncio
>>
>
> Could you post an updated patch?

Sure!

>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
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]