From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 15:19:34 +0100 Message-ID: <53B171F6.10406@citrix.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> <53B1611D.2080502@citrix.com> <1404133751.14488.56.camel@kazak.uk.xensource.com> <53B161DD.1010809@citrix.com> <53B18C5F020000780001E96B@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53B18C5F020000780001E96B@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Ian Campbell Cc: Tamas Lengyel , Aravindh Puthiyaparambil , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 30/06/14 15:12, Jan Beulich wrote: >>>> On 30.06.14 at 15:10, wrote: >> On 30/06/14 14:09, Ian Campbell wrote: >>> On Mon, 2014-06-30 at 14:07 +0100, Andrew Cooper wrote: >>>> On 30/06/14 13:44, Ian Campbell wrote: >>>>> On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: >>>>>> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add >> helper API to setup ring and enable mem_access""): >>>>>>> > Now with this function being reintroduced, it becomes more complicated >>>>>>> > to determine which version of the mem_access API does Xen actually >>>>>>> > provide. A #define indicating mem_access API version would nicely >>>>>>> > overcome this issue, or naming xc_mem_event_enable something else. >>>>>>> >>>>>>> Doesn't configure support checking for functions with a given prototype? >>>>>>> >>>>>>> It does but in a very hacky way, essentially trying to compile code where >> the >>>>>>> function is being called with different prototypes. We can work around it >> but a >>>>>>> clean solution would be preferred at some point. >>>>>> I agree with your criticism, TBH. Aravindh/Ian, can we rename this >>>>>> function ? >>>>> I have no objection to some other name. >>>>> >>>>>>> This is now a rather opaque behavior of libxc. As it is not merged >>>>>>> into master I guess I will just submit a patch for it.. >>>>>> Yes. I think this would be the right approach to addressing both of >>>>>> these problems. (Please send two patches, one for each.) >>>>>> >>>>>> Thanks, >>>>>> Ian. >>>> There is a bug in Xen. domctl pause and unpause hypercalls are not >>>> properly refcounted. Patch on its way. >>> I had convinced myself by reading that they were, but Tamas' >>> experimental evidence obviously counters that ;-) >> The "d->is_paused_by_controller" is a boolean, not an atomic count. >> >> The second pause doesn't add an extra pause ref to the domain, and the >> first unpause resumes the domain. > Which, as you see, is (in the hypervisor) intended behavior. If the > tool stack wants this to be ref-counted, it needs to do so by itself. > > Jan I can see that it is not the current behaviour, but no indication as to why it is intended not to be properly recounted. With memaccess setup like this it is perfectly possible to have multiple toolstack userspace processes needing to pause/unpause the domain over small periods to ensure safety. Refcounting in Xen is the only plausible way of maintaining consistency; there is no safe way to refcount this in the toolstack. The patch from Tanas introduces a TOCTOU race which won't solve the problem in the general case. ~Andrew