| Message ID | 20170913143958.86385-1-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | target-i386: enable kvm_pv_unhalt by default | expand |
On Wed, Sep 13, 2017 at 04:39:58PM +0200, Alexander Graf wrote: > Commit f010bc643a (target-i386: add feature kvm_pv_unhalt) introduced the > kvm_pv_unhalt feature but didn't enable it by default. > > Without kvm_pv_unhalt we see a measurable degradation in scheduling > performance, so enabling it by default does make sense IMHO. This patch > just flips it to default to on by default. > > [With kvm_pv_unhalt disabled] > $ perf bench sched messaging -l 10000 > Total time: 8.573 [sec] > > [With kvm_pv_unhalt enabled] > $ perf bench sched messaging -l 10000 > Total time: 4.416 [sec] > > Signed-off-by: Alexander Graf <agraf@suse.de> > > --- > > Let's ask everyone who was involved back then whether this is a feature > that is good to enable by default. My measurements imply so, but who knows... > > Also, I'd kindly like to ask for guidance on how to make this reasonably > well backwards compatible. I assume we only want to flip the default in newer > machine models? If so, how? There are two things that need to be taken care of: * Guest ABI compatibility: if running an older machine-type, the feature needs to remain disabled. * machine-type runnability: libvirt doesn't expect an existing VM to become not runnable on the same host if changing only the machine-type. This can happen if the new machine-type enables a feature that's not supported by the host kernel. Guest ABI compatibility is relatively easy to handle: see how we deal with kvm-pv-eoi on pc-1.2. Moving the machine-type-specific kvm-defaults to PCMachineClass (instead of introducing new pc_compat_*() functions) would be interesting, though; I will take a look and try to implement that. machine-type runnability is trickier: it's a problem in theory if people can be running old kernels that didn't support the feature, but may be not a problem in practice if the feature is so old that everybody running a recent QEMU is also running a recent enough kernel. In either case, the problem still needs to be addressed somehow; just documenting the minimum kernel version QEMU needs would probably be enough. For reference, kvm_pv_unhalt was introduced in Linux v3.12-rc1. > --- > target/i386/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 69676e13e1..c58f4ab24f 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -1581,6 +1581,7 @@ static PropValue kvm_default_props[] = { > { "kvm-asyncpf", "on" }, > { "kvm-steal-time", "on" }, > { "kvm-pv-eoi", "on" }, > + { "kvm-pv-unhalt", "on" }, > { "kvmclock-stable-bit", "on" }, > { "x2apic", "on" }, > { "acpi", "off" }, > -- > 2.12.3 > -- Eduardo
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 69676e13e1..c58f4ab24f 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1581,6 +1581,7 @@ static PropValue kvm_default_props[] = { { "kvm-asyncpf", "on" }, { "kvm-steal-time", "on" }, { "kvm-pv-eoi", "on" }, + { "kvm-pv-unhalt", "on" }, { "kvmclock-stable-bit", "on" }, { "x2apic", "on" }, { "acpi", "off" },
Commit f010bc643a (target-i386: add feature kvm_pv_unhalt) introduced the kvm_pv_unhalt feature but didn't enable it by default. Without kvm_pv_unhalt we see a measurable degradation in scheduling performance, so enabling it by default does make sense IMHO. This patch just flips it to default to on by default. [With kvm_pv_unhalt disabled] $ perf bench sched messaging -l 10000 Total time: 8.573 [sec] [With kvm_pv_unhalt enabled] $ perf bench sched messaging -l 10000 Total time: 4.416 [sec] Signed-off-by: Alexander Graf <agraf@suse.de> --- Let's ask everyone who was involved back then whether this is a feature that is good to enable by default. My measurements imply so, but who knows... Also, I'd kindly like to ask for guidance on how to make this reasonably well backwards compatible. I assume we only want to flip the default in newer machine models? If so, how? --- target/i386/cpu.c | 1 + 1 file changed, 1 insertion(+) -- 2.12.3