-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Preserve libpfm privilege modifiers in performance counters #2273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| // Copyright 2021 Google Inc. All rights reserved. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2021? and since when do we have copyright notices in headers? if you're going to use AI for a PR PLEASE PLEASE review it before sending it to us. |
||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| #ifndef BENCHMARK_PERF_COUNTERS_LIBPFM_H | ||
| #define BENCHMARK_PERF_COUNTERS_LIBPFM_H | ||
|
|
||
| #include <linux/perf_event.h> | ||
|
|
||
| #include "perfmon/pfmlib.h" | ||
| #include "perfmon/pfmlib_perf_event.h" | ||
|
|
||
| namespace benchmark { | ||
| namespace internal { | ||
|
|
||
| inline int ConfigurePerfEventAttr(const char* name, bool is_group_leader, | ||
| perf_event_attr* attr) { | ||
| *attr = {}; | ||
| attr->size = sizeof(*attr); | ||
|
|
||
| pfm_perf_encode_arg_t arg{}; | ||
| arg.attr = attr; | ||
| const int kCounterMode = PFM_PLM3; // user mode unless overridden by name | ||
| const int status = | ||
| pfm_get_os_event_encoding(name, kCounterMode, PFM_OS_PERF_EVENT, &arg); | ||
| if (status != PFM_SUCCESS) { | ||
| return status; | ||
| } | ||
|
|
||
| // Preserve the privilege exclusions encoded by libpfm for modifiers such as | ||
| // ":u" and ":k", and populate only the benchmark-owned group attributes. | ||
| attr->disabled = is_group_leader; | ||
| attr->inherit = true; | ||
| attr->pinned = is_group_leader; | ||
| attr->read_format = PERF_FORMAT_GROUP; | ||
| return PFM_SUCCESS; | ||
| } | ||
|
|
||
| } // namespace internal | ||
| } // namespace benchmark | ||
|
|
||
| #endif // BENCHMARK_PERF_COUNTERS_LIBPFM_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| #include <array> | ||
| #include <mutex> | ||
| #include <random> | ||
| #include <set> | ||
|
|
@@ -6,6 +7,12 @@ | |
| #include <vector> | ||
|
|
||
| #include "../src/perf_counters.h" | ||
| #if defined HAVE_LIBPFM | ||
| #include <linux/perf_event.h> | ||
|
|
||
| #include "../src/perf_counters_libpfm.h" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well this is clearly wrong, compared to how we include other files, isn't it? |
||
| #include "perfmon/pfmlib.h" | ||
| #endif | ||
| #include "gmock/gmock.h" | ||
| #include "gtest/gtest.h" | ||
|
|
||
|
|
@@ -19,6 +26,9 @@ struct MsgHandler { | |
| using benchmark::internal::PerfCounters; | ||
| using benchmark::internal::PerfCountersMeasurement; | ||
| using benchmark::internal::PerfCounterValues; | ||
| #if defined HAVE_LIBPFM | ||
| using benchmark::internal::ConfigurePerfEventAttr; | ||
| #endif | ||
| using ::testing::AllOf; | ||
| using ::testing::Gt; | ||
| using ::testing::Lt; | ||
|
|
@@ -49,6 +59,43 @@ TEST(PerfCountersTest, Init) { | |
| EXPECT_EQ(PerfCounters::Initialize(), PerfCounters::kSupported); | ||
| } | ||
|
|
||
| #if defined HAVE_LIBPFM | ||
| TEST(PerfCountersTest, PreservesLibpfmPrivilegeModifiers) { | ||
| ASSERT_TRUE(PerfCounters::Initialize()); | ||
|
|
||
| struct ExpectedPrivilegeMode { | ||
| const char* event_name; | ||
| bool exclude_user; | ||
| bool exclude_kernel; | ||
| }; | ||
| const std::array<ExpectedPrivilegeMode, 4> modes = {{ | ||
| {"INSTRUCTIONS", false, true}, | ||
| {"INSTRUCTIONS:u", false, true}, | ||
| {"INSTRUCTIONS:k", true, false}, | ||
| {"INSTRUCTIONS:u:k", false, false}, | ||
| }}; | ||
|
|
||
| for (const auto& mode : modes) { | ||
| perf_event_attr attr{}; | ||
| ASSERT_EQ(ConfigurePerfEventAttr(mode.event_name, true, &attr), PFM_SUCCESS) | ||
| << mode.event_name; | ||
| EXPECT_EQ(attr.exclude_user, mode.exclude_user) << mode.event_name; | ||
| EXPECT_EQ(attr.exclude_kernel, mode.exclude_kernel) << mode.event_name; | ||
| EXPECT_TRUE(attr.exclude_hv) << mode.event_name; | ||
| EXPECT_TRUE(attr.disabled) << mode.event_name; | ||
| EXPECT_TRUE(attr.inherit) << mode.event_name; | ||
| EXPECT_TRUE(attr.pinned) << mode.event_name; | ||
| EXPECT_EQ(attr.read_format, PERF_FORMAT_GROUP) << mode.event_name; | ||
| } | ||
|
|
||
| perf_event_attr follower_attr{}; | ||
| ASSERT_EQ(ConfigurePerfEventAttr("INSTRUCTIONS", false, &follower_attr), | ||
| PFM_SUCCESS); | ||
| EXPECT_FALSE(follower_attr.disabled); | ||
| EXPECT_FALSE(follower_attr.pinned); | ||
| } | ||
| #endif | ||
|
|
||
| TEST(PerfCountersTest, OneCounter) { | ||
| if (!HasRequiredPerfCounters({kGenericPerfEvent1})) { | ||
| GTEST_SKIP() << "Requested performance counters are not available."; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this new header for an inline method? why is the enormous method even marked as inline? this could be a private (.cc only) method in the perf_counters.cc file no?