From patchwork Mon Sep 24 06:24:47 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takuya Yoshikawa X-Patchwork-Id: 1496281 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 6D3953FE80 for ; Mon, 24 Sep 2012 06:25:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754934Ab2IXGY6 (ORCPT ); Mon, 24 Sep 2012 02:24:58 -0400 Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]:45963 "EHLO tama50.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754831Ab2IXGY5 (ORCPT ); Mon, 24 Sep 2012 02:24:57 -0400 Received: from mfs6.rdh.ecl.ntt.co.jp (mfs6.rdh.ecl.ntt.co.jp [129.60.39.149]) by tama50.ecl.ntt.co.jp (8.13.8/8.13.8) with ESMTP id q8O6OrxC012471; Mon, 24 Sep 2012 15:24:53 +0900 Received: from mfs6.rdh.ecl.ntt.co.jp (localhost.localdomain [127.0.0.1]) by mfs6.rdh.ecl.ntt.co.jp (Postfix) with ESMTP id 4B0E8E00F4; Mon, 24 Sep 2012 15:24:53 +0900 (JST) Received: from imail2.m.ecl.ntt.co.jp (imail2.m.ecl.ntt.co.jp [129.60.5.247]) by mfs6.rdh.ecl.ntt.co.jp (Postfix) with ESMTP id 3F4FBE0066; Mon, 24 Sep 2012 15:24:53 +0900 (JST) Received: from yshpad ([129.60.241.152]) by imail2.m.ecl.ntt.co.jp (8.13.8/8.13.8) with SMTP id q8O6OrRL030901; Mon, 24 Sep 2012 15:24:53 +0900 Date: Mon, 24 Sep 2012 15:24:47 +0900 From: Takuya Yoshikawa To: avi@redhat.com, mtosatti@redhat.com Cc: kvm@vger.kernel.org Subject: [RFC PATCH] KVM: x86: Skip request checking branches in vcpu_enter_guest() more effectively Message-Id: <20120924152447.36c71b8f.yoshikawa_takuya_b1@lab.ntt.co.jp> X-Mailer: Sylpheed 3.1.0 (GTK+ 2.24.4; x86_64-pc-linux-gnu) Mime-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org This is an RFC since I have not done any comparison with the approach using for_each_set_bit() which can be seen in Avi's work. Takuya --- We did a simple test to see which requests we would get at the same time in vcpu_enter_guest() and got the following numbers: |...........|...............|........|...............|.| (N) (E) (S) (ES) others 22.3% 30.7% 16.0% 29.5% 1.4% (N) : Nothing (E) : Only KVM_REQ_EVENT (S) : Only KVM_REQ_STEAL_UPDATE (ES): Only KVM_REQ_EVENT and KVM_REQ_STEAL_UPDATE * Note that the exact numbers can change for other guests. This motivated us to optimize the following code in vcpu_enter_guest(): if (vcpu->requests) { /** (1) **/ ... if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) /** (2) **/ record_steal_time(vcpu); ... } if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) { ... } - For case (E), we do kvm_check_request() for every request other than KVM_REQ_EVENT in the block (1) and always get false. - For case (S) and (ES), the only difference from case (E) is that we get true for KVM_REQ_STEAL_UPDATE. This means that we were wasting a lot of time for the many if branches in the block (1) even for these trivial three cases which dominated more than 75% in total. This patch mitigates the issue as follows: - For case (E), we change the first if condition to if (vcpu->requests & ~(1 << KVM_REQ_EVENT)) /** (1') **/ so that we can skip the block completely. - For case (S) and (ES), we move the check (2) upwards, out of the block (1), to clear the KVM_REQ_STEAL_UPDATE flag before doing the check (1'). Although this adds one if branch for case (N), the fact that steal update occurs frequently enough except when we give each vcpu a dedicated core justifies its tiny cost. Signed-off-by: Takuya Yoshikawa --- [My email address change is not a mistake.] arch/x86/kvm/x86.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index fc2a0a1..e351902 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5233,7 +5233,14 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) vcpu->run->request_interrupt_window; bool req_immediate_exit = 0; - if (vcpu->requests) { + /* + * Getting KVM_REQ_STEAL_UPDATE alone is so common that clearing it now + * will hopefully result in skipping the following checks. + */ + if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) + record_steal_time(vcpu); + + if (vcpu->requests & ~(1 << KVM_REQ_EVENT)) { if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu)) kvm_mmu_unload(vcpu); if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu)) @@ -5267,8 +5274,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) r = 1; goto out; } - if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) - record_steal_time(vcpu); if (kvm_check_request(KVM_REQ_NMI, vcpu)) process_nmi(vcpu); req_immediate_exit =