From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933539AbbBIQKw (ORCPT ); Mon, 9 Feb 2015 11:10:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42495 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932284AbbBIQKt (ORCPT ); Mon, 9 Feb 2015 11:10:49 -0500 Message-ID: <54D8DBFE.1070508@redhat.com> Date: Mon, 09 Feb 2015 17:10:38 +0100 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= CC: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, riel@redhat.com, mtosatti@redhat.com, jan.kiszka@siemens.com, dmatlack@google.com Subject: Re: [PATCH] kvm: add halt_poll_ns module parameter References: <1423226937-11169-1-git-send-email-pbonzini@redhat.com> <20150209152111.GB1693@potion.brq.redhat.com> In-Reply-To: <20150209152111.GB1693@potion.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/02/2015 16:21, Radim Krčmář wrote: > 2015-02-06 13:48+0100, Paolo Bonzini: > [...] >> Signed-off-by: Paolo Bonzini >> --- > > Reviewed-by: Radim Krčmář > > 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 { >> }; >> >> struct kvm_vcpu_stat { >> + u32 halt_successful_poll; >> u32 halt_wakeup; >> }; > > We don't expose it in arch/arm/kvm/guest.c, > struct kvm_stats_debugfs_item debugfs_entries[] = { > { NULL } > }; Yes. Too late for 3.20. >> +TRACE_EVENT(kvm_vcpu_wakeup, >> + TP_PROTO(__u64 ns, bool waited), > > (__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 = false; >> + >> + start = cur = ktime_get(); >> + if (halt_poll_ns) { >> + ktime_t stop = 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 = ktime_get(); >> + } while (single_task_running() && ktime_before(cur, stop)); > > 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