From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] kvm: add halt_poll_ns module parameter Date: Mon, 09 Feb 2015 17:10:38 +0100 Message-ID: <54D8DBFE.1070508@redhat.com> References: <1423226937-11169-1-git-send-email-pbonzini@redhat.com> <20150209152111.GB1693@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, riel@redhat.com, mtosatti@redhat.com, jan.kiszka@siemens.com, dmatlack@google.com To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: In-Reply-To: <20150209152111.GB1693@potion.brq.redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 09/02/2015 16:21, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > 2015-02-06 13:48+0100, Paolo Bonzini: > [...] >> Signed-off-by: Paolo Bonzini >> --- >=20 > Reviewed-by: Radim Kr=C4=8Dm=C3=A1=C5=99 >=20 > Noticed changes since RFC: > - polling is used in more situations > - new tracepoint > - module parameter in nanoseconds > - properly handled time > - no polling with overcommit Yup, pretty much what came in from Marcelo and David. >> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/= kvm_host.h >> @@ -148,6 +148,7 @@ struct kvm_vm_stat { >> }; >> =20 >> struct kvm_vcpu_stat { >> + u32 halt_successful_poll; >> u32 halt_wakeup; >> }; >=20 > We don't expose it in arch/arm/kvm/guest.c, > struct kvm_stats_debugfs_item debugfs_entries[] =3D { > { NULL } > }; Yes. Too late for 3.20. >> +TRACE_EVENT(kvm_vcpu_wakeup, >> + TP_PROTO(__u64 ns, bool waited), >=20 > (__u64 is preferred here?) Preferred to what? >> @@ -1813,29 +1816,60 @@ void mark_page_dirty(struct kvm *kvm, gfn_t = gfn) >> void kvm_vcpu_block(struct kvm_vcpu *vcpu) >> { >> + ktime_t start, cur; >> DEFINE_WAIT(wait); >> + bool waited =3D false; >> + >> + start =3D cur =3D ktime_get(); >> + if (halt_poll_ns) { >> + ktime_t stop =3D ktime_add_ns(ktime_get(), halt_poll_ns); >> + do { >> + /* >> + * This sets KVM_REQ_UNHALT if an interrupt >> + * arrives. >> + */ >> + if (kvm_vcpu_check_block(vcpu) < 0) { >> + ++vcpu->stat.halt_successful_poll; >> + goto out; >> + } >> + cur =3D ktime_get(); >> + } while (single_task_running() && ktime_before(cur, stop)); >=20 > After reading a bunch of code, I'm still not sure ... > - need_resched() can't be true when single_task_running()? > (I think it could happen -- balancing comes into mind.) Single_task_running is per-CPU; for a task to relinquish control to another task, you first need to have multiple tasks running. In other words, I think it cannot. > - is it ok to ignore need_resched() when single_task_running()? > (Most likely not.) Paolo