lkml.org 
[lkml]   [2015]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] kvm: add halt_poll_ns module parameter
2015-02-06 13:48+0100, Paolo Bonzini:
[...]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

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 :)


\
 
 \ /
  Last update: 2015-02-09 16:41    [W:0.114 / U:7.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site