* [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
@ 2023-11-01 8:25 何琼
2023-11-01 10:06 ` Andrew Jones
2023-11-01 11:04 ` Alexandru Elisei
0 siblings, 2 replies; 13+ messages in thread
From: 何琼 @ 2023-11-01 8:25 UTC (permalink / raw)
To: kvm
[-- Attachment #1.1: Type: text/plain, Size: 5841 bytes --]
hi,
This patch mainly includes the following content.
Reducing the impact of the cntvct_el0 register and isb() operation on microbenchmark test results to improve testing accuracy and reduce latency in test results.
Test in kunpeng920,
Test results before applying the patch:
[root@localhost tests]# ./micro-bench
BUILD_HEAD=767629ca
Test marked not to be run by default, are you sure (y/N)? y
timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/libexec/qemu-kvm -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.y4c4YHIprP -smp 2 # -initrd /tmp/tmp.KLLmjTuq2d
Timer Frequency 100000000 Hz (Output in microseconds)
name total ns avg ns
--------------------------------------------------------------------------------------------
hvc 26774980.0 408.0
mmio_read_user 151183350.0 2306.0
mmio_read_vgic 41849830.0 638.0
eoi 1735610.0 26.0
ipi 111260770.0 1697.0
ipi_hw test skipped
lpi 142124570.0 2168.0
timer_10ms 466660.0 1822.0
EXIT: STATUS=1
PASS micro-bench
[root@localhost tests]#
Test results after applying the patch:
[root@localhost kvm-unit-tests]# cd tests/
[root@localhost tests]# ./micro-bench
BUILD_HEAD=767629ca
Test marked not to be run by default, are you sure (y/N)? y
timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/libexec/qemu-kvm -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.FiBID6KLxB -smp 2 # -initrd /tmp/tmp.oSKZeugleF
Timer Frequency 100000000 Hz (Output in microseconds)
name total ns avg ns
--------------------------------------------------------------------------------------------
hvc 26721040.0 407.0
mmio_read_user 150824560.0 2301.0
mmio_read_vgic 41845380.0 638.0
eoi 1109180.0 16.0
ipi 106062150.0 1618.0
ipi_hw test skipped
lpi 141700760.0 2162.0
timer_10ms 470870.0 1839.0
EXIT: STATUS=1
PASS micro-bench
[root@localhost tests]#
Test in phytium S2500,
Test results before applying the patch:
[root@primecontroller tests]# ./micro-bench
BUILD_HEAD=518cd47c
Test marked not to be run by default, are you sure (y/N)? y
timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/local/bin/qemu-system-aarch64 -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.lrJJqSuLmN -smp 2 # -initrd /tmp/tmp.s18C3k2jfO
Timer Frequency 50000000 Hz (Output in microseconds)
name total ns avg ns
--------------------------------------------------------------------------------------------
hvc 100668780.0 1536.0
mmio_read_user 472806800.0 7214.0
mmio_read_vgic 140912320.0 2150.0
eoi 2972280.0 45.0
ipi 326332780.0 4979.0
ipi_hw test skipped
lpi 359226600.0 5481.0
timer_10ms 1271960.0 4968.0
EXIT: STATUS=1
PASS micro-bench
[root@primecontroller tests]#
Test results after applying the patch:
[root@primecontroller tests]# ./micro-bench
BUILD_HEAD=518cd47c
Test marked not to be run by default, are you sure (y/N)? y
timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/local/bin/qemu-system-aarch64 -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.IsEtcs1W1g -smp 2 # -initrd /tmp/tmp.885IpeoGw4
Timer Frequency 50000000 Hz (Output in microseconds)
name total ns avg ns
--------------------------------------------------------------------------------------------
hvc 99490080.0 1518.0
mmio_read_user 474781300.0 7244.0
mmio_read_vgic 140470760.0 2143.0
eoi 1693260.0 25.0
ipi 323551200.0 4936.0
ipi_hw test skipped
lpi 355690620.0 5427.0
timer_10ms 1318540.0 5150.0
EXIT: STATUS=1
PASS micro-bench
[root@primecontroller tests]#
[-- Attachment #1.2: Type: text/html, Size: 14491 bytes --]
[-- Attachment #2: 0001-arm64-microbench-Move-the-read-of-the-count-register.patch --]
[-- Type: text/plain, Size: 1245 bytes --]
From 518cd47c33fce60ef86ed66dfa9e904b66499933 Mon Sep 17 00:00:00 2001
From: heqiong <heqiong1557@phytium.com.cn>
Date: Wed, 1 Nov 2023 15:06:28 +0800
Subject: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count
register and the ISB operation out of the while loop
Reducing the impact of the cntvct_el0 register and isb() operation
on microbenchmark test results to improve testing accuracy and reduce
latency in test results.
---
arm/micro-bench.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fbe59d03..ee5b9ca0 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -346,17 +346,18 @@ static void loop_test(struct exit_test *test)
}
}
+ start = read_sysreg(cntpct_el0);
+ isb();
while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
- isb();
- start = read_sysreg(cntvct_el0);
test->exec();
- isb();
- end = read_sysreg(cntvct_el0);
ntimes++;
- total_ticks += (end - start);
- ticks_to_ns_time(total_ticks, &total_ns);
}
+ isb();
+ end = read_sysreg(cntpct_el0);
+
+ total_ticks = end - start;
+ ticks_to_ns_time(total_ticks, &total_ns);
if (test->post) {
test->post(ntimes, &total_ticks);
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-01 8:25 [kvm-unit-tests " 何琼
@ 2023-11-01 10:06 ` Andrew Jones
2023-11-01 11:04 ` Alexandru Elisei
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Jones @ 2023-11-01 10:06 UTC (permalink / raw)
To: 何琼; +Cc: kvm
Hi,
It's quite hard to read this mail because of the formatting. Also, please
do not submit patches as attachments. Despite it being an attachment, I
took a look and it looks fine, but it's missing a signed-off-by.
Please resubmit with this mail as a cover-letter and formatted properly
with line wraps, etc. and the patch, as it's own message, in-reply-to the
cover letter. You may want to use git-send-email.
Thanks,
drew
On Wed, Nov 01, 2023 at 04:25:39PM +0800, 何琼 wrote:
> hi,
>
> This patch mainly includes the following content.
>
> Reducing the impact of the cntvct_el0 register and isb() operation on microbenchmark test results to improve testing accuracy and reduce latency in test results.
>
>
>
>
>
>
>
> Test in kunpeng920,
>
> Test results before applying the patch:
>
> [root@localhost tests]# ./micro-bench
>
>
> BUILD_HEAD=767629ca
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/libexec/qemu-kvm -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.y4c4YHIprP -smp 2 # -initrd /tmp/tmp.KLLmjTuq2d
>
>
> Timer Frequency 100000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 26774980.0 408.0
>
>
> mmio_read_user 151183350.0 2306.0
>
>
> mmio_read_vgic 41849830.0 638.0
>
>
> eoi 1735610.0 26.0
>
>
> ipi 111260770.0 1697.0
>
>
> ipi_hw test skipped
>
>
> lpi 142124570.0 2168.0
>
>
> timer_10ms 466660.0 1822.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@localhost tests]#
>
>
>
>
>
> Test results after applying the patch:
>
> [root@localhost kvm-unit-tests]# cd tests/
>
>
> [root@localhost tests]# ./micro-bench
>
>
> BUILD_HEAD=767629ca
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/libexec/qemu-kvm -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.FiBID6KLxB -smp 2 # -initrd /tmp/tmp.oSKZeugleF
>
>
> Timer Frequency 100000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 26721040.0 407.0
>
>
> mmio_read_user 150824560.0 2301.0
>
>
> mmio_read_vgic 41845380.0 638.0
>
>
> eoi 1109180.0 16.0
>
>
> ipi 106062150.0 1618.0
>
>
> ipi_hw test skipped
>
>
> lpi 141700760.0 2162.0
>
>
> timer_10ms 470870.0 1839.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@localhost tests]#
>
>
>
>
>
>
>
>
> Test in phytium S2500,
>
> Test results before applying the patch:
>
> [root@primecontroller tests]# ./micro-bench
>
>
> BUILD_HEAD=518cd47c
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/local/bin/qemu-system-aarch64 -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.lrJJqSuLmN -smp 2 # -initrd /tmp/tmp.s18C3k2jfO
>
>
> Timer Frequency 50000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 100668780.0 1536.0
>
>
> mmio_read_user 472806800.0 7214.0
>
>
> mmio_read_vgic 140912320.0 2150.0
>
>
> eoi 2972280.0 45.0
>
>
> ipi 326332780.0 4979.0
>
>
> ipi_hw test skipped
>
>
> lpi 359226600.0 5481.0
>
>
> timer_10ms 1271960.0 4968.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@primecontroller tests]#
>
>
>
>
>
>
>
>
> Test results after applying the patch:
>
> [root@primecontroller tests]# ./micro-bench
>
>
> BUILD_HEAD=518cd47c
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/local/bin/qemu-system-aarch64 -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.IsEtcs1W1g -smp 2 # -initrd /tmp/tmp.885IpeoGw4
>
>
> Timer Frequency 50000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 99490080.0 1518.0
>
>
> mmio_read_user 474781300.0 7244.0
>
>
> mmio_read_vgic 140470760.0 2143.0
>
>
> eoi 1693260.0 25.0
>
>
> ipi 323551200.0 4936.0
>
>
> ipi_hw test skipped
>
>
> lpi 355690620.0 5427.0
>
>
> timer_10ms 1318540.0 5150.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@primecontroller tests]#
>
>
>
>
>
>
>
>
>
> From 518cd47c33fce60ef86ed66dfa9e904b66499933 Mon Sep 17 00:00:00 2001
> From: heqiong <heqiong1557@phytium.com.cn>
> Date: Wed, 1 Nov 2023 15:06:28 +0800
> Subject: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count
> register and the ISB operation out of the while loop
>
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
> ---
> arm/micro-bench.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..ee5b9ca0 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -346,17 +346,18 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> + start = read_sysreg(cntpct_el0);
> + isb();
> while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + isb();
> + end = read_sysreg(cntpct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-01 8:25 [kvm-unit-tests " 何琼
2023-11-01 10:06 ` Andrew Jones
@ 2023-11-01 11:04 ` Alexandru Elisei
1 sibling, 0 replies; 13+ messages in thread
From: Alexandru Elisei @ 2023-11-01 11:04 UTC (permalink / raw)
To: 何琼; +Cc: kvm
Hi,
Comments on the patch itself.
On Wed, Nov 01, 2023 at 04:25:39PM +0800, 何琼 wrote:
> hi,
>
> This patch mainly includes the following content.
>
> Reducing the impact of the cntvct_el0 register and isb() operation on microbenchmark test results to improve testing accuracy and reduce latency in test results.
>
>
>
>
>
>
>
> Test in kunpeng920,
>
> Test results before applying the patch:
>
> [root@localhost tests]# ./micro-bench
>
>
> BUILD_HEAD=767629ca
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/libexec/qemu-kvm -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.y4c4YHIprP -smp 2 # -initrd /tmp/tmp.KLLmjTuq2d
>
>
> Timer Frequency 100000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 26774980.0 408.0
>
>
> mmio_read_user 151183350.0 2306.0
>
>
> mmio_read_vgic 41849830.0 638.0
>
>
> eoi 1735610.0 26.0
>
>
> ipi 111260770.0 1697.0
>
>
> ipi_hw test skipped
>
>
> lpi 142124570.0 2168.0
>
>
> timer_10ms 466660.0 1822.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@localhost tests]#
>
>
>
>
>
> Test results after applying the patch:
>
> [root@localhost kvm-unit-tests]# cd tests/
>
>
> [root@localhost tests]# ./micro-bench
>
>
> BUILD_HEAD=767629ca
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/libexec/qemu-kvm -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.FiBID6KLxB -smp 2 # -initrd /tmp/tmp.oSKZeugleF
>
>
> Timer Frequency 100000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 26721040.0 407.0
>
>
> mmio_read_user 150824560.0 2301.0
>
>
> mmio_read_vgic 41845380.0 638.0
>
>
> eoi 1109180.0 16.0
>
>
> ipi 106062150.0 1618.0
>
>
> ipi_hw test skipped
>
>
> lpi 141700760.0 2162.0
>
>
> timer_10ms 470870.0 1839.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@localhost tests]#
>
>
>
>
>
>
>
>
> Test in phytium S2500,
>
> Test results before applying the patch:
>
> [root@primecontroller tests]# ./micro-bench
>
>
> BUILD_HEAD=518cd47c
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/local/bin/qemu-system-aarch64 -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.lrJJqSuLmN -smp 2 # -initrd /tmp/tmp.s18C3k2jfO
>
>
> Timer Frequency 50000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 100668780.0 1536.0
>
>
> mmio_read_user 472806800.0 7214.0
>
>
> mmio_read_vgic 140912320.0 2150.0
>
>
> eoi 2972280.0 45.0
>
>
> ipi 326332780.0 4979.0
>
>
> ipi_hw test skipped
>
>
> lpi 359226600.0 5481.0
>
>
> timer_10ms 1271960.0 4968.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@primecontroller tests]#
>
>
>
>
>
>
>
>
> Test results after applying the patch:
>
> [root@primecontroller tests]# ./micro-bench
>
>
> BUILD_HEAD=518cd47c
>
>
> Test marked not to be run by default, are you sure (y/N)? y
>
>
> timeout -k 1s --foreground 90s numactl -C 0-3 -m 0 /usr/local/bin/qemu-system-aarch64 -nodefaults -machine virt,gic-version=host -accel kvm -cpu host -device virtio-serial-device -device virtconsole,chardev=ctd -chardev testdev,id=ctd -device pci-testdev -display none -serial stdio -kernel /tmp/tmp.IsEtcs1W1g -smp 2 # -initrd /tmp/tmp.885IpeoGw4
>
>
> Timer Frequency 50000000 Hz (Output in microseconds)
>
>
>
>
>
> name total ns avg ns
>
>
> --------------------------------------------------------------------------------------------
>
>
> hvc 99490080.0 1518.0
>
>
> mmio_read_user 474781300.0 7244.0
>
>
> mmio_read_vgic 140470760.0 2143.0
>
>
> eoi 1693260.0 25.0
>
>
> ipi 323551200.0 4936.0
>
>
> ipi_hw test skipped
>
>
> lpi 355690620.0 5427.0
>
>
> timer_10ms 1318540.0 5150.0
>
>
>
>
>
> EXIT: STATUS=1
>
>
> PASS micro-bench
>
>
> [root@primecontroller tests]#
>
>
>
>
>
>
>
>
>
> From 518cd47c33fce60ef86ed66dfa9e904b66499933 Mon Sep 17 00:00:00 2001
> From: heqiong <heqiong1557@phytium.com.cn>
> Date: Wed, 1 Nov 2023 15:06:28 +0800
> Subject: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count
> register and the ISB operation out of the while loop
>
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
> ---
> arm/micro-bench.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..ee5b9ca0 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -346,17 +346,18 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> + start = read_sysreg(cntpct_el0);
> + isb();
> while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + isb();
> + end = read_sysreg(cntpct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
A few notes:
* The counter that is being used has been changed from the physical to the
virtual counter. Accesses to the physical counter trap on nVHE systems.
That might not be desirable if what you're after is to reduce latency.
* You need an ISB before reading 'start', otherwise the counter read might be
reworded earlier in program order.
* Memory loads or stores are not order by using an ISB. If there are memory
accesses before 'start' is read, you probably want them to be finished before
the counter is read. Similarly, I don't think there are any restrictions on
what the test->exec() function is allowed to do, so there might be memory
accesses as part of the test.
I suggest something like this:
dsb(); // Wait for loads and stores to complete.
isb(); // Order the counter read after the DSB.
start = read_sysreg(cntvct_el0);
isb(); // Order the counter read before the loop.
// No DSB needed, as per ARM DDI 0487J.a, page D11-5991.
/* test loop */
dsb(); // Wait for loads and stores to complete.
isb(); // Order the counter read after the DSB.
end = read_sysreg(cnvct_el0);
// No DSB or ISB needed, as per ARM DDI 0487J.a, page D11-5991.
Thanks,
Alex
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
@ 2023-11-07 6:40 heqiong
2023-11-07 8:40 ` Andrew Jones
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: heqiong @ 2023-11-07 6:40 UTC (permalink / raw)
To: kvm; +Cc: alexandru.elisei, heqiong
Reducing the impact of the cntvct_el0 register and isb() operation
on microbenchmark test results to improve testing accuracy and reduce
latency in test results.
---
arm/micro-bench.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fbe59d03..6b940d56 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -346,17 +346,21 @@ static void loop_test(struct exit_test *test)
}
}
+ dsb(ish);
+ isb();
+ start = read_sysreg(cntpct_el0);
+ isb();
while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
- isb();
- start = read_sysreg(cntvct_el0);
test->exec();
- isb();
- end = read_sysreg(cntvct_el0);
ntimes++;
- total_ticks += (end - start);
- ticks_to_ns_time(total_ticks, &total_ns);
}
+ dsb(ish);
+ isb();
+ end = read_sysreg(cntpct_el0);
+
+ total_ticks = end - start;
+ ticks_to_ns_time(total_ticks, &total_ns);
if (test->post) {
test->post(ntimes, &total_ticks);
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-07 6:40 [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop heqiong
@ 2023-11-07 8:40 ` Andrew Jones
2023-11-16 4:53 ` [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests heqiong
2023-11-07 9:07 ` [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop Alexandru Elisei
2023-11-07 9:51 ` [kvm-unit-tests PATCH " heqiong
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Jones @ 2023-11-07 8:40 UTC (permalink / raw)
To: heqiong; +Cc: kvm, alexandru.elisei
Thanks for submitting the patch more correctly, but there's still two
more problems with the patch submission. The patch summary (email subject)
is too long. It also simply describes the change of implementation, which
is easy to see when looking at the patch. It should instead describe the
purpose of the patch, e.g.
arm64: microbench: Improve measurement accuracy of tests
The second problem is it's missing your signed-off-by (which I think I
pointed out last time too).
Please see [1] for more information about patch formatting. You can also
run the Linux kernel's scripts/checkpatch.pl on the patch to catch these
types of things as well as other code style issues.
[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
Thanks,
drew
On Tue, Nov 07, 2023 at 02:40:06PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
> ---
> arm/micro-bench.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..6b940d56 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -346,17 +346,21 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> + dsb(ish);
> + isb();
> + start = read_sysreg(cntpct_el0);
> + isb();
> while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + dsb(ish);
> + isb();
> + end = read_sysreg(cntpct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-07 6:40 [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop heqiong
2023-11-07 8:40 ` Andrew Jones
@ 2023-11-07 9:07 ` Alexandru Elisei
2023-11-07 9:51 ` [kvm-unit-tests PATCH " heqiong
2 siblings, 0 replies; 13+ messages in thread
From: Alexandru Elisei @ 2023-11-07 9:07 UTC (permalink / raw)
To: heqiong; +Cc: kvm
Hi,
On Tue, Nov 07, 2023 at 02:40:06PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
> ---
> arm/micro-bench.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..6b940d56 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -346,17 +346,21 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> + dsb(ish);
> + isb();
> + start = read_sysreg(cntpct_el0);
I still think it would be interesting to have an explanation why CNTVCT_EL0
was replaced with CNTPCT_EL0.
Thanks,
Alex
> + isb();
> while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + dsb(ish);
> + isb();
> + end = read_sysreg(cntpct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [kvm-unit-tests PATCH 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-07 6:40 [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop heqiong
2023-11-07 8:40 ` Andrew Jones
2023-11-07 9:07 ` [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop Alexandru Elisei
@ 2023-11-07 9:51 ` heqiong
2023-11-07 12:49 ` Alexandru Elisei
2023-11-07 13:53 ` Zenghui Yu
2 siblings, 2 replies; 13+ messages in thread
From: heqiong @ 2023-11-07 9:51 UTC (permalink / raw)
To: kvm; +Cc: alexandru.elisei, heqiong
Reducing the impact of the cntvct_el0 register and isb() operation
on microbenchmark test results to improve testing accuracy and reduce
latency in test results.
---
arm/micro-bench.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fbe59d03..65f4c4dd 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -346,17 +346,21 @@ static void loop_test(struct exit_test *test)
}
}
+ dsb(ish);
+ isb();
+ start = read_sysreg(cntvct_el0);
+ isb();
while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
- isb();
- start = read_sysreg(cntvct_el0);
test->exec();
- isb();
- end = read_sysreg(cntvct_el0);
ntimes++;
- total_ticks += (end - start);
- ticks_to_ns_time(total_ticks, &total_ns);
}
+ dsb(ish);
+ isb();
+ end = read_sysreg(cntvct_el0);
+
+ total_ticks = end - start;
+ ticks_to_ns_time(total_ticks, &total_ns);
if (test->post) {
test->post(ntimes, &total_ticks);
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-07 9:51 ` [kvm-unit-tests PATCH " heqiong
@ 2023-11-07 12:49 ` Alexandru Elisei
2023-11-07 13:53 ` Zenghui Yu
1 sibling, 0 replies; 13+ messages in thread
From: Alexandru Elisei @ 2023-11-07 12:49 UTC (permalink / raw)
To: heqiong; +Cc: kvm
Hi,
On Tue, Nov 07, 2023 at 05:51:15PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
> ---
> arm/micro-bench.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..65f4c4dd 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -346,17 +346,21 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> + dsb(ish);
> + isb();
> + start = read_sysreg(cntvct_el0);
> + isb();
> while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
^^^^^^^^^^^^^^^^^^^^^^^^^^
This will always evaluate to true because total_ns is now computed at the
end of the loop instead of every iteration.
Do we want to drop the upper bound on how long a test takes to execute? I
don't have an opinion about it.
Thanks,
Alex
> - isb();
> - start = read_sysreg(cntvct_el0);
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + dsb(ish);
> + isb();
> + end = read_sysreg(cntvct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop
2023-11-07 9:51 ` [kvm-unit-tests PATCH " heqiong
2023-11-07 12:49 ` Alexandru Elisei
@ 2023-11-07 13:53 ` Zenghui Yu
1 sibling, 0 replies; 13+ messages in thread
From: Zenghui Yu @ 2023-11-07 13:53 UTC (permalink / raw)
To: heqiong; +Cc: kvm, alexandru.elisei, Andrew Jones
Hi,
In case that you may have trouble receiving emails from the @linux.dev
addresses (?), please check your inbox again and read the comments that
Drew had replied to your previous versions. You can otherwise read them
on lore, see below.
Zenghui
[1] https://lore.kernel.org/kvm/20231101-923f359769ccf8db69c25c4f@orel
[2] https://lore.kernel.org/kvm/20231107-9b361591b5d43284d4394f8a@orel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests
2023-11-07 8:40 ` Andrew Jones
@ 2023-11-16 4:53 ` heqiong
2023-11-20 8:35 ` Andrew Jones
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: heqiong @ 2023-11-16 4:53 UTC (permalink / raw)
To: andrew.jones; +Cc: alexandru.elisei, heqiong1557, kvm
Reducing the impact of the cntvct_el0 register and isb() operation
on microbenchmark test results to improve testing accuracy and reduce
latency in test results.
Signed-off-by: heqiong <heqiong1557@phytium.com.cn>
---
arm/micro-bench.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fbe59d03..22408955 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -24,7 +24,6 @@
#include <asm/gic-v3-its.h>
#include <asm/timer.h>
-#define NS_5_SECONDS (5 * 1000 * 1000 * 1000UL)
#define QEMU_MMIO_ADDR 0x0a000008
static u32 cntfrq;
@@ -346,17 +345,21 @@ static void loop_test(struct exit_test *test)
}
}
- while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
- isb();
- start = read_sysreg(cntvct_el0);
+ dsb(ish);
+ isb();
+ start = read_sysreg(cntvct_el0);
+ isb();
+ while (ntimes < test->times) {
test->exec();
- isb();
- end = read_sysreg(cntvct_el0);
ntimes++;
- total_ticks += (end - start);
- ticks_to_ns_time(total_ticks, &total_ns);
}
+ dsb(ish);
+ isb();
+ end = read_sysreg(cntvct_el0);
+
+ total_ticks = end - start;
+ ticks_to_ns_time(total_ticks, &total_ns);
if (test->post) {
test->post(ntimes, &total_ticks);
--
2.39.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests
2023-11-16 4:53 ` [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests heqiong
@ 2023-11-20 8:35 ` Andrew Jones
2023-11-20 17:25 ` Alexandru Elisei
2023-11-21 11:45 ` Andrew Jones
2 siblings, 0 replies; 13+ messages in thread
From: Andrew Jones @ 2023-11-20 8:35 UTC (permalink / raw)
To: heqiong; +Cc: alexandru.elisei, kvm
Thanks, Heqiong. The patch is looking much better. The only thing missing
now is the patch version. This is v3, so it should have a prefix like this
[kvm-unit-tests PATCH v3 1/1]
There's no need to respin for that though. afaict you've addressed
Alexandru's comments, but I'll let him take a look before merging.
Thanks,
drew
On Thu, Nov 16, 2023 at 12:53:55PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
>
> Signed-off-by: heqiong <heqiong1557@phytium.com.cn>
> ---
> arm/micro-bench.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..22408955 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -24,7 +24,6 @@
> #include <asm/gic-v3-its.h>
> #include <asm/timer.h>
>
> -#define NS_5_SECONDS (5 * 1000 * 1000 * 1000UL)
> #define QEMU_MMIO_ADDR 0x0a000008
>
> static u32 cntfrq;
> @@ -346,17 +345,21 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> - while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> + dsb(ish);
> + isb();
> + start = read_sysreg(cntvct_el0);
> + isb();
> + while (ntimes < test->times) {
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + dsb(ish);
> + isb();
> + end = read_sysreg(cntvct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests
2023-11-16 4:53 ` [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests heqiong
2023-11-20 8:35 ` Andrew Jones
@ 2023-11-20 17:25 ` Alexandru Elisei
2023-11-21 11:45 ` Andrew Jones
2 siblings, 0 replies; 13+ messages in thread
From: Alexandru Elisei @ 2023-11-20 17:25 UTC (permalink / raw)
To: heqiong; +Cc: andrew.jones, kvm
Hi,
On Thu, Nov 16, 2023 at 12:53:55PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
Sorry, lost track of which version is the latest - that's why patch version
numbers are really useful!
Everything look alright to me:
Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Thanks,
Alex
>
> Signed-off-by: heqiong <heqiong1557@phytium.com.cn>
> ---
> arm/micro-bench.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..22408955 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -24,7 +24,6 @@
> #include <asm/gic-v3-its.h>
> #include <asm/timer.h>
>
> -#define NS_5_SECONDS (5 * 1000 * 1000 * 1000UL)
> #define QEMU_MMIO_ADDR 0x0a000008
>
> static u32 cntfrq;
> @@ -346,17 +345,21 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> - while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> + dsb(ish);
> + isb();
> + start = read_sysreg(cntvct_el0);
> + isb();
> + while (ntimes < test->times) {
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + dsb(ish);
> + isb();
> + end = read_sysreg(cntvct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests
2023-11-16 4:53 ` [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests heqiong
2023-11-20 8:35 ` Andrew Jones
2023-11-20 17:25 ` Alexandru Elisei
@ 2023-11-21 11:45 ` Andrew Jones
2 siblings, 0 replies; 13+ messages in thread
From: Andrew Jones @ 2023-11-21 11:45 UTC (permalink / raw)
To: heqiong; +Cc: alexandru.elisei, kvm
On Thu, Nov 16, 2023 at 12:53:55PM +0800, heqiong wrote:
> Reducing the impact of the cntvct_el0 register and isb() operation
> on microbenchmark test results to improve testing accuracy and reduce
> latency in test results.
>
> Signed-off-by: heqiong <heqiong1557@phytium.com.cn>
> ---
> arm/micro-bench.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fbe59d03..22408955 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -24,7 +24,6 @@
> #include <asm/gic-v3-its.h>
> #include <asm/timer.h>
>
> -#define NS_5_SECONDS (5 * 1000 * 1000 * 1000UL)
> #define QEMU_MMIO_ADDR 0x0a000008
>
> static u32 cntfrq;
> @@ -346,17 +345,21 @@ static void loop_test(struct exit_test *test)
> }
> }
>
> - while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> - isb();
> - start = read_sysreg(cntvct_el0);
> + dsb(ish);
> + isb();
> + start = read_sysreg(cntvct_el0);
> + isb();
> + while (ntimes < test->times) {
> test->exec();
> - isb();
> - end = read_sysreg(cntvct_el0);
>
> ntimes++;
> - total_ticks += (end - start);
> - ticks_to_ns_time(total_ticks, &total_ns);
> }
> + dsb(ish);
> + isb();
> + end = read_sysreg(cntvct_el0);
> +
> + total_ticks = end - start;
> + ticks_to_ns_time(total_ticks, &total_ns);
>
> if (test->post) {
> test->post(ntimes, &total_ticks);
> --
> 2.39.3
>
Merged into https://gitlab.com/kvm-unit-tests/kvm-unit-tests master
Thanks,
drew
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-11-21 11:45 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-07 6:40 [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop heqiong
2023-11-07 8:40 ` Andrew Jones
2023-11-16 4:53 ` [kvm-unit-tests PATCH 1/1] arm64: microbench: Improve measurement accuracy of tests heqiong
2023-11-20 8:35 ` Andrew Jones
2023-11-20 17:25 ` Alexandru Elisei
2023-11-21 11:45 ` Andrew Jones
2023-11-07 9:07 ` [kvm-unit-tests 1/1] arm64: microbench: Move the read of the count register and the ISB operation out of the while loop Alexandru Elisei
2023-11-07 9:51 ` [kvm-unit-tests PATCH " heqiong
2023-11-07 12:49 ` Alexandru Elisei
2023-11-07 13:53 ` Zenghui Yu
-- strict thread matches above, loose matches on Subject: below --
2023-11-01 8:25 [kvm-unit-tests " 何琼
2023-11-01 10:06 ` Andrew Jones
2023-11-01 11:04 ` Alexandru Elisei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).