>>> On 24.04.15 at 08:32, <kai.hu...@linux.intel.com> wrote: > On 04/17/2015 03:37 PM, Jan Beulich wrote: >>>>> On 17.04.15 at 09:23, <kai.hu...@linux.intel.com> wrote: >>> I see. I will do as you suggested: >>> >>> ASSERT((v == current) || (!vcpu_runnable(v) && !v->is_running)); >>> >>> And v != current should be the only case requires the vcpu to be paused. >> But if you require (or at least expect) the vCPU to be paused, this >> isn't what I suggested. Afaict >> >> ASSERT((v == current) || (!v->is_running && atomic_read(v->pause_count))); >> >> would then be the right check (and, while not be a full guarantee that >> things wouldn't change behind your back, would at least increase >> chances that the vCPU's runnable state won't change, as the vCPU >> could have been non-runnable for reasons other than having been >> paused). > Hi Jan, > > I tried the ASSERT with atomic_read(&v->pause_count), and it turns out > the ASSERT would fail and panic Xen. The reason is domain_pause only > increases d->pause_count, but it doesn't internally increase > v->pause_count for all vcpus. > > vmx_vcpu_flush_pml_buffer is only supposed to be called from PML buffer > full VMEXIT, and vmx_domain_flush_pml_buffer, before which domain_pause > should be called. > > Sorry that obviously I had some misunderstanding regarding to "require > (or at least expect) vCPU to be paused", and looks !vcpu_runnable(v) is > the right choice. > > What's your opinion?
Then either go with the slightly weaker original (still quoted at the top) or (preferred by me) OR together both pause counts in the condition. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel