On 02/28/2012 04:00 PM, Nadav Har'El wrote:
> On Tue, Feb 28, 2012, Avi Kivity wrote about "Re: [PATCH] vhost: don't forget to schedule()":
> > > + if (need_resched())
> > > + schedule();
> >
> > This is cond_resched(), no?
>
> It indeed looks similar, but it appears there are some slightly
> different things happening in both cases, especially for a preemptive
> kernel... Unfortunately, I am not astute (or experienced) enough to tell
> which of the two idioms are better or more appropriate for this case.
I'd have expected that cond_resched() is a no-op with preemptible
kernels, but I see this is not the case.
>
> The idiom that I used seemed right, and seemed to work in my tests.
> Moreover I also noticed it was used in vmx.c. Also, vhost.c was already
> calling schedule(), not cond_resched(), so I thought it made sense to
> call the same thing...
>
> But I now see that in kvm_main.c, there's also this:
>
> if (!need_resched())
> return;
> cond_resched();
>
> Which seems to combine both idioms ;-) Can anybody shed a light on what
> is the right way to do it?
>
It's bogus. Look at commit 3fca03653010:
Author: Yaozu Dong <eddie.dong@xxxxxxxxx>
Date: Wed Apr 25 16:49:19 2007 +0300
KVM: VMX: Avoid unnecessary vcpu_load()/vcpu_put() cycles
By checking if a reschedule is needed, we avoid dropping the vcpu.
[With changes by me, based on Anthony Liguori's observations]
Signed-off-by: Avi Kivity <avi@xxxxxxxxxxxx>
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 03c0ee7..f535635 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -1590,6 +1590,8 @@ static int set_msr(struct kvm_vcpu *vcpu, u32
msr_index, u64 data)
void kvm_resched(struct kvm_vcpu *vcpu)
{
+ if (!need_resched())
+ return;
vcpu_put(vcpu);
cond_resched();
vcpu_load(vcpu);
at that time, it made sense to do the extra check to avoid the expensive
vcpu_put/vcpu_load. Later preempt notifiers made them redundant
(15ad71460d75), and they were removed, but the extra check remained.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
