All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: KVM: arm64: pmu: Reset sample period on overflow handling
@ 2021-06-15 15:15 Aman Priyadarshi
  2021-06-15 17:05 ` Marc Zyngier
  0 siblings, 1 reply; 6+ messages in thread
From: Aman Priyadarshi @ 2021-06-15 15:15 UTC (permalink / raw)
  To: Marc Zyngier, Andrew Murray, kvmarm
  Cc: Alexander Graf, Ali Saidi, Aman Priyadarshi

Hey,

I have been chasing down a regression on guest side related to pmu cycle
counters, and I found that reverting these two patches help:

- 30d97754b2d1 ("KVM: arm/arm64: Re-create event when setting counter
value")
- 8c3252c06516 ("KVM: arm64: pmu: Reset sample period on overflow
handling")

However, I do not yet fully understand the underlying problem.

Regression can be reproduced by running simple userspace processes
consuming 100% of cpus and checking the number of pmu cycles.

```
$ cat foo.c
#include <stdio.h>

int main()
{
    while (1) ;
    return 0;
}


$ gcc -o foo foo.c
$ for x in {0..63}; do ./foo & done
$ sudo perf stat -A -a -e cpu-clock:pppH,cycles -- sleep 10
CPU0        22,397,323,233      cycles                    #    2.240 GHz
CPU1         6,444,741,327      cycles                    #    0.644 GHz
CPU2        17,029,444,425      cycles                    #    1.703 GHz
CPU3         4,298,054,663      cycles                    #    0.430 GHz
CPU4         6,444,769,216      cycles                    #    0.644 GHz
CPU5         6,045,456,891      cycles                    #    0.605 GHz
CPU6         4,298,204,814      cycles                    #    0.430 GHz
CPU7         6,444,346,686      cycles                    #    0.644 GHz
CPU8         4,298,012,726      cycles                    #    0.430 GHz
CPU9         6,445,547,392      cycles                    #    0.645 GHz
CPU10        4,297,996,144      cycles                    #    0.430 GHz
...
```

Expected behavior is to have all cores around 2.5GHz on all CPUs given that
all CPUs are 100% in the guest.

The nonsensical values reported by the pmu counters is only observed on
certain combination of host and guest kernel.

I was able to reproduce it on v5.4.34 and v5.13.0-rc5 running on the host
side, and 4.14.10 on the guest side.

I am running Ubuntu 18.04 cloud image with 4.14.10 kernel headers
installed:

https://cloud-images.ubuntu.com/releases/bionic/release/ubuntu-18.04-server-cloudimg-arm64.img

Note, cloud image comes with 4.15.y kernel installed, which does not show
any regression.

I can also confirm that reverting the mentioned commits on either tree
(v5.4.34 or v5.13.0-rc5) fixes the problem.
```
CPU51          25020026126      cycles                    #    2.497 GHz
CPU52          25020153183      cycles                    #    2.497 GHz
CPU53          25011934659      cycles                    #    2.496 GHz
CPU54          25020521670      cycles                    #    2.497 GHz
CPU55          25015920340      cycles                    #    2.497 GHz
CPU56          25020176871      cycles                    #    2.497 GHz
CPU57          25013646763      cycles                    #    2.497 GHz
CPU58          25020736379      cycles                    #    2.497 GHz
CPU59          25019495291      cycles                    #    2.497 GHz
CPU60          25020988560      cycles                    #    2.497 GHz
CPU61          25021343608      cycles                    #    2.497 GHz
CPU62          25018942029      cycles                    #    2.497 GHz
CPU63          25019514949      cycles                    #    2.497 GHz

      10.020502287 seconds time elapsed

