[profile] Introduce role-based scheduler profile API.
Introduce a role-based profile API to avoid hard coding profile
parameters in client code, especially in out-of-tree code that
requires a roll to update.
- Add SetProfileByRole method the ProfileProvider protocol.
- Add skeleton implementation of the SetProfileByRole method.
- Add device_set_profile_by_role wrapper to DDK and fake/no DDK.
- Add tests for SetProfileByRole method.
- Move tests from bootfs to core.
Bug: 40858
Change-Id: Ieca272aa49a6c75f8676b4d278894174576e61e2
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/491260
Reviewed-by: Tom Bergan <tombergan@google.com>
Reviewed-by: Suraj Malhotra <surajmalhotra@google.com>
Reviewed-by: James Robinson <jamesr@google.com>
Reviewed-by: Carlos Pizano <cpu@google.com>
Commit-Queue: Corey Tabaka <eieio@google.com>
API-Review: Carlos Pizano <cpu@google.com>
diff --git a/bundles/bringup/BUILD.gn b/bundles/bringup/BUILD.gn
index 09c4751..851eefb8 100644
--- a/bundles/bringup/BUILD.gn
+++ b/bundles/bringup/BUILD.gn
@@ -34,7 +34,6 @@
"//src/zircon/tests/futex-ownership:futex-ownership-bootfs-test",
"//src/zircon/tests/job-policy:policy-bootfs-test",
"//src/zircon/tests/kcounter:kcounter-bootfs-test",
- "//src/zircon/tests/profile:profile-bootfs-test",
"//src/zircon/tests/thread-state:thread-state-bootfs-test",
"//src/zircon/tests/time:time-bootfs-test",
"//src/zircon/tests/timers:timers-bootfs-test",
diff --git a/garnet/bin/hwstress/profile_manager_test.cc b/garnet/bin/hwstress/profile_manager_test.cc
index 28e4008..03dda23 100644
--- a/garnet/bin/hwstress/profile_manager_test.cc
+++ b/garnet/bin/hwstress/profile_manager_test.cc
@@ -69,6 +69,11 @@
ZX_PANIC("unexpected call");
}
+ void SetProfileByRole(zx::thread thread, std::string role,
+ SetProfileByRoleCallback callback) override {
+ ZX_PANIC("unexpected call");
+ }
+
bool get_affinity_profile_called = false;
bool get_profile_called = false;
uint32_t requested_priority = -1;
diff --git a/sdk/fidl/fuchsia.scheduler/profile.fidl b/sdk/fidl/fuchsia.scheduler/profile.fidl
index 58d0869..a670d26 100644
--- a/sdk/fidl/fuchsia.scheduler/profile.fidl
+++ b/sdk/fidl/fuchsia.scheduler/profile.fidl
@@ -15,12 +15,27 @@
[Discoverable, ForDeprecatedCBindings]
protocol ProfileProvider {
/// Obtain a profile handle.
+ ///
+ /// TODO(fxbug.dev/40858): This API will be deprecated and removed in the future, use
+ /// SetProfileByRole instead.
GetProfile(uint32 priority, string:64 name) -> (zx.status status, zx.handle:PROFILE? profile);
/// Obtain a deadline profile handle.
+ ///
+ /// TODO(fxbug.dev/40858): This API will be deprecated and removed in the future, use
+ /// SetProfileByRole instead.
GetDeadlineProfile(uint64 capacity, uint64 deadline, uint64 period, string:64 name)
-> (zx.status status, zx.handle:PROFILE? profile);
/// Obtain a handle for a profile that sets CPU affinity.
+ ///
+ /// TODO(fxbug.dev/40858): This API will be deprecated and removed in the future, use
+ /// SetProfileByRole instead.
GetCpuAffinityProfile(CpuSet cpu_mask) -> (zx.status status, zx.handle:PROFILE? profile);
+
+ /// Sets the given thread's profile based on the requested role. The exact
+ /// parameters of the profile are system dependent and may vary based on
+ /// device-specific tuning and/or runtime system goals.
+ SetProfileByRole(zx.handle:THREAD thread, string:2048 role)
+ -> (zx.status status);
};
diff --git a/src/devices/bin/driver_host/api.cc b/src/devices/bin/driver_host/api.cc
index bbba538..f6d4717 100644
--- a/src/devices/bin/driver_host/api.cc
+++ b/src/devices/bin/driver_host/api.cc
@@ -244,6 +244,11 @@
return internal::get_scheduler_deadline_profile(capacity, deadline, period, name, out_profile);
}
+__EXPORT zx_status_t device_set_profile_by_role(zx_device_t* device, zx_handle_t thread,
+ const char* role, size_t role_size) {
+ return internal::set_scheduler_profile_by_role(thread, role, role_size);
+}
+
__EXPORT const char* device_get_name(zx_device_t* dev) { return dev->name(); }
__EXPORT zx_device_t* device_get_parent(zx_device_t* dev) {
diff --git a/src/devices/bin/driver_host/scheduler_profile.cc b/src/devices/bin/driver_host/scheduler_profile.cc
index ead171b2..507b9e9 100644
--- a/src/devices/bin/driver_host/scheduler_profile.cc
+++ b/src/devices/bin/driver_host/scheduler_profile.cc
@@ -8,6 +8,7 @@
#include <lib/fdio/directory.h>
#include <lib/zx/channel.h>
#include <lib/zx/profile.h>
+#include <lib/zx/thread.h>
#include <stdio.h>
#include <string.h>
#include <zircon/types.h>
@@ -62,4 +63,24 @@
return ZX_OK;
}
+zx_status_t set_scheduler_profile_by_role(zx_handle_t thread, const char* role, size_t role_size) {
+ zx::unowned_thread original_thread{thread};
+ zx::thread duplicate_thread;
+ zx_status_t status =
+ original_thread->duplicate(ZX_RIGHT_TRANSFER | ZX_RIGHT_MANAGE_THREAD, &duplicate_thread);
+ if (status != ZX_OK) {
+ return status;
+ }
+
+ zx_status_t fidl_status = ZX_ERR_INTERNAL;
+ status = fuchsia_scheduler_ProfileProviderSetProfileByRole(scheduler_profile_provider, thread,
+ role, role_size, &fidl_status);
+ if (status != ZX_OK) {
+ return status;
+ }
+ if (fidl_status != ZX_OK) {
+ return fidl_status;
+ }
+ return ZX_OK;
+}
} // namespace internal
diff --git a/src/devices/bin/driver_host/scheduler_profile.h b/src/devices/bin/driver_host/scheduler_profile.h
index 220ca85..303ee8f 100644
--- a/src/devices/bin/driver_host/scheduler_profile.h
+++ b/src/devices/bin/driver_host/scheduler_profile.h
@@ -13,6 +13,7 @@
zx_status_t get_scheduler_deadline_profile(uint64_t capacity, uint64_t deadline, uint64_t period,
const char* name, zx_handle_t* profile);
+zx_status_t set_scheduler_profile_by_role(zx_handle_t thread, const char* role, size_t role_size);
} // namespace internal
#endif // SRC_DEVICES_BIN_DRIVER_HOST_SCHEDULER_PROFILE_H_
diff --git a/src/devices/testing/fake_ddk/fake_ddk.cc b/src/devices/testing/fake_ddk/fake_ddk.cc
index 22c9e36..92e0cb4 100644
--- a/src/devices/testing/fake_ddk/fake_ddk.cc
+++ b/src/devices/testing/fake_ddk/fake_ddk.cc
@@ -537,6 +537,13 @@
}
__EXPORT
+zx_status_t device_set_profile_by_role(zx_device_t* device, zx_handle_t thread, const char* role,
+ size_t role_size) {
+ // This is currently a no-op.
+ return ZX_OK;
+}
+
+__EXPORT
void device_fidl_transaction_take_ownership(fidl_txn_t* txn, device_fidl_txn_t* new_txn) {
auto fidl_txn = fake_ddk::FromDdkInternalTransaction(ddk::internal::Transaction::FromTxn(txn));
diff --git a/src/devices/testing/no_ddk/no_ddk.cc b/src/devices/testing/no_ddk/no_ddk.cc
index 6411561..8d278d3a 100644
--- a/src/devices/testing/no_ddk/no_ddk.cc
+++ b/src/devices/testing/no_ddk/no_ddk.cc
@@ -97,6 +97,13 @@
return ZX_OK;
}
+__EXPORT
+zx_status_t device_set_profile_by_role(zx_device_t* device, zx_handle_t thread, const char* role,
+ size_t role_size) {
+ // This is currently a no-op.
+ return ZX_OK;
+}
+
__EXPORT __WEAK zx_status_t load_firmware(zx_device_t* device, const char* path, zx_handle_t* fw,
size_t* size) {
// This is currently a no-op.
diff --git a/src/lib/ddk/include/lib/ddk/driver.h b/src/lib/ddk/include/lib/ddk/driver.h
index ba5f79c..e8375a9 100644
--- a/src/lib/ddk/include/lib/ddk/driver.h
+++ b/src/lib/ddk/include/lib/ddk/driver.h
@@ -264,15 +264,36 @@
// priority.
//
// The current arguments are transitional, and will likely change in the future.
+//
+// TODO(fxbug.dev/40858): This API will be deprecated and removed in the future, use
+// device_set_profile_by_role instead.
zx_status_t device_get_profile(zx_device_t* device, uint32_t priority, const char* name,
zx_handle_t* out_profile);
// Retrieves a deadline profile handle into |out_profile| from the scheduler for
// the given deadline parameters. See |device_get_profile|
+//
+// TODO(fxbug.dev/40858): This API will be deprecated and removed in the future, use
+// device_set_profile_by_role instead.
zx_status_t device_get_deadline_profile(zx_device_t* device, uint64_t capacity, uint64_t deadline,
uint64_t period, const char* name,
zx_handle_t* out_profile);
+// Requests that the given thread be assigned a profile with parameters appropriate for the given
+// role. The available roles and the specific parameters assigned are device-dependent and may also
+// vary across builds. Requests are not guaranteed to be honored for all roles and requestors, and
+// may either be rejected or silently ignored, based on system-specific criteria.
+//
+// |role| is name of the requested role. This must not be nullptr, even if |role_size| is zero.
+//
+// |thread| is a borrowed handle that must have DUPLICATE, TRANSFER, and MANAGE_THREAD rights. It is
+// duplicated internally before transfer to the ProfileProvider service to complete the profile
+// assignment.
+//
+// This API should be used in preference to hard coding parameters in drivers.
+zx_status_t device_set_profile_by_role(zx_device_t* device, zx_handle_t thread, const char* role,
+ size_t role_size);
+
// A description of a part of a device fragment. It provides a bind program
// that will match a device on the path from the root of the device tree to the
// target device.
diff --git a/src/media/audio/audio_core/testing/fake_profile_provider.h b/src/media/audio/audio_core/testing/fake_profile_provider.h
index 8aa5b4b..cdaf873 100644
--- a/src/media/audio/audio_core/testing/fake_profile_provider.h
+++ b/src/media/audio/audio_core/testing/fake_profile_provider.h
@@ -50,6 +50,12 @@
void GetCpuAffinityProfile(fuchsia::scheduler::CpuSet cpu_mask,
GetProfileCallback callback) override {}
+ // |fuchsia::scheduler::ProfileProvider|
+ void SetProfileByRole(zx::thread thread, std::string role,
+ SetProfileByRoleCallback callback) override {
+ callback(ZX_ERR_NOT_SUPPORTED);
+ }
+
std::unordered_set<uint32_t> valid_priorities_;
fidl::BindingSet<fuchsia::scheduler::ProfileProvider> bindings_;
};
diff --git a/src/zircon/tests/BUILD.gn b/src/zircon/tests/BUILD.gn
index 46f3a4d..80e1048 100644
--- a/src/zircon/tests/BUILD.gn
+++ b/src/zircon/tests/BUILD.gn
@@ -12,6 +12,7 @@
"kernel-clocks:tests",
"kernel-cmdline:tests",
"processor:tests",
+ "profile:tests",
"property:tests",
"register-state:tests",
"status:tests",
diff --git a/src/zircon/tests/profile/BUILD.gn b/src/zircon/tests/profile/BUILD.gn
index fe60c58..e549b36a 100644
--- a/src/zircon/tests/profile/BUILD.gn
+++ b/src/zircon/tests/profile/BUILD.gn
@@ -3,7 +3,7 @@
# found in the LICENSE file.
import("//build/test.gni")
-import("//build/testing/bootfs_test.gni")
+import("//src/sys/build/fuchsia_unittest_package.gni")
test("profile") {
output_name = "profile-test"
@@ -18,7 +18,12 @@
]
}
-bootfs_test("profile-bootfs-test") {
- name = "profile-test"
+fuchsia_unittest_package("profile-test-pkg") {
+ manifest = "meta/profile-test.cmx"
deps = [ ":profile" ]
}
+
+group("tests") {
+ testonly = true
+ deps = [ ":profile-test-pkg" ]
+}
diff --git a/src/zircon/tests/profile/meta/profile-test.cmx b/src/zircon/tests/profile/meta/profile-test.cmx
new file mode 100644
index 0000000..ebe3a56
--- /dev/null
+++ b/src/zircon/tests/profile/meta/profile-test.cmx
@@ -0,0 +1,20 @@
+{
+ "facets": {
+ "fuchsia.test": {
+ "system-services": [
+ "fuchsia.scheduler.ProfileProvider"
+ ]
+ }
+ },
+ "include": [
+ "sdk/lib/diagnostics/syslog/client.shard.cmx"
+ ],
+ "program": {
+ "binary": "test/profile-test"
+ },
+ "sandbox": {
+ "services": [
+ "fuchsia.scheduler.ProfileProvider"
+ ]
+ }
+}
\ No newline at end of file
diff --git a/src/zircon/tests/profile/profile.cc b/src/zircon/tests/profile/profile.cc
index 26bf947..cb9ca16 100644
--- a/src/zircon/tests/profile/profile.cc
+++ b/src/zircon/tests/profile/profile.cc
@@ -10,8 +10,13 @@
#include <lib/zx/channel.h>
#include <lib/zx/object.h>
#include <lib/zx/profile.h>
+#include <lib/zx/thread.h>
+#include <threads.h>
+#include <zircon/errors.h>
#include <zircon/syscalls.h>
+#include <string>
+
#include <zxtest/zxtest.h>
namespace {
@@ -21,7 +26,7 @@
zx::profile* profile) {
zx_handle_t raw_profile_handle;
zx_status_t result = fuchsia_scheduler_ProfileProviderGetProfile(
- profile_provider.get(), priority, name.c_str(), name.length(), server_status,
+ profile_provider.get(), priority, name.data(), name.length(), server_status,
&raw_profile_handle);
if (result != ZX_OK) {
return ZX_OK;
@@ -36,7 +41,7 @@
zx::profile* profile) {
zx_handle_t raw_profile_handle;
zx_status_t result = fuchsia_scheduler_ProfileProviderGetDeadlineProfile(
- profile_provider.get(), capacity, relative_deadline, period, name.c_str(), name.length(),
+ profile_provider.get(), capacity, relative_deadline, period, name.data(), name.length(),
server_status, &raw_profile_handle);
if (result != ZX_OK) {
return ZX_OK;
@@ -45,6 +50,18 @@
return ZX_OK;
}
+zx_status_t SetProfileByRole(const zx::channel& profile_provider, zx::unowned_thread thread,
+ const std::string& role, zx_status_t* server_status) {
+ zx::thread duplicate;
+ if (zx_status_t status =
+ thread->duplicate(ZX_RIGHT_TRANSFER | ZX_RIGHT_MANAGE_THREAD, &duplicate);
+ status != ZX_OK) {
+ return status;
+ }
+ return fuchsia_scheduler_ProfileProviderSetProfileByRole(
+ profile_provider.get(), duplicate.release(), role.data(), role.length(), server_status);
+}
+
void GetProfileProvider(zx::channel* channel) {
// Connect to ProfileProvider.
zx::channel server_endpoint;
@@ -110,4 +127,27 @@
CheckBasicDetails(profile);
}
+// Test setting a profile via the SetProfileByRole FIDL method.
+TEST(Profile, SetProfileByRole) {
+ zx::channel provider;
+ GetProfileProvider(&provider);
+ ASSERT_FALSE(CURRENT_TEST_HAS_FAILURES());
+
+ zx_status_t server_status;
+ zx_status_t status;
+
+ status = SetProfileByRole(provider, zx::thread::self(), "fuchsia.test-role:ok", &server_status);
+ EXPECT_OK(status);
+ EXPECT_OK(server_status);
+
+ status = SetProfileByRole(provider, zx::unowned_thread{}, "fuchsia.test-role:ok", &server_status);
+ EXPECT_EQ(ZX_ERR_BAD_HANDLE, status);
+ EXPECT_OK(server_status);
+
+ status =
+ SetProfileByRole(provider, zx::thread::self(), "fuchsia.test-role:not-found", &server_status);
+ EXPECT_OK(status);
+ EXPECT_EQ(ZX_ERR_NOT_FOUND, server_status);
+}
+
} // namespace
diff --git a/zircon/system/ulib/profile/profile.cc b/zircon/system/ulib/profile/profile.cc
index 3afa770..0f6d9cb 100644
--- a/zircon/system/ulib/profile/profile.cc
+++ b/zircon/system/ulib/profile/profile.cc
@@ -5,15 +5,21 @@
#include "zircon/syscalls/profile.h"
#include <fuchsia/scheduler/c/fidl.h>
+#include <inttypes.h>
#include <lib/fidl-async/bind.h>
#include <lib/profile/profile.h>
#include <lib/syslog/global.h>
#include <lib/zx/profile.h>
#include <string.h>
#include <zircon/assert.h>
+#include <zircon/errors.h>
+#include <zircon/status.h>
#include <zircon/syscalls.h>
+#include <zircon/syscalls/object.h>
+#include <zircon/types.h>
#include <algorithm>
+#include <string>
namespace {
@@ -66,10 +72,58 @@
txn, status, status == ZX_OK ? profile.release() : ZX_HANDLE_INVALID);
}
+zx_status_t SetProfileByRoleSimple(void* ctx, zx_handle_t thread, const char* role_data,
+ size_t role_size, fidl_txn_t* txn) {
+ auto root_job = static_cast<zx_handle_t>(reinterpret_cast<uintptr_t>(ctx));
+
+ // Log the requested role and PID:TID of the thread being assigned.
+ zx_info_handle_basic_t handle_info{};
+ zx_status_t status = zx_object_get_info(thread, ZX_INFO_HANDLE_BASIC, &handle_info,
+ sizeof(handle_info), nullptr, nullptr);
+ if (status != ZX_OK) {
+ FX_LOGF(WARNING, "ProfileProvider", "Failed to get info for thread handle: %s",
+ zx_status_get_string(status));
+ handle_info.koid = ZX_KOID_INVALID;
+ handle_info.related_koid = ZX_KOID_INVALID;
+ }
+ const std::string role{role_data, role_size};
+ FX_LOGF(INFO, "ProfileProvider", "Role \"%s\" requested by %" PRId64 ":%" PRId64, role.c_str(),
+ handle_info.related_koid, handle_info.koid);
+
+ // Select the profile parameters based on the requested role. New roles and
+ // logic for selecting parameters may be added here as needed.
+ //
+ // TODO(fxbug.dev/40858): Move the role definitions into a device-specific
+ // configuration file.
+ zx_profile_info_t info = {};
+ if (role == "fuchsia.default") {
+ info.flags = ZX_PROFILE_INFO_FLAG_PRIORITY;
+ info.priority = ZX_PRIORITY_DEFAULT;
+ } else if (role == "fuchsia.test-role:not-found") {
+ return fuchsia_scheduler_ProfileProviderSetProfileByRole_reply(txn, ZX_ERR_NOT_FOUND);
+ } else if (role == "fuchsia.test-role:ok") {
+ return fuchsia_scheduler_ProfileProviderSetProfileByRole_reply(txn, ZX_OK);
+ } else {
+ return fuchsia_scheduler_ProfileProviderSetProfileByRole_reply(txn, ZX_ERR_NOT_FOUND);
+ }
+
+ zx::profile profile;
+ status = zx_profile_create(root_job, 0u, &info, profile.reset_and_get_address());
+ if (status != ZX_OK) {
+ // Failing to create a profile is likely due to a programming error in
+ // this handler. The most likely cause is invalid profile parameters.
+ return fuchsia_scheduler_ProfileProviderSetProfileByRole_reply(txn, ZX_ERR_INTERNAL);
+ }
+
+ status = zx_object_set_profile(thread, profile.get(), 0);
+ return fuchsia_scheduler_ProfileProviderSetProfileByRole_reply(txn, status);
+}
+
fuchsia_scheduler_ProfileProvider_ops ops = {
.GetProfile = GetProfileSimple,
.GetDeadlineProfile = GetDeadlineProfileSimple,
.GetCpuAffinityProfile = GetCpuAffinityProfileSimple,
+ .SetProfileByRole = SetProfileByRoleSimple,
};
constexpr const char* profile_svc_names[] = {
