Re: [Xen-devel] [PATCH v2 05/10] libxl: event: Make libxl__poller_wakeup take a gc, not an egc

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 1/17/20 1:46 PM, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH v2 05/10] libxl: event: Make 
> libxl__poller_wakeup take a gc, not an egc"):
>> On 1/13/20 5:08 PM, Ian Jackson wrote:
>>> We are going to want to call this in the following situation:
>>>
>>>  * We have just set up an ao, which is to call back - so a
>>>    non-synchronous one.  It ought not to call the application
>>>    back right away, so no egc.
>>>
>>>  * There is a libxl thread blocking somewhere but it is using
>>>    using an out of date fd or timeout set, which does not take into
>>>    account the ao we have just started.
>>>
>>>  * We try to wake that thread up, but libxl__poller_wakeup fails.
>>
>> So the idea before was that these two functions take an egc, not so much
>> because it actually uses the egc, but to make sure it's only called in a
>> restricted set of conditions; and now we're relaxing those conditions?
> 
> Yes.  Specifically, we need to make one exception, relating to ao's.
> 
> In the situation described above, there is no egc, but we need to call
> libxl__poller_wakeup.  Introducing an egc is wrong because that would
> imply that this situation might result in application callbacks, but
> it shouldn't (and not having an egc prevents that).
> 
> libxl__poller_wakeup and LIBXL__EVENT_DISASTER only take an egc for
> form's sake; they don't use any part of it other than the gc.  The
> "form's sake" is to stop them being called from libxl entrypoints that
> are not involved in event generation.
> 
> Before this patch this is enforced by the types: you can't call it in
> the wrong place because it wants an egc which you don't have.
> 
> After this patch this is no longer enforced.  But the mistake
> (principally, calling _DISASTER) seems unlikely.  The type enforcement
> I mention above was done because it was possible and easy, not because
> it was important.

That makes sense; just trying partly to make sure I have it right,
partly to have things in the public record.  In which case, re the code:

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>


> Does more of this want to be in the commit message ?

I was going to say I'm not sure we need another round-trip.  I'd be OK
with checking it in as-is; or you could edit the commit message on
check-in if you wanted.

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