Date: Tue, 30 Mar 2021 15:43:06 +0200 From: Mathieu Chouquet-Stringer <me+freebsd@mathieu.digital> To: freebsd-fs@freebsd.org, freebsd-current@freebsd.org Cc: bryanv@freebsd.org Subject: Re: Scrub incredibly slow with 13.0-RC3 (as well as RC1 & 2) Message-ID: <YGMq6i%2BUIhNANHXW@weirdfishes> In-Reply-To: <YF9YE282kOWN7ubr@weirdfishes> References: <YFhuxr0qRzchA7x8@weirdfishes> <YF8eL0dmSh6H8HX2@ceres.zyxst.net> <YF9YE282kOWN7ubr@weirdfishes>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello,
TL;DR: we need kvmclock support in FreeBSD
On Sat, Mar 27, 2021 at 05:06:43PM +0100, Mathieu Chouquet-Stringer wrote:
> TL;DR: read_max makes no difference, don't use hpet with KVM, use
> scan_legacy.
I ended up resurrecting a set of patches created by Bryan [1] to add support
for kvmclock because this whole slow zfs thing is incredibly dependent on the
timecounter infrastructure. I inlined below the patch I'm using at the moment.
And results are mind blowing.
I can scrub my zroot pool in 10s instead of 1m35s...
And I started scrubbing my raidz2 pool 30 minutes ago and it's telling
me I have about 15 hours left. The best time I got before that was 23:04:17
(and that was with scan_legacy=1 which was around 3 hours quicker than the same
scrub withscan_legacy=0)...
scan: scrub in progress since Tue Mar 30 15:00:03 2021
962G scanned at 541M/s, 810G issued at 456M/s, 24.4T total
0B repaired, 3.24% done, 15:05:39 to go
And that's with vfs.zfs.scan_legacy == 0 (the default). So it seems the scan
process uses timing information a lot more than the legacy one. And
that's what I saw in the FlameGraph when I stupidly added hpet support in KVM:
I was seeing a lot of calls to hpet_get_timecount.
So the question is: what is needed to merge that kvmclock thing in the kernel?
Regards,
[1] https://people.freebsd.org/~bryanv/patches/kvm_clock-1.patch
---
sys/conf/files.x86 | 1 +
sys/x86/include/kvm.h | 49 +++++++++++++++++++++++
sys/x86/x86/kvm_clock.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 182 insertions(+)
diff --git a/sys/conf/files.x86 b/sys/conf/files.x86
index f51392d0614..bcf53fcf1f6 100644
--- a/sys/conf/files.x86
+++ b/sys/conf/files.x86
@@ -335,6 +335,7 @@ x86/x86/dump_machdep.c standard
x86/x86/fdt_machdep.c optional fdt
x86/x86/identcpu.c standard
x86/x86/intr_machdep.c standard
+x86/x86/kvm_clock.c standard
x86/x86/legacy.c standard
x86/x86/mca.c standard
x86/x86/x86_mem.c optional mem
diff --git a/sys/x86/include/kvm.h b/sys/x86/include/kvm.h
new file mode 100644
index 00000000000..dc26822ed1c
--- /dev/null
+++ b/sys/x86/include/kvm.h
@@ -0,0 +1,49 @@
+/*-
+ * Copyright (c) 2014 Bryan Venteicher <bryanv@FreeBSD.org>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+#ifndef _X86_KVM_H_
+#define _X86_KVM_H_
+
+#define KVM_CPUID_FEATURES_LEAF 0x40000001
+
+#define KVM_FEATURE_CLOCKSOURCE 0x00000001
+#define KVM_FEATURE_CLOCKSOURCE2 0x00000008
+
+/* Deprecated: for the CLOCKSOURCE feature. */
+#define KVM_MSR_WALL_CLOCK 0x11
+#define KVM_MSR_SYSTEM_TIME 0x12
+
+#define KVM_MSR_WALL_CLOCK_NEW 0x4b564d00
+#define KVM_MSR_SYSTEM_TIME_NEW 0x4b564d01
+
+int kvm_paravirt_supported(void);
+uint32_t kvm_get_features(void);
+
+uint64_t kvm_clock_tsc_freq(void);
+
+#endif /* !_X86_KVM_H_ */
diff --git a/sys/x86/x86/kvm_clock.c b/sys/x86/x86/kvm_clock.c
new file mode 100644
index 00000000000..f16aa88c695
--- /dev/null
+++ b/sys/x86/x86/kvm_clock.c
@@ -0,0 +1,132 @@
+/*-
+ * Copyright (c) 2014 Bryan Venteicher <bryanv@FreeBSD.org>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/param.h>
+#include <sys/kernel.h>
+#include <sys/limits.h>
+#include <sys/systm.h>
+#include <sys/pcpu.h>
+#include <sys/smp.h>
+#include <sys/timetc.h>
+
+#include <vm/vm.h>
+#include <vm/pmap.h>
+
+#include <machine/pvclock.h>
+#include <x86/kvm.h>
+#include <x86/x86_var.h>
+
+static u_int kvm_clock_get_timecounter(struct timecounter *);
+static void kvm_clock_pcpu_system_time(void *);
+
+DPCPU_DEFINE(struct pvclock_vcpu_time_info, kvm_clock_vcpu_time_info);
+
+static struct timecounter kvm_clock_timecounter = {
+ kvm_clock_get_timecounter,
+ NULL,
+ ~0u,
+ 1000000000ULL,
+ "KVMCLOCK",
+ 1000,
+};
+
+static int kvm_clock_registered;
+static uint32_t kvm_clock_wall_clock_msr;
+static uint32_t kvm_clock_system_time_msr;
+
+uint64_t
+kvm_clock_tsc_freq(void)
+{
+ struct pvclock_vcpu_time_info *ti;
+ uint64_t freq;
+
+ critical_enter();
+ ti = DPCPU_PTR(kvm_clock_vcpu_time_info);
+ freq = pvclock_tsc_freq(ti);
+ critical_exit();
+
+ return (freq);
+}
+
+static u_int
+kvm_clock_get_timecounter(struct timecounter *tc)
+{
+ struct pvclock_vcpu_time_info *ti;
+ uint64_t time;
+
+ critical_enter();
+ ti = DPCPU_PTR(kvm_clock_vcpu_time_info);
+ time = pvclock_get_timecount(ti);
+ critical_exit();
+
+ return (time & UINT_MAX);
+}
+
+static void
+kvm_clock_pcpu_system_time(void *arg)
+{
+ uint64_t data;
+ int enable;
+
+ enable = *(int *) arg;
+
+ if (enable != 0)
+ data = vtophys(DPCPU_PTR(kvm_clock_vcpu_time_info)) | 1;
+ else
+ data = 0;
+
+ wrmsr(kvm_clock_system_time_msr, data);
+}
+
+static void
+kvm_clock_init(void)
+{
+ if (vm_guest != VM_GUEST_KVM)
+ return;
+
+ if (hv_base > 0 == 0)
+ return;
+
+ if (hv_high & KVM_FEATURE_CLOCKSOURCE2) {
+ kvm_clock_wall_clock_msr = KVM_MSR_WALL_CLOCK_NEW;
+ kvm_clock_system_time_msr = KVM_MSR_SYSTEM_TIME_NEW;
+ } else if (hv_high & KVM_FEATURE_CLOCKSOURCE) {
+ kvm_clock_wall_clock_msr = KVM_MSR_WALL_CLOCK;
+ kvm_clock_system_time_msr = KVM_MSR_SYSTEM_TIME;
+ } else
+ return;
+
+ kvm_clock_registered = 1;
+ smp_rendezvous(smp_no_rendezvous_barrier, kvm_clock_pcpu_system_time,
+ smp_no_rendezvous_barrier, &kvm_clock_registered);
+
+ tc_init(&kvm_clock_timecounter);
+}
+
+SYSINIT(kvm_clock, SI_SUB_SMP, SI_ORDER_ANY, kvm_clock_init, NULL);
--
Mathieu Chouquet-Stringer
The sun itself sees not till heaven clears.
-- William Shakespeare --
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YGMq6i%2BUIhNANHXW>