[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 15/15] libxl: New event generation API
On Mon, 2011-12-12 at 12:30 +0000, Ian Jackson wrote: > Ian Campbell writes ("Re: [Xen-devel] [PATCH 15/15] libxl: New event > generation API"): > > On Mon, 2011-12-12 at 11:40 +0000, Ian Jackson wrote: > > > Well, we have to have an internal version because we don't want to > > > call GC_INIT again, obviously. > > > > In this case it would take a ctx not a gc and it's ok and expected for > > libxl a function calling another libxl function to end up with another > > gc in the inner function (it happens all the time). > > This is not allowed in functions which might generate events (ie, > which might call libxl__event_occurred), because the application's > event callback will be called at an unexpected point in libxl's > execution. This has three problems: > > * There is a reentrancy hazard. Exactly whether this is a problem in > a specific case is difficult to analyse; hence the recording of > pending callbacks in the gc, so that they can be delayed until the > gc is freed. > > * The application's callbacks will be called with the libxl ctx > locked. This might result in deadlock. > > * The application's callbacks might be called in the wrong order > because the inner gc (containing later events) is unwound before > the outer gc (containing earlier events). > > In general this means that functions which lock the ctx must not > allocate an inner gc. Hmm, yes that all makes sense :-/ I was a little surprised when I looked when writing my previous reply that LIBXL_INITGC doesn't actually take nesting into account. I had expected that LIBXL_INITGC would return (or somehow chain) the existing GC when libxl is calling itself such that all the freeing work (and now callbacks) would only happen when exiting the final frame. It's probably not so important for memory allocation cleanup but it would be a semantically useful change for the callbacks if we could arrange for them to happen only on final exit of the library -- exactly because of the difficulty of reasoning about things otherwise. Alas I can't think of a good way to do this since the ctx can be shared, thread local storage would be one or libxl__gc_owner could return a special "nested ctx" instead of the real outer ctx, but, ick. If we can't figure a way round this then the restriction about libxl not calling into itself via its own public interfaces should be documented (apologies if I've simply missed that bit, I did go and look because I thought I recalled seeing it, but I didn't find it, I think I was thinking about the comment associated with the CTX lock). In particular if the restriction is that you cannot call public API functions which might generate events from within libxl then those functions need to be clearly annotated as potentially generating events. > > IOW if you make beforepoll_internal take the lock as you suggest then > > you may as well inline it into libxl_osevent_beforepoll (removing the > > second lock use) and use that internally too. > > beforepoll_unlocked is called in two places: libxl_osevent_beforepoll, > and libxl_event_wait. My suggestion was to call libxl_osevent_beforepoll from libxl_event_wait. The documentation for libxl_event_wait even says this is how libxl_event_wait is implemented (I know I'm not a liberty to take that as literally as that). Of course your explanation above put paid to that idea, unless we can work around it. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |