[Spice-devel] [PATCH 8/9] worker: make sure we dispatch after releasing items

[Frediano Ziglio]

> 
> > 
> > Hi
> > 
> > ----- Original Message -----
> > > Same comment as
> > > http://lists.freedesktop.org/archives/spice-devel/2015-December/024682.html
> > > 
> > > "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?
> > 
> > The question for me is rather, why would you wait for a timeout when you
> > know
> > you can process it now?
> > 
> 
> Process what actually? If you already deleted the item you already processed
> it.
> If the item is going out of worker you will get a reply, possibly a new item.
> If the item was going in it's already processed.
> Or am I missing something?
> 

Oh... now I start getting all these problems!
All came from Glib integration!
Basically before there was a loop were is was clear steps:
1- select
2- timers
3- fd events
4- guest events
5- push data to channel.
Now point 5 is just a GSource so you don't know if is the last and
you force an iteration to make sure you don't wait too much.

Well.. now I have another reason to prove that waiting to merge Glib loop code
was a good choice.

About how to solve this issue (GSources ordering)... I don't know!
Maybe using priorities. I'll have a look at Glib code.

Frediano

> 
> > > 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 at redhat.com>
> > > 
> > > 
> > > On Wed, 2015-12-09 at 12:17 +0000, Frediano Ziglio wrote:
> > > > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > > > 
> > > > ---
> > > >  server/display-channel.c | 2 ++
> > > >  server/red-worker.c      | 8 ++++++++
> > > >  server/red-worker.h      | 1 +
> > > >  3 files changed, 11 insertions(+)
> > > > 
> > > > diff --git a/server/display-channel.c b/server/display-channel.c
> > > > index 32d87af..60170cb 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 b59a5a1..ae4ac27 100644
> > > > --- a/server/red-worker.c
> > > > +++ b/server/red-worker.c
> > > > @@ -112,6 +112,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->event_timeout = MIN(worker->event_timeout, timeout);
> > > > +}
> > > > +
> > > >  static int display_is_connected(RedWorker *worker)
> > > >  {
> > > >      return (worker->display_channel && red_channel_is_connected(
> > > > 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);
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>