From 441c327e698ece05465bdc884af59f9df9805419 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Thu, 11 Aug 2022 16:29:49 +0900 Subject: [PATCH 01/16] tests: resource-monitor-tests: Add missing array data type if DATA_TYPE_ARRAY Change-Id: I3606a29597ba9cd16c47defba5b99055b25b5d22 Signed-off-by: Chanwoo Choi --- tests/integration-test/resource-monitor-tests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration-test/resource-monitor-tests.cpp b/tests/integration-test/resource-monitor-tests.cpp index 9435295..2901648 100644 --- a/tests/integration-test/resource-monitor-tests.cpp +++ b/tests/integration-test/resource-monitor-tests.cpp @@ -320,9 +320,9 @@ INSTANTIATE_TEST_CASE_P (ResourceMonitorTest, PhysicalResourceMonitorTest, { .attr_id = SYSTEM_ATTR_CPU_UTIL, .attr_type = DATA_TYPE_DOUBLE, }, { .attr_id = SYSTEM_ATTR_CPU_USER_UTIL, .attr_type = DATA_TYPE_DOUBLE, }, { .attr_id = SYSTEM_ATTR_CPU_SYS_UTIL, .attr_type = DATA_TYPE_DOUBLE, }, - { .attr_id = SYSTEM_ATTR_PER_CPU_UTIL, .attr_type = DATA_TYPE_ARRAY, }, - { .attr_id = SYSTEM_ATTR_PER_CPU_USER_UTIL, .attr_type = DATA_TYPE_ARRAY, }, - { .attr_id = SYSTEM_ATTR_PER_CPU_SYS_UTIL, .attr_type = DATA_TYPE_ARRAY, }, + { .attr_id = SYSTEM_ATTR_PER_CPU_UTIL, .attr_type = DATA_TYPE_ARRAY, .array_type = DATA_TYPE_DOUBLE, }, + { .attr_id = SYSTEM_ATTR_PER_CPU_USER_UTIL, .attr_type = DATA_TYPE_ARRAY, .array_type = DATA_TYPE_DOUBLE, }, + { .attr_id = SYSTEM_ATTR_PER_CPU_SYS_UTIL, .attr_type = DATA_TYPE_ARRAY, .array_type = DATA_TYPE_DOUBLE, }, { .attr_id = SYSTEM_ATTR_POSSIBLE_CPU, .attr_type = DATA_TYPE_INT, }, { .attr_id = SYSTEM_ATTR_ONLINE_CPU, .attr_type = DATA_TYPE_INT, }, } -- 2.7.4 From 62cdb7e9f1db48d40361c6f0640ca1ea2df235c6 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Fri, 12 Aug 2022 11:48:03 +0900 Subject: [PATCH 02/16] PASS v2.0.0 Changes from v1.2.0 1. Add new resource-monitor framework with daemon and various resource drivers 2. Provide libpass-resource-monitor library for resource monitoring 3. Provide resource-monitor tool and resource-monitor-tests Change-Id: I5d6ccbeb08d29a15181e2c74112b509f99769c58 Signed-off-by: Chanwoo Choi --- packaging/pass.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/pass.spec b/packaging/pass.spec index e50b60f..bde3967 100644 --- a/packaging/pass.spec +++ b/packaging/pass.spec @@ -7,7 +7,7 @@ Name: %{daemon_name} Summary: Power Aware System Service -Version: 1.2.0 +Version: 2.0.0 Release: 1 Group: System/Kernel License: Apache-2.0 -- 2.7.4 From a7b9c03b96f647e4d3c1939fba9ac0ed78dd93fb Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Wed, 17 Aug 2022 15:31:53 +0900 Subject: [PATCH 03/16] util: kernel: Add check whether taskstats is supported Since taskstats has various version by kernel, handling it should be taken by considering versions. However, some task accounting is not supported by older kernel, we should check whether it is supported or not. This adds check procedure into constructor. Change-Id: Ib18f01b1a3060baaba3212fdc728cd7a7ff77849 Signed-off-by: Dongwoo Lee --- include/util/kernel.h | 1 + src/util/kernel.c | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/include/util/kernel.h b/include/util/kernel.h index 12ae6e9..a1d13d9 100644 --- a/include/util/kernel.h +++ b/include/util/kernel.h @@ -69,4 +69,5 @@ int kernel_get_thread_group_taskstats(struct taskstats *stats, pid_t tgid, bool int kernel_get_thread_group_map_info(struct proc_map_info *map_info, pid_t tgid, bool include_gpu_mem); bool kernel_check_gpu_support(void); +bool kernel_check_taskstat_support(void); #endif diff --git a/src/util/kernel.c b/src/util/kernel.c index 0baaf4a..40fd869 100644 --- a/src/util/kernel.c +++ b/src/util/kernel.c @@ -31,6 +31,7 @@ static int have_smaps_rollup; static bool need_tgid_taskstat_add_time; static struct gpu_mem_node *gpu_mem_node; +static bool taskstat_support; static int __get_cpu_num(char *path) { @@ -501,14 +502,25 @@ bool kernel_check_gpu_support(void) return !!gpu_mem_node; } +bool kernel_check_taskstat_support(void) +{ + return taskstat_support; +} + static void __CONSTRUCTOR__ kernel_init(void) { struct taskstats self; + int ret; have_smaps_rollup = !access("/proc/self/smaps_rollup", R_OK); gpu_mem_node = get_system_gpu_mem_node(); - kernel_get_thread_group_taskstats(&self, getpid(), false); + ret = kernel_get_thread_group_taskstats(&self, getpid(), false); + if (ret < 0) { + _I("taskstat is not supported.\n"); + return; + } + taskstat_support = true; if (self.ac_etime == 0) need_tgid_taskstat_add_time = true; } -- 2.7.4 From 70b1d1682bcb6bb58d98d9479431d1dc7bfc0bc2 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Wed, 17 Aug 2022 15:33:18 +0900 Subject: [PATCH 04/16] resource: process-group: Handle the case that target cannot support taskstats all attributes in PROCESS_GROUP resource require taskstats, all attributes are mark to unsupported when the target does not support taskstats. Change-Id: I21a36949e34e7cf3fb050694b1cbddd31e954247 Signed-off-by: Dongwoo Lee --- src/resource/resource-process-group.c | 37 +++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/resource/resource-process-group.c b/src/resource/resource-process-group.c index 0ee9d90..f0a9a4d 100644 --- a/src/resource/resource-process-group.c +++ b/src/resource/resource-process-group.c @@ -51,6 +51,7 @@ struct process_group_context { static u_int64_t total_memory; static long jiffy; +static int taskstat_support = -1; static int process_group_get_pid_list(struct resource *res, const struct resource_attribute *attr, @@ -239,10 +240,16 @@ static int process_group_get_disk_attrs(struct resource *res, return 0; } +static bool process_group_check_taskstat_support(struct resource *resource, + const struct resource_attribute *attr) +{ + return !!taskstat_support; +} + static bool process_group_check_gpu_support(struct resource *resource, const struct resource_attribute *attr) { - return kernel_check_gpu_support(); + return !!taskstat_support && kernel_check_gpu_support(); } static const struct resource_attribute process_group_attrs[] = { @@ -252,7 +259,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_ARRAY, .ops = { .get = process_group_get_pid_list, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_NAME_LIST", @@ -260,7 +267,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_ARRAY, .ops = { .get = process_group_get_name_list, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_CPU_UTIL", @@ -268,7 +275,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_group_get_cpu_util, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_DISK_READ_PER_SEC", @@ -276,7 +283,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_group_get_disk_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_DISK_WRITE_PER_SEC", @@ -284,7 +291,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_group_get_disk_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_MEM_VIRT", @@ -292,7 +299,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_group_get_mem, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_MEM_RSS", @@ -300,7 +307,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_group_get_mem, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_MEM_PSS", @@ -308,7 +315,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_group_get_mem, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_MEM_SWAP", @@ -316,7 +323,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_group_get_mem, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_MEM_SWAP_PSS", @@ -324,7 +331,7 @@ static const struct resource_attribute process_group_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_group_get_mem, - .is_supported = resource_attr_supported_always, + .is_supported = process_group_check_taskstat_support, }, }, { .name = "PROCESS_GROUP_ATTR_MEM_GPU", @@ -355,7 +362,7 @@ static int process_group_setup_root_pid(struct resource *res, target_pid = (int)(intptr_t)data; - if (target_pid < 0) { + if (target_pid < 0 || !taskstat_support) { ctx->pid = -1; return 0; } @@ -519,6 +526,9 @@ static int process_group_prepare_update(struct resource *res) if (!ctx) return -EINVAL; + if (!taskstat_support) + return 0; + task_dir = opendir(PROC_DIR_PATH); if (!task_dir) return -ESRCH; @@ -644,6 +654,9 @@ static int process_group_init(struct resource *res) struct process_group_context *ctx; int ret; + if (taskstat_support < 0) + taskstat_support = (int)kernel_check_taskstat_support(); + if (jiffy == 0) { /* get system USER_HZ at once */ jiffy = sysconf(_SC_CLK_TCK); -- 2.7.4 From 86164a3a81ee70908d38b8120448030ac316df66 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Wed, 17 Aug 2022 17:42:43 +0900 Subject: [PATCH 05/16] resource: process: Handle the case that target cannot support taskstats Since PROCESS resource has some attributes which require taskstats, those attributes are mark to unsupported or provided by other methods when the target does not support taskstats. Change-Id: I83a0eab243367f59c518d1b1c422b741f3e611bb Signed-off-by: Dongwoo Lee --- src/resource/resource-process.c | 87 ++++++++++++++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/src/resource/resource-process.c b/src/resource/resource-process.c index 633fc78..192d91e 100644 --- a/src/resource/resource-process.c +++ b/src/resource/resource-process.c @@ -41,6 +41,7 @@ struct process_context { }; static long jiffy; +static int taskstat_support = -1; static int process_get_cpu_util(struct resource *res, const struct resource_attribute *attr, @@ -194,6 +195,21 @@ static int process_get_context_data(struct resource *res, switch (attr->id) { case PROCESS_ATTR_NAME: + if (!taskstat_support && (strlen(ctx->comm) == 0)) { + char *stat_fields[PROCESS_STAT_FIELD_MAX]; + char buffer[BUFF_MAX]; + int ret, len; + + ret = kernel_get_process_stat_fields(ctx->tgid, buffer, + BUFF_MAX, stat_fields); + if (ret < 0) + break; + + len = strlen(stat_fields[PROCESS_STAT_FIELD_COMM]) - 2/* '(', ')' */; + len = (len < TS_COMM_LEN - 1) ? len : TS_COMM_LEN - 1; + strncpy(ctx->comm, stat_fields[PROCESS_STAT_FIELD_COMM] + 1, len); + } + strncpy((char *)data, ctx->comm, TS_COMM_LEN); break; case PROCESS_ATTR_PGID: @@ -213,6 +229,19 @@ static int process_get_context_data(struct resource *res, *((int *)data) = ctx->pgid; break; case PROCESS_ATTR_PPID: + if (ctx->ppid < 0) { + char *stat_fields[PROCESS_STAT_FIELD_MAX]; + char buffer[BUFF_MAX]; + int ret; + + ret = kernel_get_process_stat_fields(ctx->tgid, buffer, + BUFF_MAX, stat_fields); + if (ret < 0) + break; + + ctx->ppid = atoi(stat_fields[PROCESS_STAT_FIELD_PPID]); + } + *((int *)data) = ctx->ppid; break; } @@ -220,6 +249,12 @@ static int process_get_context_data(struct resource *res, return 0; } +static bool process_check_taskstat_support(struct resource *resource, + const struct resource_attribute *attr) +{ + return !!taskstat_support; +} + static bool process_check_gpu_support(struct resource *resource, const struct resource_attribute *attr) { @@ -233,7 +268,7 @@ static const struct resource_attribute process_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_get_cpu_util, - .is_supported = resource_attr_supported_always, + .is_supported = process_check_taskstat_support, }, }, { .name = "PROCESS_ATTR_MEM_VIRT", @@ -241,7 +276,7 @@ static const struct resource_attribute process_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_get_mem_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_check_taskstat_support, }, }, { .name = "PROCESS_ATTR_MEM_RSS", @@ -249,7 +284,7 @@ static const struct resource_attribute process_attrs[] = { .type = DATA_TYPE_UINT64, .ops = { .get = process_get_mem_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_check_taskstat_support, }, }, { .name = "PROCESS_ATTR_MEM_RSS_PERCENT", @@ -257,7 +292,7 @@ static const struct resource_attribute process_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_get_mem_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_check_taskstat_support, }, }, { .name = "PROCESS_ATTR_DISK_READ_PER_SEC", @@ -265,7 +300,7 @@ static const struct resource_attribute process_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_get_disk_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_check_taskstat_support, }, }, { .name = "PROCESS_ATTR_DISK_WRITE_PER_SEC", @@ -273,7 +308,7 @@ static const struct resource_attribute process_attrs[] = { .type = DATA_TYPE_DOUBLE, .ops = { .get = process_get_disk_attrs, - .is_supported = resource_attr_supported_always, + .is_supported = process_check_taskstat_support, }, }, { .name = "PROCESS_ATTR_NAME", @@ -379,19 +414,29 @@ static int process_setup_tgid(struct resource *res, ctx->prev_total_time = get_total_cpu_time(); /* update initial status */ - ret = kernel_get_thread_group_taskstats(&ctx->curr, ctx->tgid, true); - if (ret < 0) - return ret; + if (taskstat_support) { + ret = kernel_get_thread_group_taskstats(&ctx->curr, ctx->tgid, true); + if (ret < 0) + return ret; + + memcpy(ctx->comm, ctx->curr.ac_comm, strlen(ctx->curr.ac_comm) + 1); + ctx->ppid = ctx->curr.ac_ppid; + } else { + /* + * if taskstat is not supported, comm, pgid and ppid is retrieved + * from procfs status lazily. + * + * ctx->comm is now "\0" so it is treated as not initialized. + */ + ctx->ppid = -1; + } + + ctx->pgid = -1; ret = kernel_get_thread_group_map_info(&ctx->map_info, ctx->tgid, include_gpu_mem); if (ret < 0) return ret; - memcpy(ctx->comm, ctx->curr.ac_comm, strlen(ctx->curr.ac_comm) + 1); - - ctx->pgid = -1; - ctx->ppid = ctx->curr.ac_ppid; - return 0; } @@ -421,11 +466,12 @@ static int process_prepare_update(struct resource *res) include_gpu_mem = is_resource_attr_interested(res, PROCESS_GROUP_ATTR_MEM_GPU); - memcpy(&ctx->prev, &ctx->curr, sizeof(struct taskstats)); - - ret = kernel_get_thread_group_taskstats(&ctx->curr, ctx->tgid, false); - if (ret < 0) - return ret; + if (taskstat_support) { + memcpy(&ctx->prev, &ctx->curr, sizeof(struct taskstats)); + ret = kernel_get_thread_group_taskstats(&ctx->curr, ctx->tgid, false); + if (ret < 0) + return ret; + } ret = kernel_get_thread_group_map_info(&ctx->map_info, ctx->tgid, include_gpu_mem); if (ret < 0) @@ -453,6 +499,9 @@ static int process_init(struct resource *res) return -EINVAL; } + if (taskstat_support < 0) + taskstat_support = (int)kernel_check_taskstat_support(); + ctx = calloc(1, sizeof(struct process_context)); if (!ctx) return -ENOMEM; -- 2.7.4 From d411572f8b57afe7887119f99a3fe8620715803e Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Wed, 17 Aug 2022 17:45:58 +0900 Subject: [PATCH 06/16] resource: process: Fix to use proper attribute name Change-Id: Icef625156eb2036672f2798f4f0d65d943e3ab6f Signed-off-by: Dongwoo Lee --- src/resource/resource-process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resource/resource-process.c b/src/resource/resource-process.c index 192d91e..e77d765 100644 --- a/src/resource/resource-process.c +++ b/src/resource/resource-process.c @@ -405,7 +405,7 @@ static int process_setup_tgid(struct resource *res, if (!ctx) return -EINVAL; - include_gpu_mem = is_resource_attr_interested(res, PROCESS_GROUP_ATTR_MEM_GPU); + include_gpu_mem = is_resource_attr_interested(res, PROCESS_ATTR_MEM_GPU); total_memory = ctx->total_memory; memset(ctx, 0, sizeof(*ctx)); @@ -464,7 +464,7 @@ static int process_prepare_update(struct resource *res) if (!ctx) return -EINVAL; - include_gpu_mem = is_resource_attr_interested(res, PROCESS_GROUP_ATTR_MEM_GPU); + include_gpu_mem = is_resource_attr_interested(res, PROCESS_ATTR_MEM_GPU); if (taskstat_support) { memcpy(&ctx->prev, &ctx->curr, sizeof(struct taskstats)); -- 2.7.4 From 9378b6a79949f787f437a5abff25d5747ee06e1b Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Thu, 18 Aug 2022 18:39:51 +0900 Subject: [PATCH 07/16] tools: resource-monitor: Keep the alignment of resource attribute name commit f59279ad7f63 ("resource: Change bytes unit attributes to kilo-bytes") changed the attribute name of both PROCESS_GROUP_ATTR_DISK_READ_PER_SEC and PROCESS_GROUP_ATTR_DISK_WRITE_PER_SEC. So that break the alignment when print the resource attribute name by resource-monitor cli tool. Extend the width length for resource attribute name to keep the printing alignement. - Before change: 7: 2| PROCESS_GROUP_ATTR_CPU_UTIL | 0.27 | % | Process-group CPU utilization 7: 3| PROCESS_GROUP_ATTR_DISK_READ_PER_SEC | 0.00 | kB/s | Process-group disk read per second 7: 4| PROCESS_GROUP_ATTR_DISK_WRITE_PER_SEC | 0.00 | kB/s | Process-group disk write per second 7: 5| PROCESS_GROUP_ATTR_MEM_VIRT | 132923188 | kB | Process-group VIRT memory size - After change: 7: 2| PROCESS_GROUP_ATTR_CPU_UTIL | 0.21 | % | Process-group CPU utilization 7: 3| PROCESS_GROUP_ATTR_DISK_READ_PER_SEC | 0.00 | kB/s | Process-group disk read per second 7: 4| PROCESS_GROUP_ATTR_DISK_WRITE_PER_SEC | 0.00 | kB/s | Process-group disk write per second 7: 5| PROCESS_GROUP_ATTR_MEM_VIRT | 135495517 | kB | Process-group VIRT memory size Change-Id: I62a937f1edb9c85979cd6f309b12e94256dbeaed Signed-off-by: Chanwoo Choi --- tools/resource-monitor/resource-monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/resource-monitor/resource-monitor.c b/tools/resource-monitor/resource-monitor.c index f89a60d..c3d9701 100644 --- a/tools/resource-monitor/resource-monitor.c +++ b/tools/resource-monitor/resource-monitor.c @@ -503,7 +503,7 @@ static void resource_monitor(void) pass_resource_monitor_update(id); printf("-------------------------------------------------------------------------------------------------------------------------------\n"); - printf("%2s:%2s| %35s | %40s | %-5s | %s\n", + printf("%2s:%2s| %40s | %40s | %-5s | %s\n", "", "", "Resource Attribute Name", "Resource Attribute Value", "Unit", "Resource Attribute Description"); printf("-------------------------------------------------------------------------------------------------------------------------------\n"); @@ -511,7 +511,7 @@ static void resource_monitor(void) struct resource_data *res = &g_data.res[i]; for (j = 0; j < res->num_attrs; j++) { - printf("%2d:%2d| %35s | ", i, j, res->attrs[j].name); + printf("%2d:%2d| %40s | ", i, j, res->attrs[j].name); get_resource_attr_value(res, j); printf("\n"); } -- 2.7.4 From 476a97b39e0a3f30e9ef52a0b01c87d55d1ed077 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Fri, 19 Aug 2022 15:00:06 +0900 Subject: [PATCH 08/16] lib: resource-monitor: Replace with TIZNE_ERROR_* defintion to handle error Replace with TIZNE_ERROR_* defintion to handle error as following: 0 : TIZEN_ERROR_NONE (success) -EINVAL : TIZEN_ERROR_INVALID_PARAMETER -EIO : TIZEN_ERROR_NO_DATA -ENOMEM : TIZEN_ERROR_OUT_OF_MEMORY Change-Id: I1d14719c21c3a4f6f2224c9f76e00ff762b8c889 Signed-off-by: Chanwoo Choi --- lib/resource-monitor/resource-monitor.c | 96 +++++++++++++++++---------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/lib/resource-monitor/resource-monitor.c b/lib/resource-monitor/resource-monitor.c index 938c9dc..8fa81e9 100644 --- a/lib/resource-monitor/resource-monitor.c +++ b/lib/resource-monitor/resource-monitor.c @@ -34,6 +34,8 @@ #include #include +#include + #include "resource-monitor.h" #define REQUEST_SERVER_IP "127.0.0.1" @@ -120,7 +122,7 @@ static inline int handle_request(struct request_data *data) int ret; if (!data) - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; /* Make buffer with struct request_data according to request */ switch (data->request) { @@ -165,7 +167,7 @@ static inline int handle_request(struct request_data *data) default: _E("[libpass] Unknown request type, client(%d) | request(%d)", data->client_id, data->request); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } /* Send request to resource-moniotor */ @@ -174,7 +176,7 @@ static inline int handle_request(struct request_data *data) data->client_id, request_name[data->request], data->resource_id, data->resource_type, data->ctrl_id, data->ctrl_value); - return -EIO; + return TIZEN_ERROR_NO_DATA; } /* Receive response from resource-moniotor */ @@ -184,7 +186,7 @@ static inline int handle_request(struct request_data *data) data->client_id, request_name[data->request], data->resource_id, data->resource_type, data->ctrl_id, data->ctrl_value); - return -EIO; + return TIZEN_ERROR_NO_DATA; } buffer[buffer_len] = '\0'; @@ -195,37 +197,37 @@ static inline int handle_request(struct request_data *data) case REQUEST_GET_RESOURCE_COUNT: if (sscanf(buffer, "%d$%d$%d", &response_req, &data->value_int32, &response_ret) < 3) - ret = EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_VALUE_UINT: if (sscanf(buffer, "%d$%u$%d", &response_req, &data->value_uint32, &response_ret) < 3) - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_VALUE_INT64: if (sscanf(buffer, "%d$%"PRId64"$%d", &response_req, &data->value_int64, &response_ret) < 3) - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_VALUE_UINT64: if (sscanf(buffer, "%d$%"PRIu64"$%d", &response_req, &data->value_uint64, &response_ret) < 3) - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_VALUE_DOUBLE: if (sscanf(buffer, "%d$%lf$%d", &response_req, &data->value_double, &response_ret) < 3) - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_VALUE_STRING: if (sscanf(buffer, "%d$%[^$]$%d", &response_req, data->value_string, &response_ret) < 3) - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_RESOURCE_TS: if (sscanf(buffer, "%d$%"PRId64"$%"PRId64"$%d", &response_req, &data->ts_start, &data->ts_end, &response_ret) < 4) - ret =-EINVAL; + ret =TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_UPDATE_RESOURCE_ALL: case REQUEST_UPDATE_RESOURCE: @@ -237,14 +239,14 @@ static inline int handle_request(struct request_data *data) case REQUEST_UNSET_RESOURCE_ATTR: case REQUEST_IS_RESOURCE_ATTR_SET: if (sscanf(buffer, "%d$%d", &response_req, &response_ret) < 2) - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; break; default: _E("[libpass] Unknown request type, client(%d) | request(%35s) | res,id(%d)type(%d) | ctrl,id(%"PRId64")val(%d) ", data->client_id, request_name[data->request], data->resource_id, data->resource_type, data->ctrl_id, data->ctrl_value); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } if (ret < 0) { @@ -269,7 +271,7 @@ static inline int handle_request(struct request_data *data) data->client_id, request_name[data->request], data->resource_id, data->resource_type, data->ctrl_id, data->ctrl_value); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } return response_ret; @@ -280,12 +282,12 @@ int pass_resource_monitor_init(void) { struct pass_resource_monitor_client *client; struct sockaddr_in server_addr; - int ret = -EIO; + int ret = TIZEN_ERROR_NO_DATA; client = malloc(sizeof(struct pass_resource_monitor_client)); if (!client) { _E("[libpass] failed to allocate memory"); - return -ENOMEM; + return TIZEN_ERROR_OUT_OF_MEMORY; } /* open socket to server */ @@ -329,14 +331,14 @@ int pass_resource_monitor_exit(int id) client = find_client_by_id(id); if (!client) { _E("[libpass] cannot find client-%d", id); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } close(client->id); remove_client_from_list(client); free(client); - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -354,7 +356,7 @@ int pass_resource_monitor_get_resource_count(int id, int resource_type, int *res return ret; *resource_count = request.value_int32; - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -483,7 +485,7 @@ static int pass_resource_monitor_get_json(int id, char *json_string, int request buffer = malloc(HUGE_BUFF_MAX + 1); if (!buffer) - return -ENOMEM; + return TIZEN_ERROR_OUT_OF_MEMORY; va_start(args, request_type); switch (request_type) { @@ -504,14 +506,14 @@ static int pass_resource_monitor_get_json(int id, char *json_string, int request default: va_end(args); free(buffer); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } va_end(args); if (send(id, buffer, buffer_len, 0) < 0) { _E("[libpass] error occurred while sending buffer"); free(buffer); - return -EIO; + return TIZEN_ERROR_NO_DATA; } /* wait for response */ @@ -519,19 +521,19 @@ static int pass_resource_monitor_get_json(int id, char *json_string, int request if (buffer_len <= 0) { _E("[libpass] socket disconnected"); free(buffer); - return -EIO; + return TIZEN_ERROR_NO_DATA; } buffer[buffer_len] = '\0'; if (sscanf(buffer, "%d$%[^$]$%d", &response_req, json_string, &ret) < 3) { free(buffer); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } if (response_req != request_type) { _E("[libpass] wrong response"); free(buffer); - return -EINVAL; + return TIZEN_ERROR_INVALID_PARAMETER; } free(buffer); @@ -573,7 +575,7 @@ int pass_resource_monitor_get_value_int(int id, int resource_id, u_int64_t attr_ return ret; *value = request.value_int32; - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -592,7 +594,7 @@ int pass_resource_monitor_get_value_int64(int id, int resource_id, u_int64_t att return ret; *value = request.value_int64; - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -611,7 +613,7 @@ int pass_resource_monitor_get_value_uint(int id, int resource_id, u_int64_t attr return ret; *value = request.value_uint32; - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -630,7 +632,7 @@ int pass_resource_monitor_get_value_uint64(int id, int resource_id, u_int64_t at return ret; *value = request.value_uint64; - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -649,7 +651,7 @@ int pass_resource_monitor_get_value_double(int id, int resource_id, u_int64_t at return ret; *value = request.value_double; - return 0; + return TIZEN_ERROR_NONE; } EXPORT @@ -668,7 +670,7 @@ int pass_resource_monitor_get_value_string(int id, int resource_id, u_int64_t at return ret; *value = g_strdup(request.value_string); - return 0; + return TIZEN_ERROR_NONE; } static int @@ -682,19 +684,19 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ buffer = malloc(HUGE_BUFF_MAX + 1); if (!buffer) - return -ENOMEM; + return TIZEN_ERROR_OUT_OF_MEMORY; array_str = malloc(HUGE_BUFF_MAX); if (!array_str) { free(buffer); - return -ENOMEM; + return TIZEN_ERROR_OUT_OF_MEMORY; } buffer_len = snprintf(buffer, HUGE_BUFF_MAX, "%d$%d$%"PRIu64, REQUEST_GET_VALUE_ARRAY, res_id, attr_id); if (send(id, buffer, buffer_len, 0) < 0) { _E("[libpass] error occurred while sending buffer"); - ret = -EIO; + ret = TIZEN_ERROR_NO_DATA; goto out_free; } @@ -702,26 +704,26 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ buffer_len = recv(id, buffer, HUGE_BUFF_MAX, 0); if (buffer_len <= 0) { _E("[libpass] socket disconnected"); - ret = -EIO; + ret = TIZEN_ERROR_NO_DATA; goto out_free; } buffer[buffer_len] = '\0'; if (sscanf(buffer, "%d$%d|%d|%[^$]$%d", &response_req, &array_type, &array_len, array_str, &ret) < 5) { - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; goto out_free; } if (response_req != REQUEST_GET_VALUE_ARRAY) { _E("[libpass] wrong response"); - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; goto out_free; } if (data_type != array_type) { _E("[libpass] wrong data type"); - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; goto out_free; } @@ -731,7 +733,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ int32_t *new_array = malloc(array_len * sizeof(int32_t)); if (!new_array) { - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -747,7 +749,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ int64_t *new_array = malloc(array_len * sizeof(int64_t)); if (!new_array) { - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -763,7 +765,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ u_int32_t *new_array = malloc(array_len * sizeof(u_int32_t)); if (!new_array) { - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -779,7 +781,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ u_int64_t *new_array = malloc(array_len * sizeof(u_int64_t)); if (!new_array) { - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -795,7 +797,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ double *new_array = malloc(array_len * sizeof(double)); if (!new_array) { - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -811,7 +813,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ char **new_array = calloc(array_len, sizeof(char *)); if (!new_array) { - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -822,7 +824,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ for (int j = i - 1; j >= 0; j--) free(new_array[j]); free(new_array); - ret = -ENOMEM; + ret = TIZEN_ERROR_OUT_OF_MEMORY; goto out_free; } @@ -835,7 +837,7 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ } default: _E("[libpass] not supported array data type"); - ret = -EINVAL; + ret = TIZEN_ERROR_INVALID_PARAMETER; } out_free: @@ -896,5 +898,5 @@ int pass_resource_monitor_get_resource_timestamp(int id, int res_id, int64_t *st *start = request.ts_start; *end = request.ts_end; - return 0; + return TIZEN_ERROR_NONE; } -- 2.7.4 From 5f56fe2eddb0625b622ee6ba4cd1b96ae183c211 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Mon, 22 Aug 2022 16:59:08 +0900 Subject: [PATCH 09/16] lib: resource-monitor: Change pass_resourced_monitor_is_resource_attr_supported prototype Change pass_resourced_monitor_is_resource_attr_supported prototype to express the various error case and then get the value from l-value style parameter. [Before] bool pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u_int64_t attr_id); [After] int pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u_int64_t attr_id, bool *supported); Change-Id: If4943e034db0a180807a85700ac2db7c87ecef5a Signed-off-by: Chanwoo Choi --- include/util/resource.h | 2 +- lib/resource-monitor/resource-monitor.c | 10 ++-- lib/resource-monitor/resource-monitor.h | 5 +- src/monitor/request-handler.c | 12 +++-- src/util/resource.c | 19 +++++--- tests/integration-test/resource-monitor-tests.cpp | 58 +++++++++++++++++------ tools/resource-monitor/resource-monitor.c | 23 +++++++-- 7 files changed, 95 insertions(+), 34 deletions(-) diff --git a/include/util/resource.h b/include/util/resource.h index 888943e..596f414 100644 --- a/include/util/resource.h +++ b/include/util/resource.h @@ -156,7 +156,7 @@ int update_resource_attrs(struct resource *resource); const struct resource_attribute *get_resource_attr(struct resource *resource, u_int64_t attr_id); struct resource_attribute_value * get_resource_attr_value(struct resource *resource, u_int64_t attr_id); -bool is_resource_attr_supported(struct resource *resource, u_int64_t attr_id); +int is_resource_attr_supported(struct resource *resource, u_int64_t attr_id, bool *supported); static inline bool resource_attr_supported_always(struct resource *resource, diff --git a/lib/resource-monitor/resource-monitor.c b/lib/resource-monitor/resource-monitor.c index 8fa81e9..b36006a 100644 --- a/lib/resource-monitor/resource-monitor.c +++ b/lib/resource-monitor/resource-monitor.c @@ -200,6 +200,7 @@ static inline int handle_request(struct request_data *data) ret = TIZEN_ERROR_INVALID_PARAMETER; break; case REQUEST_GET_VALUE_UINT: + case REQUEST_IS_RESOURCE_ATTR_SUPPORTED: if (sscanf(buffer, "%d$%u$%d", &response_req, &data->value_uint32, &response_ret) < 3) ret = TIZEN_ERROR_INVALID_PARAMETER; @@ -234,7 +235,6 @@ static inline int handle_request(struct request_data *data) case REQUEST_DELETE_RESOURCE: case REQUEST_CREATE_RESOURCE: case REQUEST_SET_RESOURCE_CTRL: - case REQUEST_IS_RESOURCE_ATTR_SUPPORTED: case REQUEST_SET_RESOURCE_ATTR: case REQUEST_UNSET_RESOURCE_ATTR: case REQUEST_IS_RESOURCE_ATTR_SET: @@ -424,7 +424,7 @@ int pass_resource_monitor_unset_resource_attr(int id, int resource_id, u_int64_t } EXPORT -bool pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u_int64_t attr_id) +int pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u_int64_t attr_id, bool *supported) { struct request_data request = { .request = REQUEST_IS_RESOURCE_ATTR_SUPPORTED, @@ -435,7 +435,11 @@ bool pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u int ret; ret = handle_request(&request); - return (ret < 0) ? false : (bool)ret; + if (ret < 0) + return ret; + + *supported = (bool)request.value_uint32; + return TIZEN_ERROR_NONE; } EXPORT diff --git a/lib/resource-monitor/resource-monitor.h b/lib/resource-monitor/resource-monitor.h index b32c452..84d3e9f 100644 --- a/lib/resource-monitor/resource-monitor.h +++ b/lib/resource-monitor/resource-monitor.h @@ -245,9 +245,10 @@ bool pass_resource_monitor_is_resource_attr_set(int id, int resource_id, u_int64 * @param[in] Resource monitor id * @param[in] Resource id * @param[in] Resource attribute id - * @return @c true on success, otherwise false + * @param[out] Resource attribute is either supported or not + * @return @c 0 on success, otherwise false */ -bool pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u_int64_t attr_id); +int pass_resource_monitor_is_resource_attr_supported(int id, int resource_id, u_int64_t attr_id, bool *supported); /** * @brief Update value of the interested attributes for all created resource diff --git a/src/monitor/request-handler.c b/src/monitor/request-handler.c index 2497a15..c81bce4 100644 --- a/src/monitor/request-handler.c +++ b/src/monitor/request-handler.c @@ -250,7 +250,7 @@ static int handle_request_set_resource_attr(struct request_client *client, char return ret; } -static int handle_request_is_resource_attr_supported(struct request_client *client, char *args) +static int handle_request_is_resource_attr_supported(struct request_client *client, char *args, bool *supported) { struct resource *res; int resource_id; @@ -278,7 +278,7 @@ static int handle_request_is_resource_attr_supported(struct request_client *clie return -EINVAL; } - return (int)is_resource_attr_supported(res, attr_id); + return is_resource_attr_supported(res, attr_id, supported); } static int handle_request_set_resource_ctrl(struct request_client *client, char *args) @@ -773,7 +773,13 @@ static int handle_request(struct request_client *client, char *request) ret = handle_request_set_resource_attr(client, args, request_type); break; case REQUEST_IS_RESOURCE_ATTR_SUPPORTED: - ret = handle_request_is_resource_attr_supported(client, args); + { + bool supported; + + ret = handle_request_is_resource_attr_supported(client, args, &supported); + + ADD_RESPONSE(response, buffer_len, "%u$", (u_int32_t)supported); + } break; case REQUEST_GET_RESOURCE_JSON: case REQUEST_GET_VALUE_JSON: diff --git a/src/util/resource.c b/src/util/resource.c index 59f22aa..fa68256 100644 --- a/src/util/resource.c +++ b/src/util/resource.c @@ -401,7 +401,7 @@ get_resource_attr_value(struct resource *resource, u_int64_t attr_id) return &resource->attrs_value[attr_index]; } -bool is_resource_attr_supported(struct resource *resource, u_int64_t attr_id) +int is_resource_attr_supported(struct resource *resource, u_int64_t attr_id, bool *supported) { const struct resource_attribute *attr = NULL; int attr_index = RESOURCE_ATTR_INDEX(attr_id); @@ -410,13 +410,13 @@ bool is_resource_attr_supported(struct resource *resource, u_int64_t attr_id) if (!resource || attr_index < 0 || attr_index >= resource->num_attrs) { _E("Invalid parameter\n"); - return false; + return -EINVAL; } attr = &resource->attrs[attr_index]; if (attr->id & resource->attr_supported) { - return true; + is_supported = true; } else if (attr->ops.is_supported) { is_supported = attr->ops.is_supported(resource, attr); } else if (attr->ops.get) { @@ -434,7 +434,9 @@ bool is_resource_attr_supported(struct resource *resource, u_int64_t attr_id) if (is_supported) resource->attr_supported |= attr->id; - return is_supported; + *supported = is_supported; + + return 0; } static bool check_attr_validate(struct resource *resource, u_int64_t attr_id, int type) @@ -927,6 +929,7 @@ int set_resource_attr_interest(struct resource *resource, u_int64_t interest_mas { struct resource_attribute_value *attr_value; int i, ret; + bool supported; if (!resource) return -EINVAL; @@ -935,8 +938,12 @@ int set_resource_attr_interest(struct resource *resource, u_int64_t interest_mas if (!(resource->attrs[i].id & interest_mask)) continue; - if (!is_resource_attr_supported(resource, resource->attrs[i].id)) { - ret = -EINVAL; + ret = is_resource_attr_supported(resource, resource->attrs[i].id, + &supported); + if (ret < 0) { + goto err; + } else if (!supported){ + ret = -ENOTSUP; goto err; } diff --git a/tests/integration-test/resource-monitor-tests.cpp b/tests/integration-test/resource-monitor-tests.cpp index 2901648..654fbc8 100644 --- a/tests/integration-test/resource-monitor-tests.cpp +++ b/tests/integration-test/resource-monitor-tests.cpp @@ -133,19 +133,27 @@ TEST_F(ResourceMonitorTest, EXPECT_EQ(ret, 0); for (j = 0; j < (int)ARRAY_SIZE(res_attr_ids); j++) { - bool supported = pass_resource_monitor_is_resource_attr_supported( + bool supported; + + ret = pass_resource_monitor_is_resource_attr_supported( id, res_id, - res_attr_ids[j]); + res_attr_ids[j], + &supported); + EXPECT_EQ(ret, 0); EXPECT_EQ(supported, false); - supported = pass_resource_monitor_is_resource_attr_supported( + ret = pass_resource_monitor_is_resource_attr_supported( UNKNOWN_ID, res_id, - res_attr_ids[j]); + res_attr_ids[j], + &supported); + EXPECT_EQ(ret, 0); EXPECT_EQ(supported, false); - supported = pass_resource_monitor_is_resource_attr_supported( + ret = pass_resource_monitor_is_resource_attr_supported( id, UNKNOWN_ID, - res_attr_ids[j]); + res_attr_ids[j], + &supported); + EXPECT_EQ(ret, 0); EXPECT_EQ(supported, false); } @@ -452,8 +460,14 @@ static int __pass_resource_monitor_set_resource_attr(int mon_id, int res_id, int u_int64_t mask = 0; for (i = 0; i < num_attrs; i++) { - bool supported = pass_resource_monitor_is_resource_attr_supported( - mon_id, res_id, attrs[i].attr_id); + bool supported; + + ret = pass_resource_monitor_is_resource_attr_supported( + mon_id, res_id, + attrs[i].attr_id, + &supported); + EXPECT_EQ(ret, 0); + if (supported) mask |= attrs[i].attr_id; } @@ -471,8 +485,14 @@ static int __pass_resource_monitor_unset_resource_attr(int mon_id, int res_id, i u_int64_t mask = 0; for (i = 0; i < num_attrs; i++) { - bool supported = pass_resource_monitor_is_resource_attr_supported( - mon_id, res_id, attrs[i].attr_id); + bool supported; + + ret = pass_resource_monitor_is_resource_attr_supported( + mon_id, res_id, + attrs[i].attr_id, + &supported); + EXPECT_EQ(ret, 0); + if (supported) mask |= attrs[i].attr_id; } @@ -490,8 +510,14 @@ static bool __pass_resource_monitor_is_resource_attr_set(int mon_id, int res_id, u_int64_t mask = 0; for (i = 0; i < num_attrs; i++) { - bool supported = pass_resource_monitor_is_resource_attr_supported( - mon_id, res_id, attrs[i].attr_id); + bool supported; + + ret = pass_resource_monitor_is_resource_attr_supported( + mon_id, res_id, + attrs[i].attr_id, + &supported); + EXPECT_EQ(ret, 0); + if (supported) mask |= attrs[i].attr_id; } @@ -673,8 +699,12 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att int i, ret, err_count = 0; for (i = 0; i < num_attrs; i++) { - bool supported = pass_resource_monitor_is_resource_attr_supported( - mon_id, res_id, attrs[i].attr_id); + bool supported; + + ret = pass_resource_monitor_is_resource_attr_supported( + mon_id, res_id, + attrs[i].attr_id, + &supported); if (!supported) continue; diff --git a/tools/resource-monitor/resource-monitor.c b/tools/resource-monitor/resource-monitor.c index c3d9701..ff7a9a4 100644 --- a/tools/resource-monitor/resource-monitor.c +++ b/tools/resource-monitor/resource-monitor.c @@ -256,7 +256,8 @@ struct __resource_type { static inline void create_resource_and_set_attrs(struct resource_data *res, int ctrl_val) { - int i; + int i, ret; + bool supported; if (!res) return; @@ -268,8 +269,13 @@ static inline void create_resource_and_set_attrs(struct resource_data *res, int res->mon_id, res->res_id, res->ctrl_id, ctrl_val); for (i = 0; i < res->num_attrs; i++) { - if (pass_resource_monitor_is_resource_attr_supported( - res->mon_id, res->res_id, res->attrs[i].id)) + ret = pass_resource_monitor_is_resource_attr_supported( + res->mon_id, res->res_id, res->attrs[i].id, + &supported); + if (ret < 0) + continue; + + if (supported) res->mask |= res->attrs[i].id; } @@ -335,9 +341,16 @@ static inline void get_resource_attr_value(struct resource_data *res, int i) bool supported; int ret = 0; - supported = pass_resource_monitor_is_resource_attr_supported( + ret = pass_resource_monitor_is_resource_attr_supported( res->mon_id, res->res_id, - res->attrs[i].id); + res->attrs[i].id, + &supported); + if(ret < 0) { + printf("%40s | %-5s | %s", "Not Implemented", + res->attrs[i].unit, res->attrs[i].desc); + return; + } + if (!supported) { printf("%40s | %-5s | %s", "Not Supported", res->attrs[i].unit, res->attrs[i].desc); -- 2.7.4 From c7f7b84cf86cefc36cde68379dd042b03e4d187f Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Thu, 18 Aug 2022 20:09:54 +0900 Subject: [PATCH 10/16] monitor: Restructure request server In order to make all initialization about resource monitor handled in monitor module, convert request-server and request-handler from the distinguish module to a part of monitor module. Change-Id: I820147add2b44c40fb0ce1d8e9223c708037adc1 Signed-off-by: Dongwoo Lee --- CMakeLists.txt | 1 - include/monitor/monitor.h | 3 + include/{util => monitor}/request.h | 0 include/util/request-handler.h | 35 ------- lib/resource-monitor/resource-monitor.c | 2 +- src/monitor/monitor.c | 8 ++ src/monitor/request-handler.c | 118 ++++++++++++++++++++-- src/monitor/request-server.c | 174 -------------------------------- 8 files changed, 124 insertions(+), 217 deletions(-) rename include/{util => monitor}/request.h (100%) delete mode 100644 include/util/request-handler.h delete mode 100644 src/monitor/request-server.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 81bcff5..4ea606a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,7 +54,6 @@ SET(SRCS src/monitor/monitor.c src/monitor/monitor-thread.c src/monitor/monitor-command.c - src/monitor/request-server.c src/monitor/request-handler.c ) diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 08e9e21..38b4e62 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -52,4 +52,7 @@ void monitor_command_unbind_resource(struct monitor_command *cmd); int monitor_command_init(struct monitor_command **cmd); void monitor_command_exit(struct monitor_command *cmd); +int request_server_init(void); +void request_server_exit(void); + #endif diff --git a/include/util/request.h b/include/monitor/request.h similarity index 100% rename from include/util/request.h rename to include/monitor/request.h diff --git a/include/util/request-handler.h b/include/util/request-handler.h deleted file mode 100644 index 68355ce..0000000 --- a/include/util/request-handler.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * PASS (Power Aware System Service) - * - * Copyright (c) 2022 Samsung Electronics Co., Ltd. - * - * 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 __REQUEST_HANDLER_H__ -#define __REQUEST_HANDLER_H__ - -#include -#include - -#include - -struct request_client { - int socket_fd; - int nr_resources; - struct thread *worker; - GHashTable *resource_table; -}; - -int create_request_client(int socket_fd); - -#endif /* __REQUEST_HANDLER_H__ */ diff --git a/lib/resource-monitor/resource-monitor.c b/lib/resource-monitor/resource-monitor.c index b36006a..d22540e 100644 --- a/lib/resource-monitor/resource-monitor.c +++ b/lib/resource-monitor/resource-monitor.c @@ -21,7 +21,7 @@ #define _GNU_SOURCE #include #include -#include +#include #include #include diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 50f602c..3e1787f 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -34,9 +34,16 @@ static int monitor_setup(void *data, void *user_data) { int ret; + ret = request_server_init(); + if (ret < 0) { + _E("failed to initialize request server\n"); + return ret; + } + ret = monitor_thread_init(&g_monitor); if (ret < 0) { _E("failed to initialize monitor thread\n"); + request_server_exit(); return ret; } @@ -51,6 +58,7 @@ static void monitor_init(void *data) static void monitor_exit(void *data) { monitor_thread_exit(&g_monitor); + request_server_exit(); unregister_notifier(DEVICE_NOTIFIER_INIT_DONE, monitor_setup, NULL); } diff --git a/src/monitor/request-handler.c b/src/monitor/request-handler.c index c81bce4..068d950 100644 --- a/src/monitor/request-handler.c +++ b/src/monitor/request-handler.c @@ -17,7 +17,7 @@ */ /** - * @file request-handler-thread.c + * @file request-handler.c * @brief TBD * @ingroup TBD */ @@ -28,10 +28,8 @@ #include #include #include -#include -#include -#include -#include +#include +#include #include #include @@ -41,6 +39,19 @@ #include #include +#define PENDING_MAX 3 +#define REQUEST_SERVER_PORT 10001 + +struct request_client { + int socket_fd; + int nr_resources; + struct thread *worker; + GHashTable *resource_table; +}; + +static bool g_request_server_run; +static struct thread *g_server_thread; + static void update_resource(gpointer key, gpointer value, gpointer user_data) { struct resource *res = value; @@ -1003,7 +1014,7 @@ static int request_handler_func(void *data, void **result) return THREAD_RETURN_DONE; } -int create_request_client(int socket_fd) +static int create_request_client(int socket_fd) { struct request_client *client; @@ -1024,3 +1035,98 @@ int create_request_client(int socket_fd) return 0; } +static int request_server_func(void *ctx, void **result) +{ + struct sockaddr_in address; + struct timeval wait; + int opt = true; + int server_socket; + int addrlen; + int ret; + fd_set fds; + + if (!g_request_server_run) + return THREAD_RETURN_DONE; + + init_resource_id(); + + /* 0. initialize server socket */ + server_socket = socket(AF_INET, SOCK_STREAM, 0); + if (server_socket < 0) { + _E("Failed to initialize socket"); + goto error_out; + } + + if (setsockopt(server_socket, SOL_SOCKET, SO_REUSEADDR, + (char *)&opt, sizeof(opt)) < 0) { + _E("Failed to setsockopt"); + goto error_out_close; + } + + address.sin_family = AF_INET; + address.sin_addr.s_addr = INADDR_ANY; + address.sin_port = htons(REQUEST_SERVER_PORT); + + if (bind(server_socket, (struct sockaddr *)&address, sizeof(address)) < 0) { + _E("Failed to bind"); + goto error_out_close; + } + + if (listen(server_socket, PENDING_MAX) < 0) { + _E("Failed to begin listenning"); + goto error_out_close; + } + + addrlen = sizeof(address); + + while (g_request_server_run) { + FD_ZERO(&fds); + FD_SET(server_socket, &fds); + + wait.tv_sec = 1; + wait.tv_usec = 0; + + ret = select(FD_SETSIZE, &fds, NULL, NULL, &wait); + if (ret < 0) { + _E("failed to select"); + goto error_out_close; + } + + if (FD_ISSET(server_socket, &fds)) { + int new_socket = accept(server_socket, (struct sockaddr *)&address, + (socklen_t *)&addrlen); + + if (new_socket < 0) { + _E("Failed to accept"); + goto error_out_close; + } + + create_request_client(new_socket); + } + } + + close(server_socket); + + return THREAD_RETURN_DONE; + +error_out_close: + close(server_socket); +error_out: + + return THREAD_RETURN_ERROR; +} + +int request_server_init(void) +{ + g_request_server_run = true; + if (create_daemon_thread(&g_server_thread, request_server_func, NULL)) + _E("Failed to create request_server_thread "); + + return 0; +} + +void request_server_exit(void) +{ + g_request_server_run = false; + destroy_thread(g_server_thread); +} diff --git a/src/monitor/request-server.c b/src/monitor/request-server.c deleted file mode 100644 index 8a221c5..0000000 --- a/src/monitor/request-server.c +++ /dev/null @@ -1,174 +0,0 @@ -/* - * PASS (Power Aware System Service) - * - * Copyright (c) 2022 Samsung Electronics Co., Ltd. - * - * 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. - */ - -/** - * @file request-server.c - * @brief TBD - * @ingroup TBD - */ - -#include - -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include - -#define PENDING_MAX 3 -#define REQUEST_SERVER_PORT 10001 - -static bool g_request_server_run; - -static int request_server_func(void *ctx, void **result) -{ - struct sockaddr_in address; - struct timeval wait; - int opt = true; - int server_socket; - int addrlen; - int ret; - fd_set fds; - - if (!g_request_server_run) - return THREAD_RETURN_DONE; - - init_resource_id(); - - /* 0. initialize server socket */ - server_socket = socket(AF_INET, SOCK_STREAM, 0); - if (server_socket < 0) { - _E("Failed to initialize socket"); - goto error_out; - } - - if (setsockopt(server_socket, SOL_SOCKET, SO_REUSEADDR, - (char *)&opt, sizeof(opt)) < 0) { - _E("Failed to setsockopt"); - goto error_out_close; - } - - address.sin_family = AF_INET; - address.sin_addr.s_addr = INADDR_ANY; - address.sin_port = htons(REQUEST_SERVER_PORT); - - if (bind(server_socket, (struct sockaddr *)&address, sizeof(address)) < 0) { - _E("Failed to bind"); - goto error_out_close; - } - - if (listen(server_socket, PENDING_MAX) < 0) { - _E("Failed to begin listenning"); - goto error_out_close; - } - - addrlen = sizeof(address); - - while (g_request_server_run) { - FD_ZERO(&fds); - FD_SET(server_socket, &fds); - - wait.tv_sec = 1; - wait.tv_usec = 0; - - ret = select(FD_SETSIZE, &fds, NULL, NULL, &wait); - if (ret < 0) { - _E("failed to select"); - goto error_out_close; - } - - if (FD_ISSET(server_socket, &fds)) { - int new_socket = accept(server_socket, (struct sockaddr *)&address, - (socklen_t *)&addrlen); - - if (new_socket < 0) { - _E("Failed to accept"); - goto error_out_close; - } - - create_request_client(new_socket); - } - } - - close(server_socket); - - return THREAD_RETURN_DONE; - -error_out_close: - close(server_socket); -error_out: - - return THREAD_RETURN_ERROR; -} - -static struct thread *server_thread; - -static int request_server_init_done(void *data, void *user_data) -{ - g_request_server_run = true; - if (create_daemon_thread(&server_thread, request_server_func, NULL)) - _E("Failed to create request_server_thread "); - - return 0; -} - -static void request_server_init(void *data) -{ - /* nothing to do */ -} - -static void request_server_exit(void *data) -{ - unregister_notifier(DEVICE_NOTIFIER_INIT_DONE, request_server_init_done, NULL); - g_request_server_run = false; - destroy_thread(server_thread); -} - -static int request_server_probe(void *data) -{ - int ret = 0; - - ret = register_notifier(DEVICE_NOTIFIER_INIT_DONE, - request_server_init_done, NULL); - if (ret < 0) { - _E("Failed to register a callback function"); - return ret; - } - - return 0; -} - -static const struct device_ops request_server_device_ops = { - .name = "request-server", - .probe = request_server_probe, - .init = request_server_init, - .exit = request_server_exit, -}; - -DEVICE_OPS_REGISTER(&request_server_device_ops); -- 2.7.4 From 5d39adfca161641bcedbc037abb2f65f033e7fb9 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Tue, 23 Aug 2022 08:27:57 +0900 Subject: [PATCH 11/16] pass: Include resource header inside pass.c Currently resource.h is only used in pass.c, but it is included in pass.h and it rather prevents using "delete" keyword due to be used in haltests which is built by cpp compiler. So, this makes resource header be only included in pass.c directly. Change-Id: I3360ceb217ec5d95a75f17a65f2b39bf10e12af6 Signed-off-by: Dongwoo Lee --- src/pass/pass.c | 1 + src/pass/pass.h | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pass/pass.c b/src/pass/pass.c index a0c1a6e..112fa81 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "pass.h" #include "pass-parser.h" diff --git a/src/pass/pass.h b/src/pass/pass.h index 89b31d3..509ed2b 100644 --- a/src/pass/pass.h +++ b/src/pass/pass.h @@ -38,7 +38,6 @@ #include #include -#include #define PASS_LEVEL_COND_MAX 3 -- 2.7.4 From 4b3c9315834e561b5bd21be04f013e8717150797 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Thu, 18 Aug 2022 20:23:56 +0900 Subject: [PATCH 12/16] resource: Add the new device ops create/delete To separate initialization and instance creation on resource driver, the new ops create/delete are introduced. Now, each operation is used as below: - init: Setup the common contexts for resource (called once) - exit: Clear the init-ed contexts (called once) - create: Create resource instance with an individual context - delete: Free own context and remove resource instance Change-Id: I4987f26afd6de7e7ec0b1d8ed6a724acbf55e374 Signed-off-by: Dongwoo Lee --- include/util/resource.h | 6 ++++-- src/util/resource.c | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/util/resource.h b/include/util/resource.h index 596f414..d47db5a 100644 --- a/include/util/resource.h +++ b/include/util/resource.h @@ -92,8 +92,10 @@ struct resource_control { }; struct resource_driver_ops { - int (*init)(struct resource *res); - void (*exit)(struct resource *res); + int (*init)(void); + void (*exit)(void); + int (*create)(struct resource *res); + void (*delete)(struct resource *res); /* * If prepare_update is specified, it will be called * at every update_resource_attrs(). diff --git a/src/util/resource.c b/src/util/resource.c index fa68256..02e5ff9 100644 --- a/src/util/resource.c +++ b/src/util/resource.c @@ -184,8 +184,8 @@ void delete_resource(struct resource *resource) if (!resource) return; - if (resource->driver && resource->driver->ops.exit) - resource->driver->ops.exit(resource); + if (resource->driver && resource->driver->ops.delete) + resource->driver->ops.delete(resource); unset_resource_attr_interest(resource, RESOURCE_ATTR_MASK); @@ -257,8 +257,8 @@ struct resource *create_resource(int resource_type) resource->ctrls = driver->ctrls; resource->num_ctrls = driver->num_ctrls; - if (driver->ops.init) { - ret = driver->ops.init(resource); + if (driver->ops.create) { + ret = driver->ops.create(resource); if (ret < 0) { _E("failed to initialize resource driver, res:type(%s)id(%d)\n", resource->name, resource->id); -- 2.7.4 From ac4f60421a824f1e42a3acc2930772dba4e49c46 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Fri, 19 Aug 2022 16:30:29 +0900 Subject: [PATCH 13/16] resource: Initialize all resource drivers at start up Instead of initalizing common or permanent data for resource drivers at creation, now all initialization will be done at start up once. Change-Id: Idf673d3997c3567b572bcc9ea6dae5e887a0d3f3 Signed-off-by: Dongwoo Lee --- include/util/resource.h | 3 +++ src/monitor/monitor.c | 3 +++ src/util/resource.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/include/util/resource.h b/include/util/resource.h index d47db5a..19a08ab 100644 --- a/include/util/resource.h +++ b/include/util/resource.h @@ -195,6 +195,9 @@ void init_resource_id(void); int set_resource_privdata(struct resource *resource, void *priv); +void init_resource_drivers(void); +void exit_resource_drivers(void); + inline __attribute__((always_inline)) int64_t get_time_now(void) { struct timeval tv; diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 3e1787f..dd0e818 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -34,6 +34,8 @@ static int monitor_setup(void *data, void *user_data) { int ret; + init_resource_drivers(); + ret = request_server_init(); if (ret < 0) { _E("failed to initialize request server\n"); @@ -59,6 +61,7 @@ static void monitor_exit(void *data) { monitor_thread_exit(&g_monitor); request_server_exit(); + exit_resource_drivers(); unregister_notifier(DEVICE_NOTIFIER_INIT_DONE, monitor_setup, NULL); } diff --git a/src/util/resource.c b/src/util/resource.c index 02e5ff9..3e7b828 100644 --- a/src/util/resource.c +++ b/src/util/resource.c @@ -1138,3 +1138,34 @@ int set_resource_privdata(struct resource *resource, void *priv) return 0; } + +void init_resource_drivers(void) +{ + struct resource_driver *driver; + int i, ret = 0; + + for (i = 0; i < g_list_length(g_resource_driver_head); i++) { + driver = g_list_nth(g_list_first(g_resource_driver_head), i)->data; + + if (driver->ops.init) { + ret = driver->ops.init(); + if (ret < 0) { + _D("failed to init resource driver: %s\n", driver->name); + remove_resource_driver(driver); + } + } + } +} + +void exit_resource_drivers(void) +{ + const struct resource_driver *driver; + int i; + + for (i = 0; i < g_list_length(g_resource_driver_head); i++) { + driver = g_list_nth(g_list_first(g_resource_driver_head), i)->data; + + if (driver->ops.exit) + driver->ops.exit(); + } +} -- 2.7.4 From 3104878c229f0fc9932ea920c8a9f7a7454b5314 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Fri, 19 Aug 2022 18:21:57 +0900 Subject: [PATCH 14/16] resource: Distinguish driver initalization from instance creation In the previous commits, each resource driver has the distinguished operation for both initialization and creation. This makes the codes are properly distributed into two operations. Change-Id: Ia3d72914a3277e0d6d18634dff60a9e9763ec362 Signed-off-by: Dongwoo Lee --- src/resource/resource-bus.c | 8 ++-- src/resource/resource-cpu.c | 8 ++-- src/resource/resource-disk.c | 8 ++-- src/resource/resource-display.c | 76 +++++++++++++++++------------------ src/resource/resource-gpu.c | 8 ++-- src/resource/resource-network.c | 8 ++-- src/resource/resource-process-group.c | 52 ++++++++++++------------ src/resource/resource-process.c | 53 ++++++++++++------------ src/resource/resource-system.c | 8 ++-- 9 files changed, 114 insertions(+), 115 deletions(-) diff --git a/src/resource/resource-bus.c b/src/resource/resource-bus.c index 57c02c1..007d1f4 100644 --- a/src/resource/resource-bus.c +++ b/src/resource/resource-bus.c @@ -219,7 +219,7 @@ static const struct resource_control bus_ctrls[] = { }, }; -static int bus_init(struct resource *res) +static int bus_create(struct resource *res) { struct bus_context *ctx; @@ -232,7 +232,7 @@ static int bus_init(struct resource *res) return 0; } -static void bus_exit(struct resource *res) +static void bus_delete(struct resource *res) { struct bus_context *ctx; @@ -259,8 +259,8 @@ static const struct resource_driver bus_resource_driver = { .ctrls = bus_ctrls, .num_ctrls = ARRAY_SIZE(bus_ctrls), .ops = { - .init = bus_init, - .exit = bus_exit, + .create = bus_create, + .delete = bus_delete, }, }; RESOURCE_DRIVER_REGISTER(&bus_resource_driver) diff --git a/src/resource/resource-cpu.c b/src/resource/resource-cpu.c index 23cc507..82ac454 100644 --- a/src/resource/resource-cpu.c +++ b/src/resource/resource-cpu.c @@ -218,7 +218,7 @@ static const struct resource_control cpu_ctrls[] = { }, }; -static int cpu_init(struct resource *res) +static int cpu_create(struct resource *res) { struct cpu_context *ctx = get_resource_privdata(res); @@ -231,7 +231,7 @@ static int cpu_init(struct resource *res) return 0; } -static void cpu_exit(struct resource *res) +static void cpu_delete(struct resource *res) { struct cpu_context *ctx; @@ -259,8 +259,8 @@ static const struct resource_driver cpu_resource_driver = { .ctrls = cpu_ctrls, .num_ctrls = ARRAY_SIZE(cpu_ctrls), .ops = { - .init = cpu_init, - .exit = cpu_exit, + .create = cpu_create, + .delete = cpu_delete, }, }; RESOURCE_DRIVER_REGISTER(&cpu_resource_driver) diff --git a/src/resource/resource-disk.c b/src/resource/resource-disk.c index 3493241..cc6a983 100644 --- a/src/resource/resource-disk.c +++ b/src/resource/resource-disk.c @@ -321,7 +321,7 @@ static int calculate_disk_stats(struct disk_context *ctx) return 0; } -static int disk_init(struct resource *res) +static int disk_create(struct resource *res) { struct disk_context *ctx; @@ -336,7 +336,7 @@ static int disk_init(struct resource *res) return 0; } -static void disk_exit(struct resource *res) +static void disk_delete(struct resource *res) { struct disk_context *ctx; @@ -402,8 +402,8 @@ static const struct resource_driver disk_resource_driver = { .ctrls = disk_ctrls, .num_ctrls = ARRAY_SIZE(disk_ctrls), .ops = { - .init = disk_init, - .exit = disk_exit, + .create = disk_create, + .delete = disk_delete, .prepare_update = disk_prepare_update, }, }; diff --git a/src/resource/resource-display.c b/src/resource/resource-display.c index 724b5df..bd3b203 100644 --- a/src/resource/resource-display.c +++ b/src/resource/resource-display.c @@ -250,37 +250,7 @@ static int display_monitor_func(void *data, void **result) return THREAD_RETURN_CONTINUE; } -static int init_display_monitor_thread(struct resource *res) -{ - int ret; - - g_fps_monitor.resource_count = get_resource_device_count(get_resource_type(res)); - g_fps_monitor.last_fps = calloc(g_fps_monitor.resource_count, sizeof(double)); - if (!g_fps_monitor.last_fps) - return -ENOMEM; - - ret = create_timer_thread(&g_fps_monitor.thread, DISPLAY_SAMPLE_RATE_MSEC, - display_monitor_func, NULL); - - return ret; -} - -static void __DESTRUCTOR__ exit_display_monitor_thread(void) -{ - if (g_fps_monitor.thread != NULL) { - destroy_thread(g_fps_monitor.thread); - g_fps_monitor.thread = NULL; - } - - if (g_fps_monitor.last_fps) { - free(g_fps_monitor.last_fps); - g_fps_monitor.last_fps = NULL; - } - - g_fps_monitor.resource_count = 0; -} - -static int display_init(struct resource *res) +static int display_create(struct resource *res) { int ret = 0; struct display_context *ctx; @@ -294,18 +264,14 @@ static int display_init(struct resource *res) set_resource_privdata(res, ctx); g_mutex_lock(&g_fps_monitor.display_lock); - if (g_fps_monitor.holder++ == 0) { - if (!g_fps_monitor.thread) - ret = init_display_monitor_thread(res); - else - resume_thread(g_fps_monitor.thread); - } + if (g_fps_monitor.holder++ == 0) + resume_thread(g_fps_monitor.thread); g_mutex_unlock(&g_fps_monitor.display_lock); return ret; } -static void display_exit(struct resource *res) +static void display_delete(struct resource *res) { struct display_context *ctx; @@ -323,11 +289,41 @@ static void display_exit(struct resource *res) set_resource_privdata(res, NULL); g_mutex_lock(&g_fps_monitor.display_lock); - if (--g_fps_monitor.holder == 0 && g_fps_monitor.thread != NULL) + if (--g_fps_monitor.holder == 0) suspend_thread(g_fps_monitor.thread); g_mutex_unlock(&g_fps_monitor.display_lock); } +static int display_init(void) +{ + int ret; + + g_fps_monitor.resource_count = get_resource_device_count(RESOURCE_TYPE_DISPLAY); + g_fps_monitor.last_fps = calloc(g_fps_monitor.resource_count, sizeof(double)); + if (!g_fps_monitor.last_fps) + return -ENOMEM; + + ret = create_timer_thread(&g_fps_monitor.thread, DISPLAY_SAMPLE_RATE_MSEC, + display_monitor_func, NULL); + + return ret; +} + +static void display_exit(void) +{ + if (g_fps_monitor.thread) { + destroy_thread(g_fps_monitor.thread); + g_fps_monitor.thread = NULL; + } + + if (g_fps_monitor.last_fps) { + free(g_fps_monitor.last_fps); + g_fps_monitor.last_fps = NULL; + } + + g_fps_monitor.resource_count = 0; +} + static const struct resource_driver display_resource_driver = { .name = "DISPLAY", .type = RESOURCE_TYPE_DISPLAY, @@ -336,6 +332,8 @@ static const struct resource_driver display_resource_driver = { .ctrls = display_ctrls, .num_ctrls = ARRAY_SIZE(display_ctrls), .ops = { + .create = display_create, + .delete = display_delete, .init = display_init, .exit = display_exit, }, diff --git a/src/resource/resource-gpu.c b/src/resource/resource-gpu.c index 9112f3c..5deae14 100644 --- a/src/resource/resource-gpu.c +++ b/src/resource/resource-gpu.c @@ -219,7 +219,7 @@ static const struct resource_control gpu_ctrls[] = { }, }; -static int gpu_init(struct resource *res) +static int gpu_create(struct resource *res) { struct gpu_context *ctx; @@ -232,7 +232,7 @@ static int gpu_init(struct resource *res) return 0; } -static void gpu_exit(struct resource *res) +static void gpu_delete(struct resource *res) { struct gpu_context *ctx; @@ -259,8 +259,8 @@ static const struct resource_driver gpu_resource_driver = { .ctrls = gpu_ctrls, .num_ctrls = ARRAY_SIZE(gpu_ctrls), .ops = { - .init = gpu_init, - .exit = gpu_exit, + .create = gpu_create, + .delete = gpu_delete, }, }; RESOURCE_DRIVER_REGISTER(&gpu_resource_driver) diff --git a/src/resource/resource-network.c b/src/resource/resource-network.c index cfc4887..b05f9ce 100644 --- a/src/resource/resource-network.c +++ b/src/resource/resource-network.c @@ -114,7 +114,7 @@ static const struct resource_control network_ctrls[] = { }, }; -static int network_init(struct resource *res) +static int network_create(struct resource *res) { struct network_context *ctx; @@ -129,7 +129,7 @@ static int network_init(struct resource *res) return 0; } -static void network_exit(struct resource *res) +static void network_delete(struct resource *res) { struct network_context *ctx; @@ -155,8 +155,8 @@ static const struct resource_driver network_resource_driver = { .ctrls = network_ctrls, .num_ctrls = ARRAY_SIZE(network_ctrls), .ops = { - .init = network_init, - .exit = network_exit, + .create = network_create, + .delete = network_delete, }, }; RESOURCE_DRIVER_REGISTER(&network_resource_driver) diff --git a/src/resource/resource-process-group.c b/src/resource/resource-process-group.c index f0a9a4d..5bdbf52 100644 --- a/src/resource/resource-process-group.c +++ b/src/resource/resource-process-group.c @@ -51,7 +51,7 @@ struct process_group_context { static u_int64_t total_memory; static long jiffy; -static int taskstat_support = -1; +static bool taskstat_support; static int process_group_get_pid_list(struct resource *res, const struct resource_attribute *attr, @@ -243,13 +243,13 @@ static int process_group_get_disk_attrs(struct resource *res, static bool process_group_check_taskstat_support(struct resource *resource, const struct resource_attribute *attr) { - return !!taskstat_support; + return taskstat_support; } static bool process_group_check_gpu_support(struct resource *resource, const struct resource_attribute *attr) { - return !!taskstat_support && kernel_check_gpu_support(); + return taskstat_support && kernel_check_gpu_support(); } static const struct resource_attribute process_group_attrs[] = { @@ -649,27 +649,9 @@ out_close: return ret; } -static int process_group_init(struct resource *res) +static int process_group_create(struct resource *res) { struct process_group_context *ctx; - int ret; - - if (taskstat_support < 0) - taskstat_support = (int)kernel_check_taskstat_support(); - - if (jiffy == 0) { - /* get system USER_HZ at once */ - jiffy = sysconf(_SC_CLK_TCK); - if (jiffy < 0) - return -EINVAL; - } - - if (total_memory == 0) { - /* get system total memory once at init*/ - ret = kernel_get_memory_total(&total_memory); - if (ret < 0) - return -EINVAL; - } ctx = malloc(sizeof(struct process_group_context)); if (!ctx) @@ -683,7 +665,7 @@ static int process_group_init(struct resource *res) return 0; } -static void process_group_exit(struct resource *res) +static void process_group_delete(struct resource *res) { struct process_group_context *ctx; @@ -703,6 +685,25 @@ static void process_group_exit(struct resource *res) set_resource_privdata(res, NULL); } +static int process_group_init(void) +{ + int ret; + + /* get system USER_HZ at once */ + jiffy = sysconf(_SC_CLK_TCK); + if (jiffy < 0) + return -EINVAL; + + /* get system total memory once at init*/ + ret = kernel_get_memory_total(&total_memory); + if (ret < 0) + return -EINVAL; + + taskstat_support = kernel_check_taskstat_support(); + + return 0; +} + static const struct resource_driver process_group_driver = { .name = "PROCESS_GROUP", .type = RESOURCE_TYPE_PROCESS_GROUP, @@ -712,9 +713,10 @@ static const struct resource_driver process_group_driver = { .ctrls = process_group_ctrls, .num_ctrls = ARRAY_SIZE(process_group_ctrls), .ops = { - .init = process_group_init, - .exit = process_group_exit, + .create = process_group_create, + .delete = process_group_delete, .prepare_update = process_group_prepare_update, + .init = process_group_init, }, }; RESOURCE_DRIVER_REGISTER(&process_group_driver) diff --git a/src/resource/resource-process.c b/src/resource/resource-process.c index e77d765..dfbe086 100644 --- a/src/resource/resource-process.c +++ b/src/resource/resource-process.c @@ -37,11 +37,11 @@ struct process_context { u_int64_t prev_total_time; double cpu_period; int online_cpu; - u_int64_t total_memory; }; +static u_int64_t total_memory; static long jiffy; -static int taskstat_support = -1; +static bool taskstat_support; static int process_get_cpu_util(struct resource *res, const struct resource_attribute *attr, @@ -113,7 +113,7 @@ static int process_get_mem_attrs(struct resource *res, curr = &ctx->curr; *percent = (curr->coremem * 1024) / curr->ac_stime; - *percent /= ctx->total_memory * 100.0; + *percent /= total_memory * 100.0; break; } case PROCESS_ATTR_MEM_PSS: @@ -252,7 +252,7 @@ static int process_get_context_data(struct resource *res, static bool process_check_taskstat_support(struct resource *resource, const struct resource_attribute *attr) { - return !!taskstat_support; + return taskstat_support; } static bool process_check_gpu_support(struct resource *resource, @@ -394,7 +394,6 @@ static int process_setup_tgid(struct resource *res, const void *data) { struct process_context *ctx; - u_int64_t total_memory; int ret; bool include_gpu_mem; @@ -406,10 +405,8 @@ static int process_setup_tgid(struct resource *res, return -EINVAL; include_gpu_mem = is_resource_attr_interested(res, PROCESS_ATTR_MEM_GPU); - total_memory = ctx->total_memory; memset(ctx, 0, sizeof(*ctx)); - ctx->total_memory = total_memory; ctx->tgid = (pid_t)(intptr_t)data; ctx->prev_total_time = get_total_cpu_time(); @@ -487,37 +484,20 @@ static int process_prepare_update(struct resource *res) return 0; } -static int process_init(struct resource *res) +static int process_create(struct resource *res) { struct process_context *ctx; - int ret; - - if (jiffy == 0) { - /* get system USER_HZ at once */ - jiffy = sysconf(_SC_CLK_TCK); - if (jiffy < 0) - return -EINVAL; - } - - if (taskstat_support < 0) - taskstat_support = (int)kernel_check_taskstat_support(); ctx = calloc(1, sizeof(struct process_context)); if (!ctx) return -ENOMEM; - ret = kernel_get_memory_total(&ctx->total_memory); - if (ret < 0) { - free(ctx); - return -EINVAL; - } - set_resource_privdata(res, ctx); return 0; } -static void process_exit(struct resource *res) +static void process_delete(struct resource *res) { struct process_context *ctx; @@ -532,6 +512,24 @@ static void process_exit(struct resource *res) set_resource_privdata(res, NULL); } +static int process_init(void) +{ + int ret; + + /* get system USER_HZ at once */ + jiffy = sysconf(_SC_CLK_TCK); + if (jiffy < 0) + return -EINVAL; + + ret = kernel_get_memory_total(&total_memory); + if (ret < 0) + return -EINVAL; + + taskstat_support = kernel_check_taskstat_support(); + + return 0; +} + static const struct resource_driver process_resource_driver = { .name = "PROCESS", .type = RESOURCE_TYPE_PROCESS, @@ -541,8 +539,9 @@ static const struct resource_driver process_resource_driver = { .ctrls = process_ctrls, .num_ctrls = ARRAY_SIZE(process_ctrls), .ops = { + .create = process_create, + .delete = process_delete, .init = process_init, - .exit = process_exit, .prepare_update = process_prepare_update, }, }; diff --git a/src/resource/resource-system.c b/src/resource/resource-system.c index dcbb343..6343b11 100644 --- a/src/resource/resource-system.c +++ b/src/resource/resource-system.c @@ -226,7 +226,7 @@ static const struct resource_attribute system_attrs[] = { }, }; -static int system_driver_init(struct resource *res) +static int system_create(struct resource *res) { struct system_resource_data *sysdata; const char *res_name = get_resource_name(res); @@ -272,7 +272,7 @@ err: return ret; } -static void system_driver_exit(struct resource *res) +static void system_delete(struct resource *res) { struct system_resource_data *sysdata; @@ -324,8 +324,8 @@ static const struct resource_driver system_resource_driver = { .attrs = system_attrs, .num_attrs = ARRAY_SIZE(system_attrs), .ops = { - .init = system_driver_init, - .exit = system_driver_exit, + .create = system_create, + .delete = system_delete, .prepare_update = system_driver_prepare_update, }, }; -- 2.7.4 From 744ff5034b204b9188acc4effd9bcb0442bc9e62 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Fri, 19 Aug 2022 19:03:59 +0900 Subject: [PATCH 15/16] util: kernel: Fix to return valid result on getting cpu stat In the case of the target which supports cpu hotplug, the number of possible online cpus can be changed and thus __get_cpu_stat can be failed for offline cpus. At worst case if the last __get_cpu_stat is resulted in negative, kernel_get_per_cpu_stat can return error even though it is not actual failure. To prevent this situation, this ignores the case that __get_cpu_stat returns failure. Change-Id: I5d33e740b2451e7c644b407c1f92785a1e570585 Signed-off-by: Dongwoo Lee --- src/util/kernel.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/util/kernel.c b/src/util/kernel.c index 40fd869..8226b7f 100644 --- a/src/util/kernel.c +++ b/src/util/kernel.c @@ -146,6 +146,7 @@ int kernel_get_total_cpu_stat(struct cpu_stat *total) int kernel_get_per_cpu_stat(struct cpu_stat *cpus, int num_possible_cpus, int *num_online_cpus) { + struct cpu_stat cpu; FILE *fp; char buf[BUFF_MAX]; int i, ret = 0, count = 0; @@ -167,9 +168,16 @@ int kernel_get_per_cpu_stat(struct cpu_stat *cpus, int num_possible_cpus, /* Get per-cpu utilization data */ for (i = 0; i < num_possible_cpus; i++) { - ret = __get_cpu_stat(fp, &cpus[i]); - if (ret < 0) - break; + if (__get_cpu_stat(fp, &cpu) < 0) + continue; + + if (cpu.cpu < 0) { + ret = -EINVAL; + goto err; + } + + memcpy(&cpus[cpu.cpu], &cpu, sizeof(struct cpu_stat)); + count++; } *num_online_cpus = count; -- 2.7.4 From b869d9e23f45b756475c07943308003f8ed5c71c Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Fri, 19 Aug 2022 19:05:05 +0900 Subject: [PATCH 16/16] resource: system: Calculate average cpu utilization with per-cpu stat In a situation where the number of online cpus is dynamically changed, measuring the total system cpu usage by using the sum of the online cpus usage time located at the top of '/proc/stat' will result in inaccurate results. For example, suppose that the number of online cpus changed from 2 to 1 between two time intervals. In this case, the sum of the cpu time measured later is rather reduced than the sum of the cpu time measured first, and if the cpu usage rate is calculated using the difference between these two cpu times, a negative result will come out. To prevent this situation, the average usage is calculated by summing the usage time of each CPU; assuming that the usage rate is 0% for offline CPUs. Change-Id: I93e4561f525b54488df46bf0f895fa76b3b710ca Signed-off-by: Dongwoo Lee --- src/resource/resource-system.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/resource/resource-system.c b/src/resource/resource-system.c index 6343b11..cc2b8a6 100644 --- a/src/resource/resource-system.c +++ b/src/resource/resource-system.c @@ -34,9 +34,6 @@ #include struct system_resource_data { - struct cpu_stat prev_avg; - struct cpu_stat curr_avg; - int num_possible_cpus; int num_online_cpus; struct cpu_stat *prev_cpus; @@ -57,6 +54,9 @@ static double __calculate_cpu_util(int64_t id, struct cpu_stat *prev, total = (double)(diff.user + diff.system + diff.nice + diff.idle); + if (total < 1E-6) + return 0.0; + switch (id) { case SYSTEM_ATTR_CPU_UTIL: case SYSTEM_ATTR_PER_CPU_UTIL: @@ -82,7 +82,8 @@ static int system_get_avg_cpu_util(struct resource *res, void *data) { struct system_resource_data *sysdata; - double *util = (double *)data; + double util, sum = 0.0; + int i; if (!res || !attr || !data) return -EINVAL; @@ -91,13 +92,27 @@ static int system_get_avg_cpu_util(struct resource *res, if (!sysdata) return -EINVAL; - *util = __calculate_cpu_util(attr->id, &sysdata->prev_avg, &sysdata->curr_avg); - if (*util < 0) { - _W("failed to calculate average cpu util (%s: %s)\n", - get_resource_name(res), attr->name); - *util = 0.0; + for (i = 0; i < sysdata->num_possible_cpus; i++) { + util = __calculate_cpu_util(attr->id, + &sysdata->prev_cpus[i], + &sysdata->curr_cpus[i]); + if (util < 0.0) { + _W("failed to calculate per-cpu util (%s: %s)\n", + get_resource_name(res), attr->name); + continue; + } + + sum += util; } + /* + * For offline cpus, it is reasonable to assume that the cpu utilization + * is 0%, so when calculating the average utilization, it is more + * accurate to average the number of possible cpus instead of the number + * of online cpus. + */ + *(double *)data = sum / sysdata->num_possible_cpus; + return 0; } @@ -295,14 +310,6 @@ static int system_driver_prepare_update(struct resource *res) const char *res_name = get_resource_name(res); int ret; - /* Get the average cpu utilization of all cpus */ - memcpy(&sysdata->prev_avg, &sysdata->curr_avg, sizeof(sysdata->prev_avg)); - ret = kernel_get_total_cpu_stat(&sysdata->curr_avg); - if (ret < 0) { - _I("failed to calculate average cpu util (%s)\n", res_name); - return ret; - } - /* Get the per-cpu utilization */ memcpy(sysdata->prev_cpus, sysdata->curr_cpus, sizeof(struct cpu_stat) * sysdata->num_possible_cpus); -- 2.7.4