Re: [PATCH 2/5 V5] Add functions to check if the host has stoppedthe vm

[Eric B Munson]

On Wed, 14 Dec 2011, Avi Kivity wrote:

> On 12/14/2011 02:11 PM, Marcelo Tosatti wrote:
> > On Thu, Dec 08, 2011 at 10:23:10AM -0500, Eric B Munson wrote:
> > > On Wed, 07 Dec 2011, Avi Kivity wrote:
> > > 
> > > > On 12/05/2011 10:19 PM, Eric B Munson wrote:
> > > > > When a host stops or suspends a VM it will set a flag to show this.  The
> > > > > watchdog will use these functions to determine if a softlockup is real, or the
> > > > > result of a suspended VM.
> > > > >  
> > > > > +bool kvm_check_and_clear_guest_paused(int cpu)
> > > > > +{
> > > > > +	bool ret = false;
> > > > > +	struct pvclock_vcpu_time_info *src;
> > > > > +
> > > > > +	/*
> > > > > +	 * per_cpu() is safe here because this function is only called from
> > > > > +	 * timer functions where preemption is already disabled.
> > > > > +	 */
> > > > > +	WARN_ON(!in_atomic());
> > > > > +	src = &per_cpu(hv_clock, cpu);
> > > > 
> > > > __get_cpu_var(); drop the cpu argument
> > > > 
> > > 
> > > Will change for V6.
> > > 
> > > > > +	if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
> > > > > +		src->flags = src->flags & (~PVCLOCK_GUEST_STOPPED);
> > > > 
> > > > Isn't this racy?  Between reading and writing src->flags, we can exit to
> > > > the hypervisor and add/remove new flags.  The write then overrides those
> > > > new flags.
> > > > 
> > > 
> > > If I understand (please correct me if this is wrong) because this is only
> > > called from the watchdog, which disables preemption, we should be protected
> > > from something else writing to these flags.
> >
> > The host can write, but in that case race is harmless.
> 
> Why is it harmless?  You don't know what's in those other flags.
> 
> -- 
> error compiling committee.c: too many arguments to function
> 

Currently there is only one other flag in this byte (PVCLOCK_TSC_STABLE_BIT)
and it isset once in kvmclock_init().  It is highly unlikely that the vm will
be stopped during this init and have the flag clobbered.  After the tsc stable
bit is written in the init, the field is read only outside of the guest paused
code.

Eric

Attachment: signature.asc
Description: Digital signature