| Message ID | 20220601163012.3404212-1-morbo@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/pmu: Disable inlining of measure() | expand |
On Wed, Jun 1, 2022 at 9:30 AM Bill Wendling <morbo@google.com> wrote: > > From: Bill Wendling <isanbard@gmail.com> > > Clang can be more aggressive at inlining than GCC and will fully inline > calls to measure(). This can mess with the counter overflow check. To > set up the PMC overflow, check_counter_overflow() first records the > number of instructions retired in an invocation of measure() and checks > to see that subsequent calls to measure() retire the same number of > instructions. If inlining occurs, those numbers can be different and the > overflow test fails. > > FAIL: overflow: cntr-0 > PASS: overflow: status-0 > PASS: overflow: status clear-0 > PASS: overflow: irq-0 > FAIL: overflow: cntr-1 > PASS: overflow: status-1 > PASS: overflow: status clear-1 > PASS: overflow: irq-1 > FAIL: overflow: cntr-2 > PASS: overflow: status-2 > PASS: overflow: status clear-2 > PASS: overflow: irq-2 > FAIL: overflow: cntr-3 > PASS: overflow: status-3 > PASS: overflow: status clear-3 > PASS: overflow: irq-3 > > Disabling inlining of measure() keeps the assumption that all calls to > measure() retire the same number of instructions. > > Cc: Jim Mattson <jmattson@google.com> > Signed-off-by: Bill Wendling <morbo@google.com> Reviewed-by: Jim Mattson <jmattson@google.com>
On Wed, Jun 1, 2022 at 10:22 AM Jim Mattson <jmattson@google.com> wrote: > > On Wed, Jun 1, 2022 at 9:30 AM Bill Wendling <morbo@google.com> wrote: > > > > From: Bill Wendling <isanbard@gmail.com> > > > > Clang can be more aggressive at inlining than GCC and will fully inline > > calls to measure(). This can mess with the counter overflow check. To > > set up the PMC overflow, check_counter_overflow() first records the > > number of instructions retired in an invocation of measure() and checks > > to see that subsequent calls to measure() retire the same number of > > instructions. If inlining occurs, those numbers can be different and the > > overflow test fails. > > > > FAIL: overflow: cntr-0 > > PASS: overflow: status-0 > > PASS: overflow: status clear-0 > > PASS: overflow: irq-0 > > FAIL: overflow: cntr-1 > > PASS: overflow: status-1 > > PASS: overflow: status clear-1 > > PASS: overflow: irq-1 > > FAIL: overflow: cntr-2 > > PASS: overflow: status-2 > > PASS: overflow: status clear-2 > > PASS: overflow: irq-2 > > FAIL: overflow: cntr-3 > > PASS: overflow: status-3 > > PASS: overflow: status clear-3 > > PASS: overflow: irq-3 > > > > Disabling inlining of measure() keeps the assumption that all calls to > > measure() retire the same number of instructions. > > > > Cc: Jim Mattson <jmattson@google.com> > > Signed-off-by: Bill Wendling <morbo@google.com> > Reviewed-by: Jim Mattson <jmattson@google.com> Bumping for visibility. -bw
On Tue, Oct 25, 2022, Bill Wendling wrote: > On Wed, Jun 1, 2022 at 10:22 AM Jim Mattson <jmattson@google.com> wrote: > > > > On Wed, Jun 1, 2022 at 9:30 AM Bill Wendling <morbo@google.com> wrote: > > > > > > From: Bill Wendling <isanbard@gmail.com> > > > > > > Clang can be more aggressive at inlining than GCC and will fully inline > > > calls to measure(). This can mess with the counter overflow check. To > > > set up the PMC overflow, check_counter_overflow() first records the > > > number of instructions retired in an invocation of measure() and checks > > > to see that subsequent calls to measure() retire the same number of > > > instructions. If inlining occurs, those numbers can be different and the > > > overflow test fails. > > > > > > FAIL: overflow: cntr-0 > > > PASS: overflow: status-0 > > > PASS: overflow: status clear-0 > > > PASS: overflow: irq-0 > > > FAIL: overflow: cntr-1 > > > PASS: overflow: status-1 > > > PASS: overflow: status clear-1 > > > PASS: overflow: irq-1 > > > FAIL: overflow: cntr-2 > > > PASS: overflow: status-2 > > > PASS: overflow: status clear-2 > > > PASS: overflow: irq-2 > > > FAIL: overflow: cntr-3 > > > PASS: overflow: status-3 > > > PASS: overflow: status clear-3 > > > PASS: overflow: irq-3 > > > > > > Disabling inlining of measure() keeps the assumption that all calls to > > > measure() retire the same number of instructions. > > > > > > Cc: Jim Mattson <jmattson@google.com> > > > Signed-off-by: Bill Wendling <morbo@google.com> > > Reviewed-by: Jim Mattson <jmattson@google.com> > > Bumping for visibility. Heh, make sure kvm-unit-tests is in the subject, i.e. [kvm-unit-tests PATCH]. This slipped by on my end because I didn't realize at a quick glance that it was touching KVM-unit-tests and not kernel code.
On Tue, Oct 25, 2022 at 4:12 PM Sean Christopherson <seanjc@google.com> wrote: > > On Tue, Oct 25, 2022, Bill Wendling wrote: > > On Wed, Jun 1, 2022 at 10:22 AM Jim Mattson <jmattson@google.com> wrote: > > > > > > On Wed, Jun 1, 2022 at 9:30 AM Bill Wendling <morbo@google.com> wrote: > > > > > > > > From: Bill Wendling <isanbard@gmail.com> > > > > > > > > Clang can be more aggressive at inlining than GCC and will fully inline > > > > calls to measure(). This can mess with the counter overflow check. To > > > > set up the PMC overflow, check_counter_overflow() first records the > > > > number of instructions retired in an invocation of measure() and checks > > > > to see that subsequent calls to measure() retire the same number of > > > > instructions. If inlining occurs, those numbers can be different and the > > > > overflow test fails. > > > > > > > > FAIL: overflow: cntr-0 > > > > PASS: overflow: status-0 > > > > PASS: overflow: status clear-0 > > > > PASS: overflow: irq-0 > > > > FAIL: overflow: cntr-1 > > > > PASS: overflow: status-1 > > > > PASS: overflow: status clear-1 > > > > PASS: overflow: irq-1 > > > > FAIL: overflow: cntr-2 > > > > PASS: overflow: status-2 > > > > PASS: overflow: status clear-2 > > > > PASS: overflow: irq-2 > > > > FAIL: overflow: cntr-3 > > > > PASS: overflow: status-3 > > > > PASS: overflow: status clear-3 > > > > PASS: overflow: irq-3 > > > > > > > > Disabling inlining of measure() keeps the assumption that all calls to > > > > measure() retire the same number of instructions. > > > > > > > > Cc: Jim Mattson <jmattson@google.com> > > > > Signed-off-by: Bill Wendling <morbo@google.com> > > > Reviewed-by: Jim Mattson <jmattson@google.com> > > > > Bumping for visibility. > > Heh, make sure kvm-unit-tests is in the subject, i.e. [kvm-unit-tests PATCH]. > This slipped by on my end because I didn't realize at a quick glance that it was > touching KVM-unit-tests and not kernel code. Doh! I'm not sure why I forgot that. Added. Thanks. :) -bw
On Wed, Oct 26, 2022, Bill Wendling wrote: > On Tue, Oct 25, 2022 at 4:12 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Tue, Oct 25, 2022, Bill Wendling wrote: > > > On Wed, Jun 1, 2022 at 10:22 AM Jim Mattson <jmattson@google.com> wrote: > > > > > > > > On Wed, Jun 1, 2022 at 9:30 AM Bill Wendling <morbo@google.com> wrote: > > > > > > > > > > From: Bill Wendling <isanbard@gmail.com> This "From:" should not exist. It causes your @gmail.com account be to credited as the author, whereas your SOB suggests you intended this to be credited to your @google.com address. Let me know if this should indeed list morbo@google.com as the author, I can fix it up when applying. > > > > > Signed-off-by: Bill Wendling <morbo@google.com> > > > > Reviewed-by: Jim Mattson <jmattson@google.com>
On Tue, Nov 1, 2022 at 11:53 AM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Oct 26, 2022, Bill Wendling wrote: > > On Tue, Oct 25, 2022 at 4:12 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > > On Tue, Oct 25, 2022, Bill Wendling wrote: > > > > On Wed, Jun 1, 2022 at 10:22 AM Jim Mattson <jmattson@google.com> wrote: > > > > > > > > > > On Wed, Jun 1, 2022 at 9:30 AM Bill Wendling <morbo@google.com> wrote: > > > > > > > > > > > > From: Bill Wendling <isanbard@gmail.com> > > This "From:" should not exist. It causes your @gmail.com account be to credited > as the author, whereas your SOB suggests you intended this to be credited to your > @google.com address. > > Let me know if this should indeed list morbo@google.com as the author, I can fix > it up when applying. > Please use morbo@google.com. Sorry about this mixup. -bw > > > > > > Signed-off-by: Bill Wendling <morbo@google.com> > > > > > Reviewed-by: Jim Mattson <jmattson@google.com>
On 6/1/22 18:30, Bill Wendling wrote: > From: Bill Wendling <isanbard@gmail.com> > > Clang can be more aggressive at inlining than GCC and will fully inline > calls to measure(). This can mess with the counter overflow check. To > set up the PMC overflow, check_counter_overflow() first records the > number of instructions retired in an invocation of measure() and checks > to see that subsequent calls to measure() retire the same number of > instructions. If inlining occurs, those numbers can be different and the > overflow test fails. > > FAIL: overflow: cntr-0 > PASS: overflow: status-0 > PASS: overflow: status clear-0 > PASS: overflow: irq-0 > FAIL: overflow: cntr-1 > PASS: overflow: status-1 > PASS: overflow: status clear-1 > PASS: overflow: irq-1 > FAIL: overflow: cntr-2 > PASS: overflow: status-2 > PASS: overflow: status clear-2 > PASS: overflow: irq-2 > FAIL: overflow: cntr-3 > PASS: overflow: status-3 > PASS: overflow: status clear-3 > PASS: overflow: irq-3 > > Disabling inlining of measure() keeps the assumption that all calls to > measure() retire the same number of instructions. > > Cc: Jim Mattson <jmattson@google.com> > Signed-off-by: Bill Wendling <morbo@google.com> > --- > x86/pmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/x86/pmu.c b/x86/pmu.c > index a46bdbf4788c..bbfd268aafa4 100644 > --- a/x86/pmu.c > +++ b/x86/pmu.c > @@ -211,7 +211,7 @@ static void stop_event(pmu_counter_t *evt) > evt->count = rdmsr(evt->ctr); > } > > -static void measure(pmu_counter_t *evt, int count) > +static noinline void measure(pmu_counter_t *evt, int count) > { > int i; > for (i = 0; i < count; i++) Queued, thanks. Paolo
diff --git a/x86/pmu.c b/x86/pmu.c index a46bdbf4788c..bbfd268aafa4 100644 --- a/x86/pmu.c +++ b/x86/pmu.c @@ -211,7 +211,7 @@ static void stop_event(pmu_counter_t *evt) evt->count = rdmsr(evt->ctr); } -static void measure(pmu_counter_t *evt, int count) +static noinline void measure(pmu_counter_t *evt, int count) { int i; for (i = 0; i < count; i++)