From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756196AbbIDBmV (ORCPT <rfc822;w@1wt.eu>);
	Thu, 3 Sep 2015 21:42:21 -0400
Received: from blu004-omc1s8.hotmail.com ([65.55.116.19]:57103 "EHLO
	BLU004-OMC1S8.hotmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1755228AbbIDBmT (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 3 Sep 2015 21:42:19 -0400
X-TMN: [GO5klA6bGTQoDxChRt+OqAx9YSKxWZjL2GTuuTkYmnU=]
X-Originating-Email: [wanpeng.li@hotmail.com]
Message-ID: <BLU436-SMTP48C469210EC2FC5B0AB64F80570@phx.gbl>
Subject: Re: [PATCH v6 2/3] KVM: dynamic halt_poll_ns adjustment
To: David Matlack <dmatlack@google.com>
References: <1441178971-3836-1-git-send-email-wanpeng.li@hotmail.com>
 <BLU436-SMTP5094DCF6122F1E1D93C68B80690@phx.gbl>
 <CALzav=dLvOPLszw84GE9m7dLSvg8jmoaFY2LjtTmN=LrK95HpA@mail.gmail.com>
 <BLU436-SMTP132C316315912A92D8AA61480570@phx.gbl>
CC: Paolo Bonzini <pbonzini@redhat.com>, kvm list <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
From: Wanpeng Li <wanpeng.li@hotmail.com>
Date: Fri, 4 Sep 2015 09:42:10 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0)
 Gecko/20100101 Thunderbird/38.2.0
MIME-Version: 1.0
In-Reply-To: <BLU436-SMTP132C316315912A92D8AA61480570@phx.gbl>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-OriginalArrivalTime: 04 Sep 2015 01:42:17.0182 (UTC) FILETIME=[EE63A7E0:01D0E6B2]
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 9/4/15 9:16 AM, Wanpeng Li 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.
>
> Ah, I remember why I keep poll_ns calculation in if(). Actually 
> poll_ns has already been accumulated into wait_ns(see how wait_ns is 
> calculated) if polling does not succeed. If put poll_ns calculation 
> before if() I need to reset variable 'start' to ktime_get().

So how about something like:

@@ -1928,7 +1962,8 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
  {
         ktime_t start, cur;
         DEFINE_WAIT(wait);
-       bool waited = false;
+       bool polled = false, waited = false;
+       u64 poll_ns = 0, wait_ns = 0, block_ns = 0;

         start = cur = ktime_get();
         if (vcpu->halt_poll_ns) {
@@ -1940,11 +1975,17 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
                          * arrives.
                          */
                         if (kvm_vcpu_check_block(vcpu) < 0) {
+                               polled = true;
++vcpu->stat.halt_successful_poll;
-                               goto out;
+                               break;
                         }
                         cur = ktime_get();
                 } while (single_task_running() && ktime_before(cur, stop));
+
+               if (polled) {
+                       poll_ns = ktime_to_ns(cur) - ktime_to_ns(start);
+                       goto out;
+               }
         }


Regards,
Wanpeng Li