From 38db6bd76f4064783b48ef98d098169cd42299fc Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Tue, 14 Feb 2017 10:15:44 +0900 Subject: [PATCH 01/16] pass: Fix minor typo of comment This patch just the minor typo (hotplulg -> hotplug). Change-Id: I3afb20902936397ba02521921ced9d78fc8caf37 Signed-off-by: Chanwoo Choi --- src/pass/pass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pass/pass.c b/src/pass/pass.c index 382e0e2..5a7d6e7 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -182,7 +182,7 @@ static int pass_resource_init(struct pass_policy *policy) } policy->governor->gov_timeout = policy->gov_timeout; - /* Get the instance of PASS hotplulg */ + /* Get the instance of PASS hotplug */ policy->hotplug = pass_get_hotplug(policy, policy->gov_type); if (policy->hotplug) { policy->hotplug->sequence = calloc(pass_res->cdata.num_cpus, -- 2.7.4 From 01eb2e912fd7e5afe90e6756a9b728226153b7e6 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Tue, 14 Feb 2017 13:17:18 +0900 Subject: [PATCH 02/16] pass: hal: Remove unused pass_get_num_cpus() This patch remvoes the unused pass_get_num_cpus() which get the maximum number of possible online cpus. After applying the commit 86cd810d6e1a ("pass: Add new helper function to handle the h/w resource through hal interface"), the core uses the 'num_cpus' field in the struct pass_conf_data instead of the pass_get_num_cpus(). Change-Id: I234bace1c199c03ce1a1f7e4d0b933633b6198d6 Signed-off-by: Chanwoo Choi --- src/pass/pass-hal.c | 17 ----------------- src/pass/pass-hal.h | 1 - 2 files changed, 18 deletions(-) diff --git a/src/pass/pass-hal.c b/src/pass/pass-hal.c index 18629cf..851f93b 100644 --- a/src/pass/pass-hal.c +++ b/src/pass/pass-hal.c @@ -537,26 +537,9 @@ int pass_get_resource(struct pass *pass) * FXIME: Following function is not standard interface. * Following functions will be altered by the standard interface on later. * - * - int pass_get_num_cpus(void) * - int pass_get_cpu_stats(struct pass_policy *policy) * - int64_t pass_get_time_ms(void) */ -int pass_get_num_cpus(void) -{ - int num_cpus; - int ret; - - ret = sys_get_int("/sys/devices/system/cpu/kernel_max", &num_cpus); - if (ret < 0) - return -EAGAIN; - - /* - * Return the number of cpus with plus 1 because of index - * of first CPU is 0 (zero). (e.g., Octa cores have the '7' value - * of "/sys/devices/system/cpu/kernel_max"). - */ - return (num_cpus + 1); -} /* Get the load_table of each resource to estimate the system load. */ int pass_get_cpu_stats(struct pass_policy *policy) diff --git a/src/pass/pass-hal.h b/src/pass/pass-hal.h index 77f5326..09eb879 100644 --- a/src/pass/pass-hal.h +++ b/src/pass/pass-hal.h @@ -62,7 +62,6 @@ int pass_get_resource(struct pass *pass); * FXIME: Following function is not standard interface. * These functions will be altered by the standard interface on later. */ -int pass_get_num_cpus(void); int pass_get_cpu_stats(struct pass_policy *policy); int64_t pass_get_time_ms(void); -- 2.7.4 From 8f5c594ba10ffb4bd4390454d85b5f8408997528 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 14 Feb 2017 17:28:03 +0900 Subject: [PATCH 03/16] pass: parser: Remove parsing of pass_compatible in resource conf This patch removes the parsing mechanism for the entry, pass_compatible, in the Pass section of the resource conf file. Change-Id: I27894e8cb97f798d29f215ee04a9c6af1fa51087 Signed-off-by: Wook Song --- src/pass/pass-parser.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index 5ae6cf1..b84a151 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -174,20 +174,7 @@ static int pass_parse_core(struct parse_result *result, void *user_data) if (!result->section || !result->name || !result->value) return 0; - if (MATCH(result->name, "pass_compatible")) { - char compatible[BUFF_MAX]; - int len, ret; - - ret = sys_get_str(PROC_DT_COMPATIBLE, compatible); - if (ret < 0) - return -EEXIST; - - len = strlen(result->value); - if (strncmp(compatible, result->value, len)) { - _I("Mismatch compatible string : %s\n", result->value); - return -EINVAL; - } - } else if (MATCH(result->name, "pass_support")) + if (MATCH(result->name, "pass_support")) policy->state = atoi(result->value); else if (MATCH(result->name, "pass_gov_type")) policy->gov_type = atoi(result->value); -- 2.7.4 From 895f20ba1d97f187b57ca98a441543b0bbe652b7 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Wed, 15 Feb 2017 12:41:20 +0900 Subject: [PATCH 04/16] pass: parser: Add parsing of pass_path_compatible This patch adds a parsing mechanism for the new mandatory entry, pass_path_compatible, in the PassResource section of the pass.conf file. This entry indicates the path where the pass can get the compatible board information. Change-Id: Ib4e12d8500d62585682319c18af38727e4aa6bac Signed-off-by: Wook Song --- src/pass/pass-parser.c | 76 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 14 deletions(-) diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index b84a151..d07a9cf 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -29,9 +29,6 @@ #define BUFF_MAX 255 -/* Linux standard sysfs node */ -#define PROC_DT_COMPATIBLE "/proc/device-tree/compatible" - static enum pass_state is_supported(char *value) { enum pass_state state; @@ -488,12 +485,49 @@ static int pass_parse_resource_data(struct parse_result *result, return 0; } +static int check_compatible(char *compatible, char *path_compatible) +{ + char buf[BUFF_MAX]; + int len, ret; + + len = strlen(compatible); + if (!len) { + _E("cannot parse compatible\n"); + return -EINVAL; + } + + len = strlen(path_compatible); + if (!len) { + _E("cannot parse path_compatible\n"); + return -EINVAL; + } + + ret = sys_get_str(path_compatible, buf); + if (ret < 0) { + _E("cannot read compatible path: %s\n", + path_compatible); + return -EINVAL; + } + + len = strlen(compatible); + if (strncmp(compatible, buf, len)) { + _E("cannot match compatible pairs: conf = %s, read = %s\n", + compatible, buf); + return -EINVAL; + } + + return 0; +} static int pass_resource_config(struct parse_result *result, void *user_data) { + static char path_compatible[BUFF_MAX]; + static bool is_compatible = false; + static char compatible[BUFF_MAX]; struct pass *pass = user_data; char section_name[BUFF_MAX]; - int id, ret; + char buf[BUFF_MAX]; + int id, len, ret; if (!result) return 0; @@ -504,18 +538,28 @@ static int pass_resource_config(struct parse_result *result, void *user_data) /* Parsing 'PassResource' section */ if (MATCH(result->section, "PassResource")) { if (MATCH(result->name, "pass_compatible")) { - char compatible[BUFF_MAX]; - int len; + len = strlen(result->value); + strncpy(compatible, result->value, len); - ret = sys_get_str(PROC_DT_COMPATIBLE, compatible); - if (ret < 0) - return -EEXIST; + len = strlen(path_compatible); + if (len > 0) { + ret = check_compatible(compatible, path_compatible); + if (ret < 0) + return ret; + is_compatible = true; + } + } else if (MATCH(result->name, "pass_path_compatible")) { len = strlen(result->value); - if (strncmp(compatible, result->value, len)) { - _I("Mismatch compatible string : %s\n", - result->value); - return -EINVAL; + strncpy(path_compatible, result->value, len); + + len =strlen(compatible); + if (len > 0) { + ret = check_compatible(compatible, path_compatible); + if (ret < 0) + return ret; + + is_compatible = true; } } else if (MATCH(result->name, "pass_num_resources")) { pass->num_resources = atoi(result->value); @@ -536,7 +580,11 @@ static int pass_resource_config(struct parse_result *result, void *user_data) for (id = 0; id < pass->num_resources; id++) { ret = sprintf(section_name, "PassResource%d", id); - if (MATCH(result->section, section_name)) { + if (MATCH(section_name, result->section)) { + if (!is_compatible) { + _E("cannot match [compatible] with the string read from [path_compatible]\n"); + return -EINVAL; + } ret = pass_parse_resource_data(result, pass, id); if (ret < 0) { _E("cannot parse 'PassResource%d' section\n", -- 2.7.4 From ed5687c4e35cf943b1a5371031889764057e76ad Mon Sep 17 00:00:00 2001 From: Wook Song Date: Wed, 15 Feb 2017 16:35:56 +0900 Subject: [PATCH 05/16] pass: Fix build warnings This patch fixes build warnings caused by unused variables. Change-Id: I5fbd470a4f76571638815a2a9174fa28b138a75c Signed-off-by: Wook Song --- src/pass/pass-gov.c | 10 +++------- src/pass/pass-parser.c | 1 - src/pass/pass-pmqos.c | 21 --------------------- 3 files changed, 3 insertions(+), 29 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index cda0c61..3f1528a 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -163,8 +163,6 @@ static void pass_hotplug_set_online(struct pass_policy *policy, */ static void pass_hotplug_stop(struct pass_policy *policy) { - struct pass_resource *pass_res = to_pass_resource(policy); - struct pass_conf_data *cdata = &pass_res->cdata; struct pass_table *table = policy->pass_table; int level = policy->max_level; @@ -232,7 +230,6 @@ static int pass_governor_change_level(struct pass_policy *policy, int new_level) int limit_max_freq; int limit_min_freq; int limit_min_cpu; - int online; int ret; if (new_level > policy->max_level) @@ -250,7 +247,6 @@ static int pass_governor_change_level(struct pass_policy *policy, int new_level) */ limit_max_freq = table[new_level].limit_max_freq; limit_min_freq = table[new_level].limit_min_freq; - limit_min_cpu = table[new_level].limit_min_cpu; policy->prev_level = policy->curr_level; policy->curr_level = new_level; @@ -258,11 +254,11 @@ static int pass_governor_change_level(struct pass_policy *policy, int new_level) /* Turn on/off CPUs according the maximum number of online CPU */ if (hotplug) { if (hotplug->governor) - online = hotplug->governor(policy); + limit_min_cpu = hotplug->governor(policy); else - online = 1; + limit_min_cpu = 1; - pass_hotplug_set_online(policy, online); + pass_hotplug_set_online(policy, limit_min_cpu); } /* Set maximum frequency */ diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index d07a9cf..e10ed52 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -526,7 +526,6 @@ static int pass_resource_config(struct parse_result *result, void *user_data) static char compatible[BUFF_MAX]; struct pass *pass = user_data; char section_name[BUFF_MAX]; - char buf[BUFF_MAX]; int id, len, ret; if (!result) diff --git a/src/pass/pass-pmqos.c b/src/pass/pass-pmqos.c index eccfd8e..20be7e4 100644 --- a/src/pass/pass-pmqos.c +++ b/src/pass/pass-pmqos.c @@ -57,25 +57,6 @@ static bool is_pmqos_enabled(struct pass_policy *policy) return true; } -/* - * is_scenario_locked - Check locked state of each scenario - * @policy: instance of pass_scenario structure - * - * Return true if scenario is locked and enabled - * Return false if scenario is unlocked or disabled - */ - -static bool is_scenario_locked(struct pass_scenario *scn) -{ - if (!scn) - return false; - - if (!scn->locked || scn->state != PASS_ON) - return false; - - return true; -} - static enum pass_state is_pmqos_locked(char *data, char *name) { char *unlock = NULL; @@ -123,9 +104,7 @@ int pass_notifier_pmqos_func(struct pass_policy *policy, void *data) unsigned int new_level = 0; unsigned int min_level = 0; unsigned int max_level = 0; - int count = 0; int index = -1; - int i; if (!is_pmqos_enabled(policy)) return 0; -- 2.7.4 From 8f3f035444125fb3caf6a00abd960da1b033c3cc Mon Sep 17 00:00:00 2001 From: Wook Song Date: Wed, 22 Feb 2017 10:58:24 +0900 Subject: [PATCH 06/16] shared: Add missing free for avoiding memory leaks This patch adds missing free() for avoiding possible memory leaks in the shared directory. Change-Id: Ie95db189d3d94d08d865ad3d7470d801b47c6dc1 Signed-off-by: Wook Song --- src/shared/pass-systemd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/pass-systemd.c b/src/shared/pass-systemd.c index d418da2..b3a3c12 100644 --- a/src/shared/pass-systemd.c +++ b/src/shared/pass-systemd.c @@ -197,7 +197,7 @@ static char *pass_systemd_get_unit_dbus_path(const char *unit) p += k+1; } } - + free(path); return path; } -- 2.7.4 From 34aaab07db0af719882c86d91f447310c0d4dd5d Mon Sep 17 00:00:00 2001 From: Wook Song Date: Thu, 23 Feb 2017 14:37:09 +0900 Subject: [PATCH 07/16] core: Add macro which wraps g_list_free_full() This patch adds a macro which wraps g_list_free_full(). This functions frees all the dynamically-allocated data in the GList element so that we can use it to clean up GLists well. Change-Id: I2fd538f39620ca1044b5ba3046a8a82a8bd598c4 Signed-off-by: Wook Song --- src/core/list.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/list.h b/src/core/list.h index c57452a..261ce4f 100644 --- a/src/core/list.h +++ b/src/core/list.h @@ -38,6 +38,8 @@ typedef GList dd_list; g_list_find(a, (gpointer)b) #define DD_LIST_FREE_LIST(a) \ g_list_free(a) +#define DD_LIST_FREE_LIST_FULL(a, free_func) \ + g_list_free_full(a, free_func) #define DD_LIST_SORT(a, func) \ a = g_list_sort(a, func) #define DD_LIST_FOREACH(head, elem, node) \ -- 2.7.4 From c7424f5071cccf3497497db4952f6c22aac289c4 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Thu, 23 Feb 2017 14:55:27 +0900 Subject: [PATCH 08/16] pmqos: Add cleanup function on normal exit This patch adds a cleanup function, that frees all the dynamically allocated data and assigns 0 to the static variables. This cleanup function is invoked when the pmqos module is normally terminated. Change-Id: I0471f3ca13a4ba8a385b7829afab16dbf68f4988 Signed-off-by: Wook Song --- src/pmqos/pmqos.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/pmqos/pmqos.c b/src/pmqos/pmqos.c index 395a513..ea40cb7 100644 --- a/src/pmqos/pmqos.c +++ b/src/pmqos/pmqos.c @@ -49,7 +49,7 @@ static dd_list *pmqos_head; static Ecore_Timer *unlock_timer; static struct timespec unlock_timer_start_st; static struct timespec unlock_timer_end_st; -static struct pmqos_cpu unlock_timer_owner = {"NULL", 0}; +static struct pmqos_cpu unlock_timer_owner = {"", 0}; int set_pmqos(const char *name, int val) { @@ -455,9 +455,30 @@ static void pmqos_init(void *data) register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, booting_done, NULL); } +static void pmqos_free(void) +{ + /* Assign 0 to static variables */ + memset(&unlock_timer_start_st, 0, sizeof(struct timespec)); + memset(&unlock_timer_end_st, 0, sizeof(struct timespec)); + memset(&unlock_timer_owner, 0, sizeof(struct pmqos_cpu)); + + /* Clean up pmqos_head */ + if (pmqos_head) { + DD_LIST_FREE_LIST_FULL(pmqos_head, free); + pmqos_head = NULL; + } + + /* Clean up unlock_timer */ + if (unlock_timer) { + ecore_timer_del(unlock_timer); + unlock_timer = NULL; + } +} + static void pmqos_exit(void *data) { unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, booting_done); + pmqos_free(); } static const struct device_ops pmqos_device_ops = { -- 2.7.4 From 0f18ebc087c6a596fa1c819daac97be8e3cd9b16 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 24 Feb 2017 11:04:39 +0900 Subject: [PATCH 09/16] pmqos: parser: Initialize number of scenarios in pmqos_put_scenario This patch adds initialization of the variable that indicates the number of scenarios to the pmqos_put_scenario function. Change-Id: I22a041054324f0def2db0781c123471a0832de7e Signed-off-by: Wook Song --- src/pmqos/pmqos-parser.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pmqos/pmqos-parser.c b/src/pmqos/pmqos-parser.c index 690a465..5caae75 100644 --- a/src/pmqos/pmqos-parser.c +++ b/src/pmqos/pmqos-parser.c @@ -129,8 +129,10 @@ int pmqos_put_scenario(struct pmqos_scenario *scenarios) if (!scenarios) return -EINVAL; - if (scenarios->num > 0 && !scenarios->list) + if (scenarios->num > 0 && !scenarios->list) { + scenarios->num = 0; free(scenarios->list); + } return 0; } -- 2.7.4 From 3217834ce2456b70c8492c52ec29c877b2b06c85 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 24 Feb 2017 13:26:09 +0900 Subject: [PATCH 10/16] pmqos: Add missing pmqos_put_scenario for freeing data This patch adds missing pmqos_put_scenario function calls for avoiding possible memory leaks. Change-Id: If3372c279d8115106c9e9cf96a205d080759f7a5 Signed-off-by: Wook Song --- src/pmqos/pmqos.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pmqos/pmqos.c b/src/pmqos/pmqos.c index ea40cb7..baaf748 100644 --- a/src/pmqos/pmqos.c +++ b/src/pmqos/pmqos.c @@ -356,7 +356,10 @@ static int get_methods_from_conf(const char *path, struct edbus_method **edbus_m } *edbus_methods = methods; - return scenarios.num; + ret = scenarios.num; + pmqos_put_scenario(&scenarios); + + return ret; } /* Add pmqos name as alphabetically */ -- 2.7.4 From 1dc51132943017adb9726ffa0de6b440b729b490 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Fri, 17 Feb 2017 15:25:43 +0900 Subject: [PATCH 11/16] pass: Add new PASS_GOV_BASIC type and adjust the unique number of governor This patch adds new PASS_GOV_BASIC type which uses the 'HAL interface' and 'Resource controller'. For example, the bus/gpu resources use this governor type in order to control the min/max frequency when pass recevies the pmqos debus message with scenario name. * Detailed description according to each governor type: --------------------------------------------------------------------------- | Governor name | HAL | Resource | Hotplug | Runtime | | | interface | controller | interface | governor | -------------------------------------------------------------------------- | GOV_DUMMY | Used | Un-used | Un-used | Un-used | | GOV_BASIC | Used | Used | Un-used | Un-used | | GOV_HOTPLUG_ONLY | Used | Used | Used | Un-used | | GOV_RADIATION | Used | Used | Used | Used (Radiation)| | GOV_STEP | Used | Used | Used | Used (Step) | --------------------------------------------------------------------------- * Detailed description according to PASS's feature: - HAL interface : Call the HAL implementation such as pass-hal-(board).rpm - Resource controller: Control the h/w resource such as CPU/BUS/GPU resource. - Hotplug interface : Turn on/off the CPU h/w resource. - Runtime governor : Monitor the system status and then decide the proper level. After deciding the level, it control the h/w resource by using both 'Resource controller' and 'Hotplug interface'. Change-Id: I3b7d7a8f5142baa409cbbfe221afad8decfb2149 Signed-off-by: Chanwoo Choi --- src/pass/pass-gov.c | 8 ++++++++ src/pass/pass.h | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index 3f1528a..5e94a9b 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -191,10 +191,13 @@ struct pass_hotplug* pass_get_hotplug(struct pass_policy *policy, switch (type) { case PASS_GOV_DUMMY: + case PASS_GOV_BASIC: + /* Don't use Hotplug interface */ return NULL; case PASS_GOV_HOTPLUG_ONLY: case PASS_GOV_STEP: case PASS_GOV_RADIATION: + /* Use Hotplug interface */ hotplug = calloc(1, sizeof(struct pass_hotplug)); if (!hotplug) { _E("cannot allocate the memory of struct pass_hotplug"); @@ -687,11 +690,16 @@ struct pass_governor* pass_get_governor(struct pass_policy *policy, { switch (type) { case PASS_GOV_DUMMY: + return NULL; + case PASS_GOV_BASIC: case PASS_GOV_HOTPLUG_ONLY: + /* Use the Resource controller only */ return &pass_gov_dummy; case PASS_GOV_STEP: + /* Use the Resource controller and Step governor. */ return &pass_gov_step; case PASS_GOV_RADIATION: + /* Use the Resource controller and Radiation governor. */ return &pass_gov_radiation; default: _E("Unknown governor type"); diff --git a/src/pass/pass.h b/src/pass/pass.h index 000a39c..6271799 100644 --- a/src/pass/pass.h +++ b/src/pass/pass.h @@ -50,12 +50,24 @@ enum pass_state { /* * PASS Governor type + * + * --------------------------------------------------------------------------- + * | Governor name | HAL | Resource | Hotplug | Runtime | + * | | interface | controller | interface | governor | + * --------------------------------------------------------------------------- + * | GOV_DUMMY | Used | Un-used | Un-used | Un-used | + * | GOV_BASIC | Used | Used | Un-used | Un-used | + * | GOV_HOTPLUG_ONLY | Used | Used | Used | Un-used | + * | GOV_RADIATION | Used | Used | Used | Used (Radiation)| + * | GOV_STEP | Used | Used | Used | Used (Step) | + * --------------------------------------------------------------------------- */ enum pass_gov_type { PASS_GOV_DUMMY = -1, - PASS_GOV_HOTPLUG_ONLY = 0, - PASS_GOV_STEP = 1, - PASS_GOV_RADIATION = 2, + PASS_GOV_BASIC = 0, + PASS_GOV_HOTPLUG_ONLY = 1, + PASS_GOV_STEP = 2, + PASS_GOV_RADIATION = 3, PASS_GOV_END, }; -- 2.7.4 From 9b6368201c08883a94687e3492e1be2389d3bfc7 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 14:19:07 +0900 Subject: [PATCH 12/16] pass: parser: Add gov_timeout to Level section in resource conf file This patch adds a gov_timeout entry to the Level section in the resource configuration file. Comparing with pass_gov_timeout in the Pass section, this entry indicates the timer interval for a specific level. If this entry does not appear in the Level section, the value of pass_gov_timeout will be assigned to the governor timer interval for that level by default. Change-Id: I57ed24f8666711c11c3ceeeeaab9bfcc13c550a1 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-parser.c | 8 ++++++++ src/pass/pass.c | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index e10ed52..6e58f22 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -157,6 +157,14 @@ static int pass_parse_level(struct parse_result *result, policy->pass_table[level].right_cond[0].nr_running = atoi(result->value); else if (MATCH(result->name, "num_right_cond_busy_cpu")) policy->pass_table[level].right_cond[0].busy_cpu = atoi(result->value); + else if (MATCH(result->name, "gov_timeout")) { + double gov_timeout = atof(result->value); + + if (gov_timeout < PASS_MIN_GOV_TIMEOUT) + gov_timeout = PASS_MIN_GOV_TIMEOUT; + + policy->pass_table[level].gov_timeout = gov_timeout; + } return 0; } diff --git a/src/pass/pass.c b/src/pass/pass.c index 5a7d6e7..0fe797d 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -158,6 +158,8 @@ static int pass_resource_init(struct pass_policy *policy) max_freq = policy->pass_table[i].limit_max_freq; if (max_cpu < policy->pass_table[i].limit_min_cpu) max_cpu = policy->pass_table[i].limit_min_cpu; + if (policy->pass_table[i].gov_timeout == 0) + policy->pass_table[i].gov_timeout = policy->gov_timeout; } policy->freq.max_freq = max_freq; -- 2.7.4 From 61a676d38ea6d126ef6abb7775e5a08e148682ed Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 14:38:59 +0900 Subject: [PATCH 13/16] pass: Remove policy-specific data from struct pass_governor This patch removes policy-specific data from struct pass_governor. The policy-specific data should be included in struct pass_policy only. Change-Id: Id9bd77e9dbd8b0668170696b40a0d40c9c166e42 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-gov.c | 35 +++++++++++++++++------------------ src/pass/pass.c | 1 - src/pass/pass.h | 3 +-- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index 5e94a9b..27d72d6 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -417,18 +417,18 @@ static Eina_Bool pass_governor_core_timer(void *data) * Change the period of govenor timer according to PASS level */ if (policy->pass_table[level].gov_timeout >= PASS_MIN_GOV_TIMEOUT && - (policy->governor->gov_timeout + (policy->gov_timeout != policy->pass_table[level].gov_timeout)) { _I("Change the period of governor timer from %fs to %fs\n", - policy->governor->gov_timeout, + policy->gov_timeout, policy->pass_table[level].gov_timeout); - policy->governor->gov_timeout = + policy->gov_timeout = policy->pass_table[level].gov_timeout; - ecore_timer_interval_set(policy->governor->gov_timer_id, - policy->governor->gov_timeout); - ecore_timer_reset(policy->governor->gov_timer_id); + ecore_timer_interval_set(policy->gov_timer_id, + policy->gov_timeout); + ecore_timer_reset(policy->gov_timer_id); } return ECORE_CALLBACK_RENEW; @@ -455,18 +455,18 @@ static void __pass_governor_start(struct pass_policy *policy) } /* Create the core timer of PASS governor */ - if (policy->governor->gov_timeout > 0) { - policy->governor->gov_timer_id = ecore_timer_add( - policy->governor->gov_timeout, + if (policy->gov_timeout > 0) { + policy->gov_timer_id = ecore_timer_add( + policy->gov_timeout, (Ecore_Task_Cb)pass_governor_core_timer, (void *)policy); - if (!policy->governor->gov_timer_id) { + if (!policy->gov_timer_id) { _E("cannot add core timer for governor"); pass_governor_update(policy, PASS_GOV_STOP); return; } } else { - policy->governor->gov_timer_id = NULL; + policy->gov_timer_id = NULL; } /* @@ -509,9 +509,9 @@ static void __pass_governor_stop(struct pass_policy *policy) pass_hotplug_stop(policy); - if (policy->governor->gov_timer_id) { - ecore_timer_del(policy->governor->gov_timer_id); - policy->governor->gov_timer_id = NULL; + if (policy->gov_timer_id) { + ecore_timer_del(policy->gov_timer_id); + policy->gov_timer_id = NULL; } /* Set PASS state as PASS_GOV_STOP */ @@ -525,9 +525,8 @@ static int __pass_governor_init(struct pass_policy *policy) struct pass_resource *res = to_pass_resource(policy); struct pass_conf_data *cdata = &res->cdata; - if(policy->governor->gov_timeout < 0) { - _E("invalid timeout value [%d]!", - policy->governor->gov_timeout); + if(policy->gov_timeout < 0) { + _E("invalid timeout value [%d]!", policy->gov_timeout); pass_governor_update(policy, PASS_GOV_STOP); return -EINVAL; } @@ -583,7 +582,7 @@ static int __pass_governor_exit(struct pass_policy *policy) policy->pass_table = NULL; policy->num_pass_cpu_stats = 0; - policy->governor->gov_timeout = 0; + policy->gov_timeout = 0; policy->governor = NULL; diff --git a/src/pass/pass.c b/src/pass/pass.c index 0fe797d..c7399dc 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -182,7 +182,6 @@ static int pass_resource_init(struct pass_policy *policy) _E("cannot get the instance of PASS governor"); return -1; } - policy->governor->gov_timeout = policy->gov_timeout; /* Get the instance of PASS hotplug */ policy->hotplug = pass_get_hotplug(policy, policy->gov_type); diff --git a/src/pass/pass.h b/src/pass/pass.h index 6271799..8eaa82e 100644 --- a/src/pass/pass.h +++ b/src/pass/pass.h @@ -95,8 +95,6 @@ struct pass_governor { int (*exit)(struct pass_policy *); int (*update)(struct pass_policy *, enum pass_gov_state state); int (*governor)(struct pass_policy *); - Ecore_Timer *gov_timer_id; - double gov_timeout; }; /****************************************************** @@ -247,6 +245,7 @@ struct pass_policy { struct pass_governor *governor; double gov_timeout; + Ecore_Timer *gov_timer_id; struct pass_hotplug *hotplug; }; -- 2.7.4 From 10b2ca13dcd1b63de08b6dec17324f176fbf3fbe Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 17:15:11 +0900 Subject: [PATCH 14/16] pass: gov: Modify timer interval setting mechanism This patch modifies the timer setting mechanism for the governor to select either the level-specific timer interval or the default one. The default timer interval is used when the gov_timeout does not appear in the Level section of the resource configuration file. Change-Id: Id1b9d31a3249846ee59b34d00cb41f126440f5bc Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-gov.c | 26 +++++++++++++------------- src/pass/pass-parser.c | 1 + 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index 27d72d6..d7d359c 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -251,7 +251,7 @@ static int pass_governor_change_level(struct pass_policy *policy, int new_level) limit_max_freq = table[new_level].limit_max_freq; limit_min_freq = table[new_level].limit_min_freq; - policy->prev_level = policy->curr_level; + policy->prev_level = curr_level; policy->curr_level = new_level; /* Turn on/off CPUs according the maximum number of online CPU */ @@ -372,6 +372,7 @@ static Eina_Bool pass_governor_core_timer(void *data) { struct pass_policy *policy = (struct pass_policy *)data; static int count = 0; + double curr_gov_timeout, next_gov_timeout; int level; int ret; @@ -402,6 +403,9 @@ static Eina_Bool pass_governor_core_timer(void *data) /* Calculate the number of busy cpu */ pass_calculate_busy_cpu(policy); + /* Store current governor timeout */ + curr_gov_timeout = policy->pass_table[policy->curr_level].gov_timeout; + /* Determine the amount of proper resource */ if (policy->governor->governor) { level = policy->governor->governor(policy); @@ -413,21 +417,16 @@ static Eina_Bool pass_governor_core_timer(void *data) return ECORE_CALLBACK_CANCEL; } - /* - * Change the period of govenor timer according to PASS level - */ - if (policy->pass_table[level].gov_timeout >= PASS_MIN_GOV_TIMEOUT && - (policy->gov_timeout - != policy->pass_table[level].gov_timeout)) { + /* Store next governor timeout */ + next_gov_timeout = policy->pass_table[policy->curr_level].gov_timeout; + /* Change the period of govenor timer according to PASS level */ + if (curr_gov_timeout != next_gov_timeout) { _I("Change the period of governor timer from %fs to %fs\n", - policy->gov_timeout, - policy->pass_table[level].gov_timeout); - - policy->gov_timeout = - policy->pass_table[level].gov_timeout; + curr_gov_timeout, + next_gov_timeout); ecore_timer_interval_set(policy->gov_timer_id, - policy->gov_timeout); + next_gov_timeout); ecore_timer_reset(policy->gov_timer_id); } @@ -476,6 +475,7 @@ static void __pass_governor_start(struct pass_policy *policy) policy->curr_level = -1; if (policy->init_level > policy->max_level) policy->init_level = policy->max_level; + pass_governor_change_level(policy, policy->init_level); /* Set PASS state as PASS_GOV_START */ diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index 6e58f22..e953c88 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -308,6 +308,7 @@ int pass_get_table(struct pass_policy *policy, char *pass_conf_path) policy->up_threshold = 0; policy->down_threshold = 0; policy->init_level = 0; + policy->prev_level = policy->init_level; policy->level_up_threshold = 0; policy->gov_timeout = 0; policy->last_time = 0; -- 2.7.4 From 5ef9c9555b3dafccfc493d335230393952ecaf7b Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 15:54:07 +0900 Subject: [PATCH 15/16] pass: pmqos: Prevent from accessing variables before initialization This patch modifies the callback function of DEVICE_NOTIFIER_PMQOS notifier to prevent from accessing variables before their initialization. Change-Id: Idc8a760ec8a6a53a1cfbc34f17626ea6c186ed57 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-pmqos.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/pass/pass-pmqos.c b/src/pass/pass-pmqos.c index 20be7e4..3929dff 100644 --- a/src/pass/pass-pmqos.c +++ b/src/pass/pass-pmqos.c @@ -95,9 +95,9 @@ static int find_scenario_index(struct pass_scenario_policy *scenario, #define MAX(a,b) (a > b ? a : b) int pass_notifier_pmqos_func(struct pass_policy *policy, void *data) { - struct pass_resource *pass_res = to_pass_resource(policy); - struct pass_conf_data *cdata = &pass_res->cdata; - struct pass_scenario_policy *scenario = &policy->scenario; + struct pass_resource *pass_res; + struct pass_conf_data *cdata; + struct pass_scenario_policy *scenario; struct pass_scenario *scn = NULL; enum pass_state locked = PASS_UNUSED; char name[PASS_NAME_LEN] = {""}; @@ -109,6 +109,10 @@ int pass_notifier_pmqos_func(struct pass_policy *policy, void *data) if (!is_pmqos_enabled(policy)) return 0; + pass_res = to_pass_resource(policy); + cdata = &pass_res->cdata; + scenario = &policy->scenario; + /* * Parse scenario name(data) whether to include 'Lock' or 'Unlock' * string and divide correct scenario name. -- 2.7.4 From aae2ba1d4cf3205628c9db641d26a68c4c6be3bc Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 24 Feb 2017 16:36:41 +0900 Subject: [PATCH 16/16] pass: Add init_done and exit_done callbacks This patch adds init_done and exit_done callbacks to the pass module. These callbacks are in charge of actual initialization and finalization of the daemon, instead of the init and exit functions. Callback functions for dbus messages such as start and stop are also modified to use the init_done and exit_done functions now. Especially, the init_done is registered as a booting-done callback at the probe time. Change-Id: I2f1a929b8b0b6b64b877c3f627d30af42bb26cfb Signed-off-by: Wook Song --- src/pass/pass-gov.c | 29 +-------- src/pass/pass.c | 165 +++++++++++++++++++++++++++++++++------------------- 2 files changed, 108 insertions(+), 86 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index d7d359c..0decd8b 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -50,22 +50,6 @@ static bool is_enabled(struct pass_policy *policy) } /* - * pass_notifier_booting_done - Callback func of DEVICE_NOTIFIER_BOOTING_DONE - * @data: NULL, this parameter isn't used - * @user_data: the instance of struct pass_policy - */ -static int pass_notifier_booting_done(void *data, void *user_data) -{ - struct pass_policy *policy = user_data; - - /* Start PASS governor if 'pass_support' in pass.conf is true */ - if (policy->state == PASS_ON) - pass_governor_update(policy, PASS_GOV_START); - - return 0; -} - -/* * pass_notifier_pmqos - Callback func of DEVICE_NOTIFIER_PMQOS * @data: the scenario name * @user_data: the instance of struct pass_policy @@ -89,10 +73,6 @@ static int pass_notifier_init(struct pass_policy *policy) /* Register DEVICE_NOTIFIER_PMQOS */ register_notifier(DEVICE_NOTIFIER_PMQOS, pass_notifier_pmqos, policy); - /* Register DEVICE_NOTIFIER_BOOTING_DONE */ - register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, - pass_notifier_booting_done, policy); - return 0; } @@ -105,10 +85,6 @@ static int pass_notifier_exit(struct pass_policy *policy) /* Un-register DEVICE_NOTIFIER_PMQOS */ unregister_notifier(DEVICE_NOTIFIER_PMQOS, pass_notifier_pmqos); - /* Un-Register DEVICE_NOTIFIER_BOOTING_DONE */ - unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, - pass_notifier_booting_done); - return 0; } @@ -533,12 +509,13 @@ static int __pass_governor_init(struct pass_policy *policy) /* Set default PASS state */ policy->gov_state = PASS_GOV_STOP; - - /* Initialize notifier */ pass_notifier_init(policy); _I("Initialize governor for '%s' resource", cdata->res_name); + if (policy->state == PASS_ON) + pass_governor_update(policy, PASS_GOV_START); + return 0; } diff --git a/src/pass/pass.c b/src/pass/pass.c index c7399dc..ce85a02 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -27,10 +27,12 @@ #include "pass-hal.h" #include "pass-gov.h" +#include "core/device-notifier.h" #include "core/devices.h" #include "core/common.h" #include "core/edbus-handler.h" +#define PASS_CONF_PATH "/etc/pass/pass.conf" #define PASS_DEFAULT_MIN_LEVEL 0 #define PASS_DEFAULT_CPU_THRESHOLD 20 #define PASS_DEFAULT_LEVEL_UP_THRESHOLD 30 @@ -39,50 +41,47 @@ static struct pass g_pass; /****************************************************** + * PASS interfaces: Probe/Init/Exit * + * (Declarations) * + ******************************************************/ +static int pass_probe(void *data); +static void pass_init(void *data); +static int pass_init_done(void *data, void *user_data); +static void pass_exit(void *data); +static int pass_exit_done(void); + +/****************************************************** * PASS D-Bus interface * ******************************************************/ static DBusMessage* e_dbus_start_cb(E_DBus_Object *obj, DBusMessage* msg) { DBusMessage *ret_dbus = NULL; - int i, ret; - - for (i = 0; i < g_pass.num_resources; i++) { - struct pass_resource *pass_res = &g_pass.res[i]; - struct pass_policy *policy = &pass_res->policy; + int ret; - ret = pass_governor_update(policy, PASS_GOV_START); - if (ret < 0) { - _E("cannot start the governor with dbus"); - return ret_dbus; - } - ret_dbus = dbus_message_new_method_return(msg); - if (!ret_dbus) - return ret_dbus; + ret = pass_init_done(NULL, NULL); + if (ret < 0) { + _E("failed to initialize the daemon in dbus callback \ + for a start message\n"); + return ret_dbus; } + ret_dbus = dbus_message_new_method_return(msg); return ret_dbus; } static DBusMessage* e_dbus_stop_cb(E_DBus_Object *obj, DBusMessage* msg) { DBusMessage *ret_dbus = NULL; - int i, ret; - - for (i = 0; i < g_pass.num_resources; i++) { - struct pass_resource *pass_res = &g_pass.res[i]; - struct pass_policy *policy = &pass_res->policy; - - ret = pass_governor_update(policy, PASS_GOV_STOP); - if (ret < 0) { - _E("cannot stop the governor"); - return ret_dbus; - } + int ret; - ret_dbus = dbus_message_new_method_return(msg); - if (!ret_dbus) - return ret_dbus; + ret = pass_exit_done(); + if (ret < 0) { + _E("failed to exit the daemon in dbus callback \ + for a stop message\n"); + return ret_dbus; } + ret_dbus = dbus_message_new_method_return(msg); return ret_dbus; } @@ -220,16 +219,24 @@ static int pass_resource_exit(struct pass_policy *policy) return 0; } -/* - * pass_init - Initialize PASS(Power Aware System Service) - * - * @data: the instance of structre pass_policy - */ - -static void pass_init(void *data) +static int pass_init_done(void *data, void *user_data) { int i, ret; + /* Parse configuration file (/etc/pass/pass.conf) */ + ret = pass_parse_resource(&g_pass, PASS_CONF_PATH); + if (ret < 0) { + _E("cannot parse %s\n", PASS_CONF_PATH); + return ret; + } + + /* Initialize pass resources data based on parsed configuration */ + ret = pass_get_resource(&g_pass); + if (ret < 0) { + _E("cannot get the pass resource\n"); + return ret; + } + for (i = 0; i < g_pass.num_resources; i++) { struct pass_resource *pass_res = &g_pass.res[i]; struct pass_policy *policy = &pass_res->policy; @@ -245,17 +252,6 @@ static void pass_init(void *data) _I("Resource%d first cpu : %d\n\n", i, cdata->cpu); } - /* - * Initialzie D-Bus interface of PASS. User can be able to - * turn on/off PASS through D-Bus interface. - */ - ret = register_edbus_method(PASS_PATH_CORE, edbus_methods, - ARRAY_SIZE(edbus_methods)); - if (ret < 0) { - _I("cannot initialize PASS D-Bus (%d)", ret); - return; - } - for (i = 0; i < g_pass.num_resources; i++) { struct pass_resource *pass_res = &g_pass.res[i]; struct pass_policy *policy = &pass_res->policy; @@ -263,51 +259,100 @@ static void pass_init(void *data) ret = pass_resource_init(policy); if (ret < 0) { _E("cannot initialize the pass resource\n"); - return; + pass_exit(NULL); + return -EINVAL; } } + + return 0; } -/* - * pass_exit - Exit PASS - * - * @data: the instance of structre pass_policy - */ -static void pass_exit(void *data) +static int pass_exit_done(void) { int i, ret; + ret = 0; for (i = 0; i < g_pass.num_resources; i++) { struct pass_resource *pass_res = &g_pass.res[i]; struct pass_policy *policy = &pass_res->policy; ret = pass_resource_exit(policy); if (ret < 0) { - _E("cannot exit the pass resource"); + _E("failed to clean up the pass resource #%d\n", i); + break; } } + + return ret; } -int pass_probe(void *data) +/* + * pass_init - Initialize PASS(Power Aware System Service) + * + * @data: When the data is passed from pass_main(), it is NULL. + * On the other hand, it will contain the result of this function + */ +static void pass_init(void *data) +{ + /* This is the only case of daemon creation: DO NOTHING */ +} + +/* + * pass_exit - Exit PASS + * + * @data: When the data is passed from pass_main(), it is NULL. + * On the other hand, it will contain the result of this function + */ +static void pass_exit(void *data) { int ret; + unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, pass_init_done); + + ret = pass_exit_done(); + if (ret < 0) { + _E("cannot exit PASS daemon\n"); + return; + } + + _I("exit PASS daemon without any errors\n"); +} + +/* + * pass_probe - Probe PASS + * + * @data: the data passed from pass_main(), currently NULL + */ +static int pass_probe(void *data) +{ + int ret = 0; + /* - * Initialize pass resources data by parsing pass.conf + * Register methods to D-Bus interface of PASS. By using it, + * user can turn on/off PASS */ - ret = pass_parse_resource(&g_pass, "/etc/pass/pass.conf"); + ret = register_edbus_method(PASS_PATH_CORE, edbus_methods, + ARRAY_SIZE(edbus_methods)); if (ret < 0) { - _E("cannot parse /etc/pass/pass.conf\n"); + _E("cannot register dbus methods required \ + to control the daemon (%d)\n", ret); return ret; } - ret = pass_get_resource(&g_pass); + /* + * Register a notifier for the booting-done event. The actual + * initialization of the daemon is performed by this notifier after + * booting is completely done. + */ + ret = register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, + pass_init_done, NULL); if (ret < 0) { - _E("cannot get the pass resource\n"); + _E("cannot register a callback function \ + for the booting-done event (%d)\n", ret); return ret; } - return 0; + return ret; } static const struct device_ops pass_device_ops = { -- 2.7.4