On Mon, Nov 04, 2013 at 10:44:43PM +0200, Gleb Natapov wrote: > On Mon, Nov 04, 2013 at 10:33:57PM +0200, Michael S. Tsirkin wrote: > > On Mon, Nov 04, 2013 at 10:13:39PM +0200, Gleb Natapov wrote: > > > On Mon, Nov 04, 2013 at 10:11:33PM +0200, Michael S. Tsirkin wrote: > > > > On Thu, Oct 31, 2013 at 09:48:08AM +0200, Gleb Natapov wrote: > > > > > On Thu, Oct 31, 2013 at 02:21:46AM +0200, Michael S. Tsirkin wrote: > > > > > > commit 8bf00a529967dafbbb210b377c38a15834d1e979: > > > > > > " KVM: VMX: add support for switching of PERF_GLOBAL_CTRL " was > > > > > > as far as I can tell supposed to bring about performance improvement > > > > > > on hardware that supports it? > > > > > No, it (and commits after it) supposed to fix a bug which it did. > > > > > > > > > > > Instead it seems to make the typical case (not running guest > > > > > > under perf) a bit slower than it used to be. > > > > > > the cost of VMexit goes up by about 50 cycles > > > > > > on sandy bridge where the optimization in question > > > > > > actually is activated. > > > > > > > > > > > You seams to be confused. 8bf00a529967dafbbb210 adds support for special > > > > > PERF_GLOBAL_CTRL switching, but does not add code to switch anything, > > > > > so the commit itself is a nop. > > > > > > > > It does add code to add_atomic_switch_msr. > > > > > > > So what? You do not read what I wrote. > > > > > > It's simple: if I revert 8bf00a529967dafbbb210 then exit latency > > is reduced. > > You seem to tell me it should be a nop, but in practice it isn't. > > > > No, if you read below I am saying that it looks like you are claiming that > generic msr switch mechanism is faster and I am not buying that. If you > believe this to be the case ask Intel for explanation. Your claim about > "not running guest under perf" is even stranger since in this case no msr > switch should happen regardless of the aforementioned commit (unless guest > or host runs nmi watchdog, but then switch will happen no matter if perf > is running, so again not running guest under perf" does not make sense). > So, in short, you do not really know where the slow down is coming > from. That's true. > My guess is that it comes from the fact that we unconditionally > call clear_atomic_switch_msr() in atomic_switch_perf_msrs(), but then > fix that instead of reverting the patch. We can try, but reverting is much simpler, it removes code instead of adding code. Do you know which workload is actually improved by 8bf00a529967dafbbb210? > > > > > Next commit d7cd97964ba6d70c5 > > > > > uses add_atomic_switch_msr()/clear_atomic_switch_msr() > > > > > to switch PERF_GLOBAL_CTRL, but it does not depend on > > > > > VM_(ENTRY|EXIT)_LOAD_IA32_PERF_GLOBAL_CTRL support which previous > > > > > patch added, if the support is not there the switching will use > > > > > another mechanism which is even slower. So MSR is switched no matter > > > > > if PERF_GLOBAL_CTRL is enabled or not. If you saying that using > > > > > VM_(ENTRY|EXIT)_LOAD_IA32_PERF_GLOBAL_CTRL is slower than using generic > > > > > vmentry MSR switching then I pretty much doubt it since the only purpose > > > > > of special VM_(ENTRY|EXIT)_LOAD_IA32_PERF_GLOBAL_CTRL is to be faster > > > > > then general mechanism. > > > > > > > > > > -- > > > > > Gleb. > > > > > -- > > > > > To unsubscribe from this list: send the line "unsubscribe kvm" in > > > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > > -- > > > Gleb. > > -- > Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
