| Message ID | 20240821095127.45d17b19@gandalf.local.home (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] KVM: Remove HIGH_RES_TIMERS dependency | expand |
On Wed, Aug 21, 2024, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency > to high resolution timers with the comment: > > KVM lapic timer and tsc deadline timer based on hrtimer, > setting a leftmost node to rb tree and then do hrtimer reprogram. > If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram > do nothing and then make kvm lapic timer and tsc deadline timer fail. > > That was back in 2012, where hrtimer_start_range_ns() would do the > reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with > high resolution timers disabled, this did not work. But a lot has changed > in the last 12 years. > > For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on > timer enqueue") modifies __hrtimer_start_range_ns() to work with low res > timers. There's been lots of other changes that make low res work. > > I added this change to my main server that runs all my VMs (my mail > server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the > system has been running just fine for over a month. > > ChromeOS has tested this before as well, and it hasn't seen any issues with > running KVM with high res timers disabled. Can you provide some background on why this is desirable, and what the effective tradeoffs are? Mostly so that future users have some chance of making an informed decision. Realistically, anyone running with HIGH_RES_TIMERS=n is likely already aware of the tradeoffs, but it'd be nice to capture the info here. > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > arch/x86/kvm/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index 472a1537b7a9..c65127e796a9 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -19,7 +19,6 @@ if VIRTUALIZATION > > config KVM > tristate "Kernel-based Virtual Machine (KVM) support" > - depends on HIGH_RES_TIMERS I did some very basic testing and nothing exploded on me either. So long as nothing in the host catches fire, I don't see a good reason to make high resolution timers a hard requirement. My only concern is that this could, at least in theory, result in people unintentionally breaking their setups, but that seems quite unlikely. One thought would be to require the user to enable EXPERT in order to break the HIGH_RES_TIMERS dependency. In practice, I doubt that will be much of a deterrent since (IIRC) many distros ship with EXPERT=y. But it would at least document that using KVM x86 without HIGH_RES_TIMERS may come with caveats. E.g. depends on HIGH_RES_TIMERS || EXPERT
On Wed, Aug 28, 2024 at 12:34:26PM -0700, Sean Christopherson wrote: > On Wed, Aug 21, 2024, Steven Rostedt wrote: > > From: Steven Rostedt <rostedt@goodmis.org> > > > > Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency > > to high resolution timers with the comment: > > > > KVM lapic timer and tsc deadline timer based on hrtimer, > > setting a leftmost node to rb tree and then do hrtimer reprogram. > > If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram > > do nothing and then make kvm lapic timer and tsc deadline timer fail. > > > > That was back in 2012, where hrtimer_start_range_ns() would do the > > reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with > > high resolution timers disabled, this did not work. But a lot has changed > > in the last 12 years. > > > > For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on > > timer enqueue") modifies __hrtimer_start_range_ns() to work with low res > > timers. There's been lots of other changes that make low res work. > > > > I added this change to my main server that runs all my VMs (my mail > > server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the > > system has been running just fine for over a month. > > > > ChromeOS has tested this before as well, and it hasn't seen any issues with > > running KVM with high res timers disabled. > > Can you provide some background on why this is desirable, and what the effective > tradeoffs are? Mostly so that future users have some chance of making an > informed decision. Realistically, anyone running with HIGH_RES_TIMERS=n is likely > already aware of the tradeoffs, but it'd be nice to capture the info here. We have found that disabling HR timers saves power without degrading the user experience too much. > > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > --- > > arch/x86/kvm/Kconfig | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > > index 472a1537b7a9..c65127e796a9 100644 > > --- a/arch/x86/kvm/Kconfig > > +++ b/arch/x86/kvm/Kconfig > > @@ -19,7 +19,6 @@ if VIRTUALIZATION > > > > config KVM > > tristate "Kernel-based Virtual Machine (KVM) support" > > - depends on HIGH_RES_TIMERS > > I did some very basic testing and nothing exploded on me either. So long as > nothing in the host catches fire, I don't see a good reason to make high resolution > timers a hard requirement. > > My only concern is that this could, at least in theory, result in people > unintentionally breaking their setups, but that seems quite unlikely. > > One thought would be to require the user to enable EXPERT in order to break the > HIGH_RES_TIMERS dependency. In practice, I doubt that will be much of a deterrent > since (IIRC) many distros ship with EXPERT=y. But it would at least document that > using KVM x86 without HIGH_RES_TIMERS may come with caveats. E.g. > > depends on HIGH_RES_TIMERS || EXPERT This sounds like a good compromise. -- Suleiman
On 9/4/24 09:35, Suleiman Souhlal wrote: > On Wed, Aug 28, 2024 at 12:34:26PM -0700, Sean Christopherson wrote: >> On Wed, Aug 21, 2024, Steven Rostedt wrote: >>> From: Steven Rostedt <rostedt@goodmis.org> >>> >>> Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency >>> to high resolution timers with the comment: >>> >>> KVM lapic timer and tsc deadline timer based on hrtimer, >>> setting a leftmost node to rb tree and then do hrtimer reprogram. >>> If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram >>> do nothing and then make kvm lapic timer and tsc deadline timer fail. >>> >>> That was back in 2012, where hrtimer_start_range_ns() would do the >>> reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with >>> high resolution timers disabled, this did not work. But a lot has changed >>> in the last 12 years. >>> >>> For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on >>> timer enqueue") modifies __hrtimer_start_range_ns() to work with low res >>> timers. There's been lots of other changes that make low res work. >>> >>> I added this change to my main server that runs all my VMs (my mail >>> server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the >>> system has been running just fine for over a month. >>> >>> ChromeOS has tested this before as well, and it hasn't seen any issues with >>> running KVM with high res timers disabled. >> >> Can you provide some background on why this is desirable, and what the effective >> tradeoffs are? Mostly so that future users have some chance of making an >> informed decision. Realistically, anyone running with HIGH_RES_TIMERS=n is likely >> already aware of the tradeoffs, but it'd be nice to capture the info here. > > We have found that disabling HR timers saves power without degrading > the user experience too much. This might have some issues on guests that do not support kvmclock, because they rely on precise delivery of periodic timers to keep their clock running. This can be the APIC timer (provided by the kernel), the RTC (provided by userspace), or the i8254 (choice of kernel/userspace). These guests are few and far between these days, and in the case of the APIC timer + Intel hosts we can use the preemption timer (which is TSC-based and has better latency _and_ accuracy). Furthermore, only x86 is requiring CONFIG_HIGH_RES_TIMERS, so it's probably just excessive care and we can even apply Steven's patch as is. Alternatively, the "depends on HIGH_RES_TIMERS || EXPERT" could be added to virt/kvm. Or a pr_warn could be added to kvm_init if HIGH_RES_TIMERS are not enabled. But in general, it seems that Linux has a laissez-faire approach to disabling CONFIG_HIGH_RES_TIMERS - there must be other code in the kernel (maybe sound/?) that is relying on having high-enough HZ or hrtimers but that's not documented anywhere. I don't have an objection to doing the same in KVM, honestly, since most systems are running CONFIG_HIGH_RES_TIMERS anyway. Paolo
On Wed, Sep 04, 2024 at 03:25:51PM +0200, Paolo Bonzini wrote: > On 9/4/24 09:35, Suleiman Souhlal wrote: > > On Wed, Aug 28, 2024 at 12:34:26PM -0700, Sean Christopherson wrote: > > > On Wed, Aug 21, 2024, Steven Rostedt wrote: > > > > From: Steven Rostedt <rostedt@goodmis.org> > > > > > > > > Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency > > > > to high resolution timers with the comment: > > > > > > > > KVM lapic timer and tsc deadline timer based on hrtimer, > > > > setting a leftmost node to rb tree and then do hrtimer reprogram. > > > > If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram > > > > do nothing and then make kvm lapic timer and tsc deadline timer fail. > > > > > > > > That was back in 2012, where hrtimer_start_range_ns() would do the > > > > reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with > > > > high resolution timers disabled, this did not work. But a lot has changed > > > > in the last 12 years. > > > > > > > > For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on > > > > timer enqueue") modifies __hrtimer_start_range_ns() to work with low res > > > > timers. There's been lots of other changes that make low res work. > > > > > > > > I added this change to my main server that runs all my VMs (my mail > > > > server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the > > > > system has been running just fine for over a month. > > > > > > > > ChromeOS has tested this before as well, and it hasn't seen any issues with > > > > running KVM with high res timers disabled. > > > > > > Can you provide some background on why this is desirable, and what the effective > > > tradeoffs are? Mostly so that future users have some chance of making an > > > informed decision. Realistically, anyone running with HIGH_RES_TIMERS=n is likely > > > already aware of the tradeoffs, but it'd be nice to capture the info here. > > > > We have found that disabling HR timers saves power without degrading > > the user experience too much. > > This might have some issues on guests that do not support kvmclock, because > they rely on precise delivery of periodic timers to keep their clock > running. This can be the APIC timer (provided by the kernel), the RTC > (provided by userspace), or the i8254 (choice of kernel/userspace). > > These guests are few and far between these days, and in the case of the APIC > timer + Intel hosts we can use the preemption timer (which is TSC-based and > has better latency _and_ accuracy). Furthermore, only x86 is requiring > CONFIG_HIGH_RES_TIMERS, so it's probably just excessive care and we can even > apply Steven's patch as is. > > Alternatively, the "depends on HIGH_RES_TIMERS || EXPERT" could be added to > virt/kvm. Or a pr_warn could be added to kvm_init if HIGH_RES_TIMERS are > not enabled. > > But in general, it seems that Linux has a laissez-faire approach to > disabling CONFIG_HIGH_RES_TIMERS - there must be other code in the kernel > (maybe sound/?) that is relying on having high-enough HZ or hrtimers but > that's not documented anywhere. I don't have an objection to doing the same > in KVM, honestly, since most systems are running CONFIG_HIGH_RES_TIMERS > anyway. I'm not sure how much my opinion on the matter counts, but I would be more than happy to see Steven's current patch get applied as is. It would make our (ChromeOS) life a bit simpler. -- Suleiman
> Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency > to high resolution timers with the comment: > > KVM lapic timer and tsc deadline timer based on hrtimer, > setting a leftmost node to rb tree and then do hrtimer reprogram. > If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram > do nothing and then make kvm lapic timer and tsc deadline timer fail. > > That was back in 2012, where hrtimer_start_range_ns() would do the > reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with > high resolution timers disabled, this did not work. But a lot has changed > in the last 12 years. > > For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on > timer enqueue") modifies __hrtimer_start_range_ns() to work with low res > timers. There's been lots of other changes that make low res work. > > I added this change to my main server that runs all my VMs (my mail > server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the > system has been running just fine for over a month. > > ChromeOS has tested this before as well, and it hasn't seen any issues with > running KVM with high res timers disabled. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> Queued, thanks. Paolo
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 472a1537b7a9..c65127e796a9 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -19,7 +19,6 @@ if VIRTUALIZATION config KVM tristate "Kernel-based Virtual Machine (KVM) support" - depends on HIGH_RES_TIMERS depends on X86_LOCAL_APIC select KVM_COMMON select KVM_GENERIC_MMU_NOTIFIER