Re: [PATCH v6 2/3] KVM: dynamic halt_poll_ns adjustment

[Wanpeng Li]

On 9/3/15 2:09 AM, David Matlack wrote:
[...]
> > +
> >   static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
> >   {
> >          if (kvm_arch_vcpu_runnable(vcpu)) {
> > @@ -1929,6 +1963,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> >          ktime_t start, cur;
> >          DEFINE_WAIT(wait);
> >          bool waited = false;
> > +       u64 poll_ns = 0, wait_ns = 0, block_ns = 0;
> >
> >          start = cur = ktime_get();
> >          if (vcpu->halt_poll_ns) {
> > @@ -1941,10 +1976,15 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> >                           */
> >                          if (kvm_vcpu_check_block(vcpu) < 0) {
> >                                  ++vcpu->stat.halt_successful_poll;
> > -                               goto out;
> > +                               break;
> >                          }
> >                          cur = ktime_get();
> >                  } while (single_task_running() && ktime_before(cur, stop));
> > +
> > +               if (ktime_before(cur, stop)) {
> You can't use 'cur' to tell if the interrupt arrived. single_task_running()
> can break us out of the loop before 'stop'.
>
> > +                       poll_ns = ktime_to_ns(cur) - ktime_to_ns(start);
> Put this line before the if(). block_ns should always include the time
> spent polling; even if polling does not succeed.

How about something like:

@@ -1941,10 +1976,14 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
                         */
                        if (kvm_vcpu_check_block(vcpu) < 0) {
++vcpu->stat.halt_successful_poll;
-                               goto out;
+                               break;
                        }
                        cur = ktime_get();
                } while (single_task_running() && ktime_before(cur, stop));
+
+               poll_ns = ktime_to_ns(cur) - ktime_to_ns(start);
+               if (ktime_before(cur, stop) && single_task_running())
+                       goto out;
        }

> > +                       goto out;
> > +               }
> >          }
> >
> >          for (;;) {
> > @@ -1959,9 +1999,24 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> >
> >          finish_wait(&vcpu->wq, &wait);
> >          cur = ktime_get();
> > +       wait_ns = ktime_to_ns(cur) - ktime_to_ns(start);
> >
> >   out:
> > -       trace_kvm_vcpu_wakeup(ktime_to_ns(cur) - ktime_to_ns(start), waited);
> > +       block_ns = poll_ns + wait_ns;
> > +
> > +       if (halt_poll_ns) {
> If you want, you can leave this if() out and save some indentation.

Then we will miss the tracepoint.

Regards,
Wanpeng Li

> > +               if (block_ns <= vcpu->halt_poll_ns)
> > +                       ;
> > +               /* we had a long block, shrink polling */
> > +               else if (vcpu->halt_poll_ns && block_ns > halt_poll_ns)
> > +                       shrink_halt_poll_ns(vcpu);
> > +               /* we had a short halt and our poll time is too small */
> > +               else if (vcpu->halt_poll_ns < halt_poll_ns &&
> > +                       block_ns < halt_poll_ns)
> > +                       grow_halt_poll_ns(vcpu);
> > +       }
> > +
> > +       trace_kvm_vcpu_wakeup(block_ns, waited);
> >   }
> >   EXPORT_SYMBOL_GPL(kvm_vcpu_block);
> >
> > --
> > 1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/