root@ubuntu:~# cat /proc/version  # Guest
Linux version 4.14.10-041410-generic (kernel@kathleen) (gcc version 7.2.0
(Ubuntu/Linaro 7.2.0-6ubuntu1)) #201712291810 SMP Fri Dec 29 18:37:30 UTC
2017
root@ubuntu:~# QEMU: Terminated
ubuntu@ip-10-0-98-166:~/images$ cat /proc/version  # Host
Linux version 5.13.0-rc5 (ubuntu@ip-10-0-98-166) (gcc (Ubuntu 9.3.0-
17ubuntu1~20.04) 9.3.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #2 SMP Mon
Jun 14 21:05:25 UTC 2021
```

Regards,
Aman Priyadarshi




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: KVM: arm64: pmu: Reset sample period on overflow handling
  2021-06-15 15:15 KVM: arm64: pmu: Reset sample period on overflow handling Aman Priyadarshi
@ 2021-06-15 17:05 ` Marc Zyngier
  2021-06-16  9:17   ` Aman Priyadarshi
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2021-06-15 17:05 UTC (permalink / raw)
  To: Aman Priyadarshi; +Cc: Alexander Graf, kvmarm, Ali Saidi

Hi Aman,

[- Andrew]

On Tue, 15 Jun 2021 16:15:28 +0100,
Aman Priyadarshi <apeureka@amazon.de> wrote:
> 
> Hey,
> 
> I have been chasing down a regression on guest side related to pmu cycle
> counters, and I found that reverting these two patches help:
> 
> - 30d97754b2d1 ("KVM: arm/arm64: Re-create event when setting counter
> value")
> - 8c3252c06516 ("KVM: arm64: pmu: Reset sample period on overflow
> handling")
> 
> However, I do not yet fully understand the underlying problem.
> 
> Regression can be reproduced by running simple userspace processes
> consuming 100% of cpus and checking the number of pmu cycles.
> 
> ```
> $ cat foo.c
> #include <stdio.h>
> 
> int main()
> {
>     while (1) ;
>     return 0;
> }
> 
> 
> $ gcc -o foo foo.c
> $ for x in {0..63}; do ./foo & done
> $ sudo perf stat -A -a -e cpu-clock:pppH,cycles -- sleep 10
> CPU0        22,397,323,233      cycles                    #    2.240 GHz
> CPU1         6,444,741,327      cycles                    #    0.644 GHz
> CPU2        17,029,444,425      cycles                    #    1.703 GHz
> CPU3         4,298,054,663      cycles                    #    0.430 GHz
> CPU4         6,444,769,216      cycles                    #    0.644 GHz
> CPU5         6,045,456,891      cycles                    #    0.605 GHz
> CPU6         4,298,204,814      cycles                    #    0.430 GHz
> CPU7         6,444,346,686      cycles                    #    0.644 GHz
> CPU8         4,298,012,726      cycles                    #    0.430 GHz
> CPU9         6,445,547,392      cycles                    #    0.645 GHz
> CPU10        4,297,996,144      cycles                    #    0.430 GHz
> ...
> ```
> 
> Expected behavior is to have all cores around 2.5GHz on all CPUs given that
> all CPUs are 100% in the guest.
> 
> The nonsensical values reported by the pmu counters is only observed on
> certain combination of host and guest kernel.
> 
> I was able to reproduce it on v5.4.34 and v5.13.0-rc5 running on the host
> side, and 4.14.10 on the guest side.
> 
> I am running Ubuntu 18.04 cloud image with 4.14.10 kernel headers
> installed:
> 
> https://cloud-images.ubuntu.com/releases/bionic/release/ubuntu-18.04-server-cloudimg-arm64.img
> 
> Note, cloud image comes with 4.15.y kernel installed, which does not show
> any regression.

Can you reproduce the issue with vanilla guest kernels? It'd be
interesting to understand what makes it work on the guest side. Can
you please bisect it?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: KVM: arm64: pmu: Reset sample period on overflow handling
  2021-06-15 17:05 ` Marc Zyngier
@ 2021-06-16  9:17   ` Aman Priyadarshi
  2021-06-16 10:31     ` Marc Zyngier
  0 siblings, 1 reply; 6+ messages in thread
From: Aman Priyadarshi @ 2021-06-16  9:17 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Alexander Graf, kvmarm, Ali Saidi

Hi Marc,

On Tue, 2021-06-15 at 18:05 +0100, Marc Zyngier wrote:
> 
> Can you reproduce the issue with vanilla guest kernels? It'd be
> interesting to understand what makes it work on the guest side. Can
> you please bisect it?
> 

yes, I was able to narrow it down to the commit 0cbb058be904 ("arm64: perf:
Disable PMU while processing counter overflows"), which fixes the problem
on the guest side.

I _think_, I understand the problem now. Please correct me if I am wrong.

commit 30d97754b2d1 ("KVM: arm/arm64: Re-create event when setting counter
value") adds a new code path for perf event when counter value is set,
therefore kvm would generate more events than before. Without this change,
we have a lot less events, thus reducing the chances of guest messing
things up.

On the other side, commit 8c3252c06516 ("KVM: arm64: pmu: Reset sample
period on overflow handling") resets the sample period to the max value,
thus reducing the number of overflow events to guest to an optimal value
(note, number of interrupts actually handled by guest would remain same in
either case). Less number of overflow interrupts to the guest, reduces the
chance of guest making up for any left over overflow event that it did not
see earlier.

Thanks,
Aman Priyadarshi





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: KVM: arm64: pmu: Reset sample period on overflow handling
  2021-06-16  9:17   ` Aman Priyadarshi
@ 2021-06-16 10:31     ` Marc Zyngier
  2021-06-16 10:52       ` Aman Priyadarshi
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2021-06-16 10:31 UTC (permalink / raw)
  To: Aman Priyadarshi; +Cc: Alexander Graf, kvmarm, Ali Saidi

Hi Aman,

On Wed, 16 Jun 2021 10:17:28 +0100,
Aman Priyadarshi <apeureka@amazon.de> wrote:
> 
> Hi Marc,
> 
> On Tue, 2021-06-15 at 18:05 +0100, Marc Zyngier wrote:
> > 
> > Can you reproduce the issue with vanilla guest kernels? It'd be
> > interesting to understand what makes it work on the guest side. Can
> > you please bisect it?
> > 
> 
> yes, I was able to narrow it down to the commit 0cbb058be904 ("arm64: perf:
> Disable PMU while processing counter overflows"), which fixes the problem
> on the guest side.

Which is 3cce50dfec4a5b0414c974190940f47dd32c6dee in mainline. This
doesn't seem to have ever been backported before 4.18. So I don't know
why your 4.15 kernel was correctly behaving, but it could be that the
distro had randomly picked up the correct patch!

You may want to backport it to 4.14.y and let Greg know about that.

> 
> I _think_, I understand the problem now. Please correct me if I am wrong.
> 
> commit 30d97754b2d1 ("KVM: arm/arm64: Re-create event when setting counter
> value") adds a new code path for perf event when counter value is set,
> therefore kvm would generate more events than before. Without this change,
> we have a lot less events, thus reducing the chances of guest messing
> things up.

Without this fix, we don't communicate the new guest sample period to
the host's perf counter, and depending on what the guest wrote (and
the previous value), it can go one way or the other.

> On the other side, commit 8c3252c06516 ("KVM: arm64: pmu: Reset sample
> period on overflow handling") resets the sample period to the max value,
> thus reducing the number of overflow events to guest to an optimal value
> (note, number of interrupts actually handled by guest would remain same in
> either case). Less number of overflow interrupts to the guest, reduces the
> chance of guest making up for any left over overflow event that it did not
> see earlier.

This fix is the natural complement of the previous one. We need to
emulate the actual overflow, and prevent perf from doing its thing on
the host (reloading from the previously provided value). So we reset
the period to the value that perf did observe on taking the physical
interrupt.

Together, these two patches provide a more correct PMU emulation.

The guest patch fixes prevents additional overflow being observed due
while the guest is reprogramming its counters and observe a moving
target. Note that the host itself needs that initial fix to correctly
emulate the PMU! ;-)

It is pretty hard to picture exactly *what* happens when you are
missing any of these 3 patches. Both the kernel and KVM were buggy at
some point, and you need all three patches to ensure something
correct.

Anyway, thanks for having bisected it, and worked out that this was a
guest issue!

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: KVM: arm64: pmu: Reset sample period on overflow handling
  2021-06-16 10:31     ` Marc Zyngier
@ 2021-06-16 10:52       ` Aman Priyadarshi
  2021-06-16 11:03         ` Marc Zyngier
  0 siblings, 1 reply; 6+ messages in thread
From: Aman Priyadarshi @ 2021-06-16 10:52 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Alexander Graf, kvmarm, Ali Saidi

On Wed, 2021-06-16 at 11:31 +0100, Marc Zyngier wrote:
> 
> Hi Aman,
> 
> On Wed, 16 Jun 2021 10:17:28 +0100,
> Aman Priyadarshi <apeureka@amazon.de> wrote:
> > 
> > Hi Marc,
> > 
> > On Tue, 2021-06-15 at 18:05 +0100, Marc Zyngier wrote:
> > > 
> > > Can you reproduce the issue with vanilla guest kernels? It'd be
> > > interesting to understand what makes it work on the guest side. Can
> > > you please bisect it?
> > > 
> > 
> > yes, I was able to narrow it down to the commit 0cbb058be904 ("arm64:
> > perf:
> > Disable PMU while processing counter overflows"), which fixes the
> > problem
> > on the guest side.
> 
> Which is 3cce50dfec4a5b0414c974190940f47dd32c6dee in mainline. This
> doesn't seem to have ever been backported before 4.18. So I don't know
> why your 4.15 kernel was correctly behaving, but it could be that the
> distro had randomly picked up the correct patch!

Yes. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1836117

> 
> You may want to backport it to 4.14.y and let Greg know about that.
> 

Ack.

> > 
> > I _think_, I understand the problem now. Please correct me if I am
> > wrong.
> > 
> > commit 30d97754b2d1 ("KVM: arm/arm64: Re-create event when setting
> > counter
> > value") adds a new code path for perf event when counter value is set,
> > therefore kvm would generate more events than before. Without this
> > change,
> > we have a lot less events, thus reducing the chances of guest messing
> > things up.
> 
> Without this fix, we don't communicate the new guest sample period to
> the host's perf counter, and depending on what the guest wrote (and
> the previous value), it can go one way or the other.
> 
> > On the other side, commit 8c3252c06516 ("KVM: arm64: pmu: Reset sample
> > period on overflow handling") resets the sample period to the max
> > value,
> > thus reducing the number of overflow events to guest to an optimal
> > value
> > (note, number of interrupts actually handled by guest would remain same
> > in
> > either case). Less number of overflow interrupts to the guest, reduces
> > the
> > chance of guest making up for any left over overflow event that it did
> > not
> > see earlier.
> 
> This fix is the natural complement of the previous one. We need to
> emulate the actual overflow, and prevent perf from doing its thing on
> the host (reloading from the previously provided value). So we reset
> the period to the value that perf did observe on taking the physical
> interrupt.
> 
> Together, these two patches provide a more correct PMU emulation.
> 
> The guest patch fixes prevents additional overflow being observed due
> while the guest is reprogramming its counters and observe a moving
> target. Note that the host itself needs that initial fix to correctly
> emulate the PMU! ;-)
> 
> It is pretty hard to picture exactly *what* happens when you are
> missing any of these 3 patches. Both the kernel and KVM were buggy at
> some point, and you need all three patches to ensure something
> correct.
> 

Thanks for the explanation!

Regards,
Aman Priyadarshi




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: KVM: arm64: pmu: Reset sample period on overflow handling
  2021-06-16 10:52       ` Aman Priyadarshi
@ 2021-06-16 11:03         ` Marc Zyngier
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2021-06-16 11:03 UTC (permalink / raw)
  To: Aman Priyadarshi; +Cc: Alexander Graf, kvmarm, Ali Saidi

On Wed, 16 Jun 2021 11:52:42 +0100,
Aman Priyadarshi <apeureka@amazon.de> wrote:
> 
> On Wed, 2021-06-16 at 11:31 +0100, Marc Zyngier wrote:
> > 
> > Hi Aman,
> > 
> > On Wed, 16 Jun 2021 10:17:28 +0100,
> > Aman Priyadarshi <apeureka@amazon.de> wrote:
> > > 
> > > Hi Marc,
> > > 
> > > On Tue, 2021-06-15 at 18:05 +0100, Marc Zyngier wrote:
> > > > 
> > > > Can you reproduce the issue with vanilla guest kernels? It'd be
> > > > interesting to understand what makes it work on the guest side. Can
> > > > you please bisect it?
> > > > 
> > > 
> > > yes, I was able to narrow it down to the commit 0cbb058be904 ("arm64:
> > > perf:
> > > Disable PMU while processing counter overflows"), which fixes the
> > > problem
> > > on the guest side.
> > 
> > Which is 3cce50dfec4a5b0414c974190940f47dd32c6dee in mainline. This
> > doesn't seem to have ever been backported before 4.18. So I don't know
> > why your 4.15 kernel was correctly behaving, but it could be that the
> > distro had randomly picked up the correct patch!
> 
> Yes. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1836117

Right. I guess that's their "value add".

> > You may want to backport it to 4.14.y and let Greg know about that.
> > 
> 
> Ack.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-16 12:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 15:15 KVM: arm64: pmu: Reset sample period on overflow handling Aman Priyadarshi
2021-06-15 17:05 ` Marc Zyngier
2021-06-16  9:17   ` Aman Priyadarshi
2021-06-16 10:31     ` Marc Zyngier
2021-06-16 10:52       ` Aman Priyadarshi
2021-06-16 11:03         ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.