On Wed, 14 Dec 2011, Avi Kivity wrote:

On 12/14/2011 02:11 PM, Marcelo Tosatti wrote:

quoted

On Thu, Dec 08, 2011 at 10:23:10AM -0500, Eric B Munson wrote:

quoted

On Wed, 07 Dec 2011, Avi Kivity wrote:

quoted

On 12/05/2011 10:19 PM, Eric B Munson wrote:

quoted

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.

quoted

quoted

+	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