| Message ID | 20230307141400.1486314-5-aaronlewis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix "Instructions Retired" from incorrectly counting | expand |
Same shortlog problem. On Tue, Mar 07, 2023, Aaron Lewis wrote: > Fix up both ASSERT_PMC_COUNTING and ASSERT_PMC_NOT_COUNTING in the > pmu_event_filter_test by adding additional context in the assert > message. I 100% agree that the asserts are flawed, but in the context of changelogs, "fix" almost always implies something is broken. In this case, what is being asserted is completely ok, it's only the messages that are bad. Easiest thing is to just describe the change and explain why it's desirable, and dodge the question of whether or not this should be considered a fix. Provide the actual vs. expected count in the PMU event filter test's asserts instead of relying on pr_info() to provide the context, e.g. so that all information needed to triage a failure is readily available even if the environment in which the test is run captures only the assert itself.
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c index 8277b8f49dca..78bb48fcd33e 100644 --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c @@ -252,18 +252,17 @@ static struct kvm_pmu_event_filter *remove_event(struct kvm_pmu_event_filter *f, #define ASSERT_PMC_COUNTING(count) \ do { \ - if (count != NUM_BRANCHES) \ + if (count && count != NUM_BRANCHES) \ pr_info("%s: Branch instructions retired = %lu (expected %u)\n", \ __func__, count, NUM_BRANCHES); \ - TEST_ASSERT(count, "Allowed PMU event is not counting."); \ + TEST_ASSERT(count, "%s: Branch instructions retired = %lu (expected > 0)", \ + __func__, count); \ } while (0) #define ASSERT_PMC_NOT_COUNTING(count) \ do { \ - if (count) \ - pr_info("%s: Branch instructions retired = %lu (expected 0)\n", \ - __func__, count); \ - TEST_ASSERT(!count, "Disallowed PMU Event is counting"); \ + TEST_ASSERT(!count, "%s: Branch instructions retired = %lu (expected 0)", \ + __func__, count); \ } while (0) static void test_without_filter(struct kvm_vcpu *vcpu)
Fix up both ASSERT_PMC_COUNTING and ASSERT_PMC_NOT_COUNTING in the pmu_event_filter_test by adding additional context in the assert message. With the added context the print in ASSERT_PMC_NOT_COUNTING is redundant. Remove it. Signed-off-by: Aaron Lewis <aaronlewis@google.com> --- .../selftests/kvm/x86_64/pmu_event_filter_test.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)