From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 1/2] KVM: Remove unused KVM_REQ_KICK to save a bit in vcpu->requests Date: Fri, 8 Jan 2016 13:22:42 +0100 Message-ID: <568FAA12.5030604@redhat.com> 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> <568F154D.40407@lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:36490 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752424AbcAHMWq (ORCPT ); Fri, 8 Jan 2016 07:22:46 -0500 Received: by mail-wm0-f65.google.com with SMTP id l65so15875492wmf.3 for ; Fri, 08 Jan 2016 04:22:45 -0800 (PST) In-Reply-To: <568F154D.40407@lab.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: On 08/01/2016 02:47, Takuya Yoshikawa wrote: > > 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. True, on the other hand they will be well predicted. Any time I tried reordering kvm_check_request or other similar micro-optimizations, everything became slower. Same for replacing all the clear_bit with a single xchg and then testing the bits on the value that xchg returned. Even "obvious" replacements such as - if (vcpu->requests) + if (vcpu->requests & ~(1 << KVM_REQ_EVENT)) weren't clear winners! But anyway, this is where I would start from; not for_each_set_bit or __ffs. There are other optimizations that are easily done, unrelated to vcpu->requests: lapic_in_kernel is a dup of kvm_vcpu_has_lapic, but without the static key optimization. I prefer the lapic_in_kernel name, but it has the less efficient implementation. They ought to be merged. If you really want to free bits, a possible optimization would be to move KVM_REQ_REPORT_TPR_ACCESS, KVM_REQ_TRIPLE_FAULT, KVM_REQ_HV_CRASH, KVM_REQ_HV_RESET, and KVM_REQ_HV_EXIT to a separate vcpu->vmexit_requests field. You could either check it on every vmexit, or just consolidate them into a KVM_REQ_VMEXIT bit. > 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. I've never liked the dummy request really, but I've never felt like arguing for its removal either. :) Paolo