From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH 1/2] vm_event: sync domctl Date: Wed, 23 Dec 2015 21:11:14 +0200 Message-ID: <567AF1D2.8050004@bitdefender.com> References: <1450882432-10484-1-git-send-email-tamas@tklengyel.com> <567AC09B.2040403@bitdefender.com> <567AD725.9090207@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aBooi-0008Gz-P9 for xen-devel@lists.xenproject.org; Wed, 23 Dec 2015 19:11:21 +0000 Received: from smtp02.buh.bitdefender.net (unknown [10.17.80.76]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 9DD6F80510 for ; Wed, 23 Dec 2015 21:11:15 +0200 (EET) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel , Andrew Cooper Cc: Xen-devel , Stefano Stabellini , Wei Liu , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 12/23/2015 08:11 PM, Tamas K Lengyel wrote: > > > On Wed, Dec 23, 2015 at 6:17 PM, Andrew Cooper > > wrote: > > On 23/12/2015 15:41, Razvan Cojocaru wrote: > > On 12/23/2015 04:53 PM, Tamas K Lengyel wrote: > >> Introduce new vm_event domctl option which allows an event subscriber > >> to request all vCPUs not currently pending a vm_event request to be paused, > >> thus allowing the subscriber to sync up on the state of the domain. This > >> is especially useful when the subscribed wants to disable certain events > >> from being delivered and wants to ensure no more requests are pending on the > >> ring before doing so. > >> > >> Cc: Ian Jackson > > >> Cc: Stefano Stabellini > > >> Cc: Ian Campbell > > >> Cc: Wei Liu > > >> Cc: Razvan Cojocaru > > >> Signed-off-by: Tamas K Lengyel > > > This certainly looks very interesting. Would xc_domain_pause() not be > > enough for your use case then? > > I second this query. I would have thought xc_domain_pause() does > exactly what you want in this case. > > > The problem is in what order the responses are processed. I may not be > correct about the logic but here is what my impression was: > xc_domain_unpause resumes all vCPUs even if there is still a vm_event > response that has not been processed. Now, if the subscriber set > response flags (altp2m switch, singlestep toggle, etc) those actions > would not be properly performed on the vCPU before it's resumed. If the > subscriber processes all requests and signals via the event channel that > the responses are on the ring, then calls xc_domain_unpause, we can > still have a race between processing the responses from the ring and > unpausing the vCPU. > > > The code provided is racy, as it is liable to alter which pause > references it takes/releases depending on what other pause/unpause > actions are being made. > > > It's understood that the user would not use xc_domain_pause/unpause > while using vm_event responses with response flags specified. Even then, > it was already racy IMHO if the user called xc_domain_unpause before > processing requests from the vm_event ring that originally paused the > vCPU, so this doesn't change that situation. There are a bunch of checks in vcpu_wake() (xen/common/schedule.c) that I've always assumed guard against the problem you're describing. I may be wrong (I don't have any experience with the scheduling code), but even if I am, I still think having xc_domain_pause() / xc_domain_unpause() behave correctly is better than adding a new libxc function. Is that an unreasonable goal? Thanks, Razvan