From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760916AbbBIPVX (ORCPT ); Mon, 9 Feb 2015 10:21:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44417 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759816AbbBIPVW (ORCPT ); Mon, 9 Feb 2015 10:21:22 -0500 Date: Mon, 9 Feb 2015 16:21:12 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini 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 Message-ID: <20150209152111.GB1693@potion.brq.redhat.com> References: <1423226937-11169-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1423226937-11169-1-git-send-email-pbonzini@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > 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 } }; > diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h > @@ -37,6 +37,25 @@ TRACE_EVENT(kvm_userspace_exit, > __entry->errno < 0 ? -__entry->errno : __entry->reason) > ); > > +TRACE_EVENT(kvm_vcpu_wakeup, > + TP_PROTO(__u64 ns, bool waited), (__u64 is preferred here?) > @@ -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.) - is it ok to ignore need_resched() when single_task_running()? (Most likely not.) Thanks. --- Annoying detail: single_task_running() makes it look that we are only trying to avoid CPU power saving with accompanied latency. We aren't doing it perfectly though: scheduler can run to short task and the CPU will go to sleep. (Before the event KVM is waiting for arrives.) Adding a general API (cpu_don't_sleep_for_next_x_ns) and modifying the cpu_idle_loop() would cover this improbable situation and may be applied to other use-cases too ... but still is extra work with uncertain acceptability :)