Re: [Xen-devel] [PATCH v2 10/10] libxl: event: Move poller pipe emptying to the end of afterpoll

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 1/17/20 2:24 PM, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH v2 10/10] libxl: event: Move poller pipe 
> emptying to the end of afterpoll"):
>> On 1/13/20 5:08 PM, Ian Jackson wrote:
>>> If a timer event callback causes this poller to be woken (not very
>>> unlikely) we would go round the poll loop twice rather than once.
>>>
>>> Do the poller pipe emptying at the end; this is slightly more
>>> efficient because it can't cause any callbacks, so it happens after
>>> all the callbacks have been run.
>>>
>>> (This pipe-emptying has to happen in afterpoll rather than the
>>> apparently more logical beforepoll, because the application calling
>>> beforepoll doesn't constitute a promise to actually do anything.)
>>>
>>> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>>
>> I can't quite figure out: why would you end up going around the loop
>> twice, and how does this fix it?
> 
> I now think this is not true and the situation I describe cannot
> happen.
> 
> What I was thinking was that pollers_note_osevent_added might be
> called by something from time_occurs, and that would write a byte into
> the poller pipe.  But pollers_note_osevent_added doesn't wake up
> pollers; it just tags them osevents_added.
> 
> I now think the spurious wakeup cannot happen because:
> 
> For this patch to make any difference, the poller pipe would have to
> be woken up by something in the time scan loop in afterpoll_internal.
> 
> But poller pipes are only woken up by ao completion or by
> cleanup_1_baton.
> 
> cleanup_1_baton is not called anywhere there (as an argument against:
> any such call would violate the rule that cleanup_1_baton may not be
> called with a poller in hand).
> 
> And as for ao completion, we would indeed wake up the poller.  But we
> also mark the ao as complete, so ao_inprogress would spot
> !ao_work_outstanding, and not reenter eventloop_iteration at all.
> The woken-up poller would be put by ao_inprogress.
> 
> This leads me to this observation: poller_get might give you a
> woken-up poller.  This is not incorrect, but it is pointless.  So
> maybe I should write a patch that puts a call to
> libxl__self_pipe_eatall in libxl__poller_get.
> 
> TBH I still think this patch tidies the code up a bit.

No objection to it on those grounds. :-)

Thanks for the explanation,
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel