lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALzav=cSo6r+cW5JuvfzPzkwEx4eziDK3VyLcfmSw-B01XoPgQ@mail.gmail.com>
Date:	Mon, 9 Feb 2015 11:52:36 -0800
From:	David Matlack <dmatlack@...gle.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	kvm list <kvm@...r.kernel.org>, riel@...hat.com,
	Marcelo Tosatti <mtosatti@...hat.com>, rkrcmar@...hat.com,
	jan.kiszka@...mens.com
Subject: Re: [PATCH] kvm: add halt_poll_ns module parameter

On Fri, Feb 6, 2015 at 4:48 AM, Paolo Bonzini <pbonzini@...hat.com> wrote:
> This patch introduces a new module parameter for the KVM module; when it
> is present, KVM attempts a bit of polling on every HLT before scheduling
> itself out via kvm_vcpu_block.
>
[...]
>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---

Looks good, thanks for making those changes. I ran this patch on my
benchmarks (loopback TCP_RR and memcache) using halt_poll_ns=70000 and
saw performance go from 40% to 60-65% of bare-metal.

Tested-by: David Matlack <dmatlack@...gle.com>
Reviewed-by: David Matlack <dmatlack@...gle.com>

>  arch/arm/include/asm/kvm_host.h     |  1 +
>  arch/arm64/include/asm/kvm_host.h   |  1 +
>  arch/mips/include/asm/kvm_host.h    |  1 +
>  arch/mips/kvm/mips.c                |  1 +
>  arch/powerpc/include/asm/kvm_host.h |  1 +
>  arch/powerpc/kvm/book3s.c           |  1 +
>  arch/powerpc/kvm/booke.c            |  1 +
>  arch/s390/include/asm/kvm_host.h    |  1 +
>  arch/s390/kvm/kvm-s390.c            |  1 +
>  arch/x86/include/asm/kvm_host.h     |  1 +
>  arch/x86/kvm/x86.c                  |  1 +
>  include/trace/events/kvm.h          | 19 +++++++++++++++
>  virt/kvm/kvm_main.c                 | 48 +++++++++++++++++++++++++++++++------
>  13 files changed, 71 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index bde494654bcc..6a79314bc1df 100644
> --- 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;
>  };
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 2c49aa4ac818..8efde89613f2 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -165,6 +165,7 @@ struct kvm_vm_stat {
>  };
>
>  struct kvm_vcpu_stat {
> +       u32 halt_successful_poll;
>         u32 halt_wakeup;
>  };
>
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index f2c249796ea8..ac4fc716062b 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -120,6 +120,7 @@ struct kvm_vcpu_stat {
>         u32 resvd_inst_exits;
>         u32 break_inst_exits;
>         u32 flush_dcache_exits;
> +       u32 halt_successful_poll;
>         u32 halt_wakeup;
>  };
>
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index e97b90784031..c9eccf5df912 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -49,6 +49,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>         { "resvd_inst",   VCPU_STAT(resvd_inst_exits),   KVM_STAT_VCPU },
>         { "break_inst",   VCPU_STAT(break_inst_exits),   KVM_STAT_VCPU },
>         { "flush_dcache", VCPU_STAT(flush_dcache_exits), KVM_STAT_VCPU },
> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll), KVM_STAT_VCPU },
>         { "halt_wakeup",  VCPU_STAT(halt_wakeup),        KVM_STAT_VCPU },
>         {NULL}
>  };
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 7efd666a3fa7..8ef05121d3cd 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -107,6 +107,7 @@ struct kvm_vcpu_stat {
>         u32 emulated_inst_exits;
>         u32 dec_exits;
>         u32 ext_intr_exits;
> +       u32 halt_successful_poll;
>         u32 halt_wakeup;
>         u32 dbell_exits;
>         u32 gdbell_exits;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 888bf466d8c6..cfbcdc654201 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -52,6 +52,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>         { "dec",         VCPU_STAT(dec_exits) },
>         { "ext_intr",    VCPU_STAT(ext_intr_exits) },
>         { "queue_intr",  VCPU_STAT(queue_intr) },
> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll), },
>         { "halt_wakeup", VCPU_STAT(halt_wakeup) },
>         { "pf_storage",  VCPU_STAT(pf_storage) },
>         { "sp_storage",  VCPU_STAT(sp_storage) },
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 9b55dec2d6cc..6c1316a15a27 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -62,6 +62,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>         { "inst_emu",   VCPU_STAT(emulated_inst_exits) },
>         { "dec",        VCPU_STAT(dec_exits) },
>         { "ext_intr",   VCPU_STAT(ext_intr_exits) },
> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
>         { "halt_wakeup", VCPU_STAT(halt_wakeup) },
>         { "doorbell", VCPU_STAT(dbell_exits) },
>         { "guest doorbell", VCPU_STAT(gdbell_exits) },
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index d1ecc7fd0579..f79058e3fd98 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -196,6 +196,7 @@ struct kvm_vcpu_stat {
>         u32 exit_stop_request;
>         u32 exit_validity;
>         u32 exit_instruction;
> +       u32 halt_successful_poll;
>         u32 halt_wakeup;
>         u32 instruction_lctl;
>         u32 instruction_lctlg;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index b2371c0fd1f8..1dbab2340a66 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -51,6 +51,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>         { "exit_instruction", VCPU_STAT(exit_instruction) },
>         { "exit_program_interruption", VCPU_STAT(exit_program_interruption) },
>         { "exit_instr_and_program_int", VCPU_STAT(exit_instr_and_program) },
> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
>         { "halt_wakeup", VCPU_STAT(halt_wakeup) },
>         { "instruction_lctlg", VCPU_STAT(instruction_lctlg) },
>         { "instruction_lctl", VCPU_STAT(instruction_lctl) },
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 848947ac6ade..a236e39cc385 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -655,6 +655,7 @@ struct kvm_vcpu_stat {
>         u32 irq_window_exits;
>         u32 nmi_window_exits;
>         u32 halt_exits;
> +       u32 halt_successful_poll;
>         u32 halt_wakeup;
>         u32 request_irq_exits;
>         u32 irq_exits;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1373e04e1f19..bd7a70be41b3 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -145,6 +145,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>         { "irq_window", VCPU_STAT(irq_window_exits) },
>         { "nmi_window", VCPU_STAT(nmi_window_exits) },
>         { "halt_exits", VCPU_STAT(halt_exits) },
> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
>         { "halt_wakeup", VCPU_STAT(halt_wakeup) },
>         { "hypercalls", VCPU_STAT(hypercalls) },
>         { "request_irq", VCPU_STAT(request_irq_exits) },
> diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
> index 6edf1f2028cd..6bfe7eec1c2c 100644
> --- 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),
> +           TP_ARGS(ns, waited),
> +
> +       TP_STRUCT__entry(
> +               __field(        __u64,          ns              )
> +               __field(        bool,           waited          )
> +       ),
> +
> +       TP_fast_assign(
> +               __entry->ns             = ns;
> +               __entry->waited         = waited;
> +       ),
> +
> +       TP_printk("%s time %lld ns",
> +                 __entry->waited ? "wait" : "poll",
> +                 __entry->ns)
> +);
> +
>  #if defined(CONFIG_HAVE_KVM_IRQFD)
>  TRACE_EVENT(kvm_set_irq,
>         TP_PROTO(unsigned int gsi, int level, int irq_source_id),
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 0c281760a1c5..32449e0e9aa8 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -66,6 +66,9 @@
>  MODULE_AUTHOR("Qumranet");
>  MODULE_LICENSE("GPL");
>
> +unsigned int halt_poll_ns = 0;
> +module_param(halt_poll_ns, uint, S_IRUGO | S_IWUSR);
> +
>  /*
>   * Ordering of locks:
>   *
> @@ -1813,29 +1816,60 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
>  }
>  EXPORT_SYMBOL_GPL(mark_page_dirty);
>
> +static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
> +{
> +       if (kvm_arch_vcpu_runnable(vcpu)) {
> +               kvm_make_request(KVM_REQ_UNHALT, vcpu);
> +               return -EINTR;
> +       }
> +       if (kvm_cpu_has_pending_timer(vcpu))
> +               return -EINTR;
> +       if (signal_pending(current))
> +               return -EINTR;
> +
> +       return 0;
> +}
> +
>  /*
>   * The vCPU has executed a HLT instruction with in-kernel mode enabled.
>   */
>  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));
> +       }
>
>         for (;;) {
>                 prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
>
> -               if (kvm_arch_vcpu_runnable(vcpu)) {
> -                       kvm_make_request(KVM_REQ_UNHALT, vcpu);
> -                       break;
> -               }
> -               if (kvm_cpu_has_pending_timer(vcpu))
> -                       break;
> -               if (signal_pending(current))
> +               if (kvm_vcpu_check_block(vcpu) < 0)
>                         break;
>
> +               waited = true;
>                 schedule();
>         }
>
>         finish_wait(&vcpu->wq, &wait);
> +       cur = ktime_get();
> +
> +out:
> +       trace_kvm_vcpu_wakeup(ktime_to_ns(cur) - ktime_to_ns(start), waited);
>  }
>  EXPORT_SYMBOL_GPL(kvm_vcpu_block);
>
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