On 23/12/2015 21:06, Tamas K Lengyel wrote: > > > On Wed, Dec 23, 2015 at 9:55 PM, Tamas K Lengyel <ta...@tklengyel.com > <mailto:ta...@tklengyel.com>> wrote: > > > > > On Wed, Dec 23, 2015 at 8:14 PM, Andrew Cooper > <andrew.coop...@citrix.com <mailto:andrew.coop...@citrix.com>> wrote: > > On 23/12/2015 18:11, Tamas K Lengyel wrote: >> >> >> On Wed, Dec 23, 2015 at 6:17 PM, Andrew Cooper >> <andrew.coop...@citrix.com >> <mailto:andrew.coop...@citrix.com>> 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 <ian.jack...@eu.citrix.com >> <mailto:ian.jack...@eu.citrix.com>> >> >> Cc: Stefano Stabellini >> <stefano.stabell...@eu.citrix.com >> <mailto:stefano.stabell...@eu.citrix.com>> >> >> Cc: Ian Campbell <ian.campb...@citrix.com >> <mailto:ian.campb...@citrix.com>> >> >> Cc: Wei Liu <wei.l...@citrix.com >> <mailto:wei.l...@citrix.com>> >> >> Cc: Razvan Cojocaru <rcojoc...@bitdefender.com >> <mailto:rcojoc...@bitdefender.com>> >> >> Signed-off-by: Tamas K Lengyel <ta...@tklengyel.com >> <mailto:ta...@tklengyel.com>> >> > 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. > > Pausing is strictly reference counted. (or rather, it is since > c/s 3eb1c70 "properly reference count DOMCTL_{,un}pausedomain > hypercalls". Before then, it definitely was buggy.) > > There is the domain pause count, and pause counts per vcpu. > All domain pause operations take both a domain pause > reference, and a vcpu pause reference on each vcpu. A vcpu is > only eligible to be scheduled if its pause reference count is > zero. If two independent tasks call vcpu_pause() on the same > vcpu, it will remain paused until both independent tasks have > called vcpu_unpause(). > > > Actually, I've double-checked and v->pause_count and > v->vm_event_pause_count are both increased for a vm_event request. So > you are right, the reference counting will make sure that > v->pause_count > 0 until we process the vm_event response and call > xc_domain_unpause. I was under the impression that wasn't the case. We > can ignore this patch. > > Thanks and sorry for the noise ;)
Not a problem at all. This is complicated stuff, and IMO it was equally as likely that there was a real bug lurking. ~Andrew
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
