Modify priority tweaking to use CGroups.
This modifies the behavior of setpriority and getpriority
to use CGroups rather than niceness levels.
I've removed the unittest comparing priorities as its not
really valid any more as we are just writing numbers to files.
I've also refactored references to priority to reference cpu
shares etc.
BUG=chromium-os:36229
TEST=Unittest + doing end to end test in bgnd
CQ-DEPEND=I6a0e56073e7281268e0550919c9ec9202b18db26
Change-Id: I48c8270c2065f1e0677e5e53df3557131577b97c
Reviewed-on: https://gerrit.chromium.org/gerrit/39147
Reviewed-by: Chris Sosa <sosa@chromium.org>
Tested-by: Chris Sosa <sosa@chromium.org>
Commit-Ready: Chris Sosa <sosa@chromium.org>
diff --git a/update_attempter.cc b/update_attempter.cc
index e6a897a..3e0249b 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -117,8 +117,8 @@
update_check_scheduler_(NULL),
fake_update_success_(false),
http_response_code_(0),
- priority_(utils::kProcessPriorityNormal),
- manage_priority_source_(NULL),
+ shares_(utils::kCpuSharesNormal),
+ manage_shares_source_(NULL),
download_active_(false),
status_(UPDATE_STATUS_IDLE),
download_progress_(0.0),
@@ -142,7 +142,7 @@
}
UpdateAttempter::~UpdateAttempter() {
- CleanupPriorityManagement();
+ CleanupCpuSharesManagement();
}
void UpdateAttempter::Update(const string& app_version,
@@ -551,8 +551,8 @@
LOG(INFO) << "Processing Done.";
actions_.clear();
- // Reset process priority back to normal.
- CleanupPriorityManagement();
+ // Reset cpu shares back to normal.
+ CleanupCpuSharesManagement();
if (status_ == UPDATE_STATUS_REPORTING_ERROR_EVENT) {
LOG(INFO) << "Error event sent.";
@@ -617,8 +617,8 @@
}
void UpdateAttempter::ProcessingStopped(const ActionProcessor* processor) {
- // Reset process priority back to normal.
- CleanupPriorityManagement();
+ // Reset cpu shares back to normal.
+ CleanupCpuSharesManagement();
download_progress_ = 0.0;
SetStatusAndNotify(UPDATE_STATUS_IDLE, kUpdateNoticeUnspecified);
actions_.clear();
@@ -678,7 +678,7 @@
new_version_ = "0.0.0.0";
new_payload_size_ = plan.payload_size;
SetupDownload();
- SetupPriorityManagement();
+ SetupCpuSharesManagement();
SetStatusAndNotify(UPDATE_STATUS_UPDATE_AVAILABLE,
kUpdateNoticeUnspecified);
} else if (type == DownloadAction::StaticType()) {
@@ -897,41 +897,41 @@
return true;
}
-void UpdateAttempter::SetPriority(utils::ProcessPriority priority) {
- if (priority_ == priority) {
+void UpdateAttempter::SetCpuShares(utils::CpuShares shares) {
+ if (shares_ == shares) {
return;
}
- if (utils::SetProcessPriority(priority)) {
- priority_ = priority;
- LOG(INFO) << "Process priority = " << priority_;
+ if (utils::SetCpuShares(shares)) {
+ shares_ = shares;
+ LOG(INFO) << "CPU shares = " << shares_;
}
}
-void UpdateAttempter::SetupPriorityManagement() {
- if (manage_priority_source_) {
- LOG(ERROR) << "Process priority timeout source hasn't been destroyed.";
- CleanupPriorityManagement();
+void UpdateAttempter::SetupCpuSharesManagement() {
+ if (manage_shares_source_) {
+ LOG(ERROR) << "Cpu shares timeout source hasn't been destroyed.";
+ CleanupCpuSharesManagement();
}
- const int kPriorityTimeout = 2 * 60 * 60; // 2 hours
- manage_priority_source_ = g_timeout_source_new_seconds(kPriorityTimeout);
- g_source_set_callback(manage_priority_source_,
- StaticManagePriorityCallback,
+ const int kCpuSharesTimeout = 2 * 60 * 60; // 2 hours
+ manage_shares_source_ = g_timeout_source_new_seconds(kCpuSharesTimeout);
+ g_source_set_callback(manage_shares_source_,
+ StaticManageCpuSharesCallback,
this,
NULL);
- g_source_attach(manage_priority_source_, NULL);
- SetPriority(utils::kProcessPriorityLow);
+ g_source_attach(manage_shares_source_, NULL);
+ SetCpuShares(utils::kCpuSharesLow);
}
-void UpdateAttempter::CleanupPriorityManagement() {
- if (manage_priority_source_) {
- g_source_destroy(manage_priority_source_);
- manage_priority_source_ = NULL;
+void UpdateAttempter::CleanupCpuSharesManagement() {
+ if (manage_shares_source_) {
+ g_source_destroy(manage_shares_source_);
+ manage_shares_source_ = NULL;
}
- SetPriority(utils::kProcessPriorityNormal);
+ SetCpuShares(utils::kCpuSharesNormal);
}
-gboolean UpdateAttempter::StaticManagePriorityCallback(gpointer data) {
- return reinterpret_cast<UpdateAttempter*>(data)->ManagePriorityCallback();
+gboolean UpdateAttempter::StaticManageCpuSharesCallback(gpointer data) {
+ return reinterpret_cast<UpdateAttempter*>(data)->ManageCpuSharesCallback();
}
gboolean UpdateAttempter::StaticStartProcessing(gpointer data) {
@@ -945,9 +945,9 @@
g_idle_add(&StaticStartProcessing, this);
}
-bool UpdateAttempter::ManagePriorityCallback() {
- SetPriority(utils::kProcessPriorityNormal);
- manage_priority_source_ = NULL;
+bool UpdateAttempter::ManageCpuSharesCallback() {
+ SetCpuShares(utils::kCpuSharesNormal);
+ manage_shares_source_ = NULL;
return false; // Destroy the timeout source.
}
diff --git a/update_attempter.h b/update_attempter.h
index c873aa9..dd47f4d 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -193,22 +193,22 @@
// otherwise.
bool ScheduleErrorEventAction();
- // Sets the process priority to |priority| and updates |priority_| if the new
- // |priority| is different than the current |priority_|, otherwise simply
+ // Sets the cpu shares to |shares| and updates |shares_| if the new
+ // |shares| is different than the current |shares_|, otherwise simply
// returns.
- void SetPriority(utils::ProcessPriority priority);
+ void SetCpuShares(utils::CpuShares shares);
- // Sets the process priority to low and sets up timeout events to increase it.
- void SetupPriorityManagement();
+ // Sets the cpu shares to low and sets up timeout events to increase it.
+ void SetupCpuSharesManagement();
- // Resets the process priority to normal and destroys any scheduled timeout
+ // Resets the cpu shares to normal and destroys any scheduled timeout
// sources.
- void CleanupPriorityManagement();
+ void CleanupCpuSharesManagement();
- // The process priority timeout source callback sets the current priority to
+ // The cpu shares timeout source callback sets the current cpu shares to
// normal. Returns false so that GLib destroys the timeout source.
- static gboolean StaticManagePriorityCallback(gpointer data);
- bool ManagePriorityCallback();
+ static gboolean StaticManageCpuSharesCallback(gpointer data);
+ bool ManageCpuSharesCallback();
// Callback to start the action processor.
static gboolean StaticStartProcessing(gpointer data);
@@ -300,11 +300,11 @@
// HTTP server response code from the last HTTP request action.
int http_response_code_;
- // Current process priority.
- utils::ProcessPriority priority_;
+ // Current cpu shares.
+ utils::CpuShares shares_;
- // The process priority management timeout source.
- GSource* manage_priority_source_;
+ // The cpu shares management timeout source.
+ GSource* manage_shares_source_;
// Set to true if an update download is active (and BytesReceived
// will be called), set to false otherwise.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 43ed55c..937a70f 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -55,8 +55,8 @@
EXPECT_EQ(NULL, attempter_.system_state_);
EXPECT_EQ(NULL, attempter_.update_check_scheduler_);
EXPECT_EQ(0, attempter_.http_response_code_);
- EXPECT_EQ(utils::kProcessPriorityNormal, attempter_.priority_);
- EXPECT_EQ(NULL, attempter_.manage_priority_source_);
+ EXPECT_EQ(utils::kCpuSharesNormal, attempter_.shares_);
+ EXPECT_EQ(NULL, attempter_.manage_shares_source_);
EXPECT_FALSE(attempter_.download_active_);
EXPECT_EQ(UPDATE_STATUS_IDLE, attempter_.status_);
EXPECT_EQ(0.0, attempter_.download_progress_);
diff --git a/utils.cc b/utils.cc
index 1267256..b4763e4 100644
--- a/utils.cc
+++ b/utils.cc
@@ -56,6 +56,10 @@
static const char kDevImageMarker[] = "/root/.dev_mode";
const char* const kStatefulPartition = "/mnt/stateful_partition";
+// Cgroup container is created in update-engine's upstart script located at
+// /etc/init/update-engine.conf.
+static const char kCGroupDir[] = "/sys/fs/cgroup/cpu/update-engine";
+
bool IsOfficialBuild() {
return !file_util::PathExists(FilePath(kDevImageMarker));
}
@@ -624,16 +628,23 @@
g_idle_add(&TriggerCrashReporterUpload, NULL);
}
-bool SetProcessPriority(ProcessPriority priority) {
- int prio = static_cast<int>(priority);
- LOG(INFO) << "Setting process priority to " << prio;
- TEST_AND_RETURN_FALSE(setpriority(PRIO_PROCESS, 0, prio) == 0);
- return true;
+bool SetCpuShares(CpuShares shares) {
+ string string_shares = base::IntToString(static_cast<int>(shares));
+ string cpu_shares_file = string(utils::kCGroupDir) + "/cpu.shares";
+ LOG(INFO) << "Setting cgroup cpu shares to " << string_shares;
+ if(utils::WriteFile(cpu_shares_file.c_str(), string_shares.c_str(),
+ string_shares.size())){
+ return true;
+ } else {
+ LOG(ERROR) << "Failed to change cgroup cpu shares to "<< string_shares
+ << " using " << cpu_shares_file;
+ return false;
+ }
}
-int ComparePriorities(ProcessPriority priority_lhs,
- ProcessPriority priority_rhs) {
- return static_cast<int>(priority_rhs) - static_cast<int>(priority_lhs);
+int CompareCpuShares(CpuShares shares_lhs,
+ CpuShares shares_rhs) {
+ return static_cast<int>(shares_lhs) - static_cast<int>(shares_rhs);
}
int FuzzInt(int value, unsigned int range) {
diff --git a/utils.h b/utils.h
index 555f54c..ffc2f39 100644
--- a/utils.h
+++ b/utils.h
@@ -243,21 +243,24 @@
// Returns empty string on failure.
const std::string BootKernelDevice(const std::string& boot_device);
-enum ProcessPriority {
- kProcessPriorityHigh = -10,
- kProcessPriorityNormal = 0,
- kProcessPriorityLow = 10,
+// Cgroups cpu shares constants. 1024 is the default shares a standard process
+// gets and 2 is the minimum value. We set High as a value that gives the
+// update-engine 2x the cpu share of a standard process.
+enum CpuShares {
+ kCpuSharesHigh = 2048,
+ kCpuSharesNormal = 1024,
+ kCpuSharesLow = 2,
};
-// Compares process priorities and returns an integer that is less
-// than, equal to or greater than 0 if |priority_lhs| is,
-// respectively, lower than, same as or higher than |priority_rhs|.
-int ComparePriorities(ProcessPriority priority_lhs,
- ProcessPriority priority_rhs);
+// Compares cpu shares and returns an integer that is less
+// than, equal to or greater than 0 if |shares_lhs| is,
+// respectively, lower than, same as or higher than |shares_rhs|.
+int CompareCpuShares(CpuShares shares_lhs,
+ CpuShares shares_rhs);
-// Sets the current process priority to |priority|. Returns true on
+// Sets the current process shares to |shares|. Returns true on
// success, false otherwise.
-bool SetProcessPriority(ProcessPriority priority);
+bool SetCpuShares(CpuShares shares);
// Assumes data points to a Closure. Runs it and returns FALSE;
gboolean GlibRunClosure(gpointer data);
diff --git a/utils_unittest.cc b/utils_unittest.cc
index 39ae150..39c8ad7 100644
--- a/utils_unittest.cc
+++ b/utils_unittest.cc
@@ -184,31 +184,15 @@
EXPECT_EQ("3", utils::PartitionNumber("/dev/mmc0p3"));
}
-
-TEST(UtilsTest, RunAsRootSetProcessPriorityTest) {
- // getpriority may return -1 on error so the getpriority logic needs to be
- // enhanced if any of the pre-defined priority constants are changed to -1.
- ASSERT_NE(-1, utils::kProcessPriorityLow);
- ASSERT_NE(-1, utils::kProcessPriorityNormal);
- ASSERT_NE(-1, utils::kProcessPriorityHigh);
- EXPECT_EQ(utils::kProcessPriorityNormal, getpriority(PRIO_PROCESS, 0));
- EXPECT_TRUE(utils::SetProcessPriority(utils::kProcessPriorityHigh));
- EXPECT_EQ(utils::kProcessPriorityHigh, getpriority(PRIO_PROCESS, 0));
- EXPECT_TRUE(utils::SetProcessPriority(utils::kProcessPriorityLow));
- EXPECT_EQ(utils::kProcessPriorityLow, getpriority(PRIO_PROCESS, 0));
- EXPECT_TRUE(utils::SetProcessPriority(utils::kProcessPriorityNormal));
- EXPECT_EQ(utils::kProcessPriorityNormal, getpriority(PRIO_PROCESS, 0));
-}
-
-TEST(UtilsTest, ComparePrioritiesTest) {
- EXPECT_LT(utils::ComparePriorities(utils::kProcessPriorityLow,
- utils::kProcessPriorityNormal), 0);
- EXPECT_GT(utils::ComparePriorities(utils::kProcessPriorityNormal,
- utils::kProcessPriorityLow), 0);
- EXPECT_EQ(utils::ComparePriorities(utils::kProcessPriorityNormal,
- utils::kProcessPriorityNormal), 0);
- EXPECT_GT(utils::ComparePriorities(utils::kProcessPriorityHigh,
- utils::kProcessPriorityNormal), 0);
+TEST(UtilsTest, CompareCpuSharesTest) {
+ EXPECT_LT(utils::CompareCpuShares(utils::kCpuSharesLow,
+ utils::kCpuSharesNormal), 0);
+ EXPECT_GT(utils::CompareCpuShares(utils::kCpuSharesNormal,
+ utils::kCpuSharesLow), 0);
+ EXPECT_EQ(utils::CompareCpuShares(utils::kCpuSharesNormal,
+ utils::kCpuSharesNormal), 0);
+ EXPECT_GT(utils::CompareCpuShares(utils::kCpuSharesHigh,
+ utils::kCpuSharesNormal), 0);
}
TEST(UtilsTest, FuzzIntTest) {