From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753323AbdDJLOe (ORCPT ); Mon, 10 Apr 2017 07:14:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51984 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752760AbdDJLOc (ORCPT ); Mon, 10 Apr 2017 07:14:32 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D946861B91 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=drjones@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D946861B91 Date: Mon, 10 Apr 2017 13:14:27 +0200 From: Andrew Jones To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Christoffer Dall , Marc Zyngier , Paolo Bonzini , Christian Borntraeger , Cornelia Huck , James Hogan , Paul Mackerras Subject: Re: [PATCH 6/6] KVM: perform a wake_up in kvm_make_all_cpus_request Message-ID: <20170410111427.uq3neitfcssm6vbn@kamzik.brq.redhat.com> References: <20170406202056.18379-1-rkrcmar@redhat.com> <20170406202056.18379-7-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170406202056.18379-7-rkrcmar@redhat.com> User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 10 Apr 2017 11:14:32 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 06, 2017 at 10:20:56PM +0200, Radim Krčmář wrote: > We want to have kvm_make_all_cpus_request() to be an optmized version of > > kvm_for_each_vcpu(i, vcpu, kvm) { > kvm_make_request(vcpu, request); > kvm_vcpu_kick(vcpu); > } > > and kvm_vcpu_kick() wakes up the target vcpu. We know which requests do > not need the wake up and use it to optimize the loop. Any reason we don't want kvm_vcpu_kick() to also get the if (!(req & KVM_REQUEST_NO_WAKEUP)) optimization condition? I did some grepping, and don't see any kicks of the requests that have been marked as NO_WAKEUP, so nothing should change by adding it now. But the consistency would be nice for the doc I'm writing. Also, the condition in kvm_vcpu_kick() looks like overkill cpu != me && (unsigned)cpu < nr_cpu_ids && cpu_online(cpu) How could vcpu->cpu ever be any offline/invalid cpu, other than -1? The condition in kvm_make_all_cpus_request() makes more sense to me cpu != -1 && cpu != me I guess a lot this stuff is planned for a larger requests rework, when kicks get integrated with requests? I'm a bit anxious, though, as it changes how I document stuff now, and even how I approach the ARM series. For example, if kvm_make_request() already integrated kvm_vcpu_kick(), which means also adding the smp_mb__after_atomic(), like kvm_make_all_cpus_request() has, then I wouldn't need to add the smp_mb() to kvm_arch_vcpu_should_kick(). Thanks, drew > > Thanks to that, this patch doesn't change the behavior of current users > (the all don't need the wake up) and only prepares for future where the > wake up is going to be needed. > > I think that most requests do not need the wake up, so we would flip the > bit then. > > Signed-off-by: Radim Krčmář > --- > virt/kvm/kvm_main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index a486c6ad27a6..1db503bab3dc 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -186,6 +186,9 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req) > /* Set ->requests bit before we read ->mode. */ > smp_mb__after_atomic(); > > + if (!(req & KVM_REQUEST_NO_WAKEUP)) > + kvm_vcpu_wake_up(vcpu); > + > if (cpus != NULL && cpu != -1 && cpu != me && > kvm_arch_vcpu_should_kick(vcpu)) > cpumask_set_cpu(cpu, cpus); > -- > 2.12.0 >