From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Subject: Re: [PATCH 1/2] KVM: Remove unused KVM_REQ_KICK to save a bit in vcpu->requests Date: Fri, 8 Jan 2016 10:47:57 +0900 Message-ID: <568F154D.40407@lab.ntt.co.jp> References: <1452166994-11344-1-git-send-email-yoshikawa_takuya_b1@lab.ntt.co.jp> <1452166994-11344-2-git-send-email-yoshikawa_takuya_b1@lab.ntt.co.jp> <568E5C15.90804@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Paolo Bonzini Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:44275 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753482AbcAHBrz (ORCPT ); Thu, 7 Jan 2016 20:47:55 -0500 In-Reply-To: <568E5C15.90804@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2016/01/07 21:37, Paolo Bonzini wrote: > On 07/01/2016 12:43, Takuya Yoshikawa wrote: >> Signed-off-by: Takuya Yoshikawa >> --- >> include/linux/kvm_host.h | 45 ++++++++++++++++++++++----------------------- >> 1 file changed, 22 insertions(+), 23 deletions(-) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 61c3e6c..ca9b93e 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -122,29 +122,28 @@ static inline bool is_error_page(struct page *page) >> #define KVM_REQ_UNHALT 6 >> #define KVM_REQ_MMU_SYNC 7 >> #define KVM_REQ_CLOCK_UPDATE 8 >> -#define KVM_REQ_KICK 9 > > I'd prefer to leave just an /* unused: 9 */ here. Since some request handlers can make other requests which need to be checked in the following path, this young number is valuable. In this sense, I agree. Your patch looks good to me. > This patch can go in for 4.5. Regarding the other patch, > KVM_REQ_MCLOCK_INPROGRESS is indeed not really necessary, see > http://www.spinics.net/lists/kvm/msg95944.html and the follow-up. Were > you thinking of the same? If so I would prefer to have some comments. I did not notice that discussion. What I have in mind is: http://www.spinics.net/lists/kvm/msg80477.html We have more requests than that time and the overhead of requests checking has become much worse: Hyper-V code may still add some more if-branches, which will always be false for many guests. To find a solution, I am doing some experiments and trying to re-implement Avi's for_each_set_bit based solution. The problem is that for_each_set_bit is too generic and involves too many function calls, which can actually make the code slower. But since vcpu->requests is unsigned long, we can directly use __ffs as kvm_mmu_write_protect_pt_masked() does. This is the reason why I object to expanding it to 64-bit size now. I still need to think carefully about the dependencies between a few request handlers and am not sure what I am thinking now will really work. Anyway, your patches make enough room for now, so I do not need to remove KVM_REQ_MCLOCK_INPROGRESS dummy request for that. It depends just on our preference. Takuya