Re: [PATCH 13/15] worker: make sure we dispatch after releasing items

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

 



On Thu, 2015-12-03 at 16:27 +0000, Frediano Ziglio wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> 
> ---
>  server/display-channel.c |  2 ++
>  server/red-worker.c      | 10 ++++++++++
>  server/red-worker.h      |  1 +
>  3 files changed, 13 insertions(+)
> 
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 02ccc15..838b8ff 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1987,9 +1987,11 @@ static void hold_item(RedChannelClient *rcc, PipeItem
> *item)
>  static void release_item(RedChannelClient *rcc, PipeItem *item, int
> item_pushed)
>  {
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> +    RedWorker *worker = DCC_TO_WORKER(dcc);
>  
>      spice_return_if_fail(item != NULL);
>      dcc_release_item(dcc, item, item_pushed);
> +    red_worker_update_timeout(worker, 0);
>  }
>  
>  static int handle_migrate_flush_mark(RedChannelClient *rcc)
> diff --git a/server/red-worker.c b/server/red-worker.c
> index ba3454e..6fc4382 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -110,6 +110,14 @@ RedMemSlotInfo* red_worker_get_memslot(RedWorker *worker)
>      return &worker->mem_slots;
>  }
>  
> +void red_worker_update_timeout(RedWorker *worker, gint timeout)
> +{
> +    spice_return_if_fail(worker != NULL);
> +    spice_return_if_fail(timeout >= 0);
> +
> +    worker->timeout = MIN(worker->timeout, timeout);
> +}
> +
>  static int display_is_connected(RedWorker *worker)
>  {
>      return (worker->display_channel && red_channel_is_connected(
> @@ -1641,6 +1649,8 @@ static gboolean worker_source_prepare(GSource *source,
> gint *timeout)
>      *timeout = worker->timeout;
>      *timeout = MIN(worker->timeout,
>                     display_channel_get_streams_timeout(worker
> ->display_channel));
> +    if (*timeout == 0)
> +        return TRUE;

I think that this particular hunk could probably be squashed into the original
glib loop commit.

>  
>      return FALSE; /* do no timeout poll */
>  }
> diff --git a/server/red-worker.h b/server/red-worker.h
> index b55a45c..3a9dc19 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -94,6 +94,7 @@ static inline void red_pipes_add_verb(RedChannel *channel,
> uint16_t verb)
>  
>  RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher);
>  bool       red_worker_run(RedWorker *worker);
> +void       red_worker_update_timeout(RedWorker *worker, gint timeout);
>  QXLInstance* red_worker_get_qxl(RedWorker *worker);
>  RedChannel* red_worker_get_cursor_channel(RedWorker *worker);
>  RedChannel* red_worker_get_display_channel(RedWorker *worker);



I don't really understand the reason for this patch. Why do we need to wakeup
the worker immediately after a display pipe item has been released, but not a
cursor item? Why can't we wait for the next worker poll timeout to expire?
Updating the worker timeout seems like an implementation detail, so adding an
API for other classes to call seems a little weird to me.

Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
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]