From 0b250556790f86f92e3a11f8fceca176e830b10a Mon Sep 17 00:00:00 2001 From: YoungHun Kim Date: Wed, 25 Mar 2020 20:57:56 +0900 Subject: [PATCH] Revise the coding rule and dlog of resource handle Change-Id: I4d2b41274666005c62373f91cb14d639f8b1b20d (cherry picked from commit 119f63110c1c752c96deee6b9f7c1af7a2aeeff6) --- .../backend/murphy/mm_resource_manager_rset.c | 2 +- src/daemon/mm_resource_manager_daemon.c | 8 +-- src/daemon/mm_resource_manager_daemon_conf.c | 2 +- src/daemon/mm_resource_manager_daemon_conf.h | 2 +- src/daemon/mm_resource_manager_daemon_priv.c | 70 ++++++++++------------ src/lib/mm_resource_manager_priv.c | 26 ++++---- 6 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/daemon/backend/murphy/mm_resource_manager_rset.c b/src/daemon/backend/murphy/mm_resource_manager_rset.c index 88278ed..32a8b0e 100644 --- a/src/daemon/backend/murphy/mm_resource_manager_rset.c +++ b/src/daemon/backend/murphy/mm_resource_manager_rset.c @@ -25,7 +25,7 @@ -static const char* state_str[MRP_RES_RESOURCE_ABOUT_TO_LOOSE + 1] = { +static const char *state_str[MRP_RES_RESOURCE_ABOUT_TO_LOOSE + 1] = { "lost", "pending", "acquired", diff --git a/src/daemon/mm_resource_manager_daemon.c b/src/daemon/mm_resource_manager_daemon.c index d2416ce..6877296 100644 --- a/src/daemon/mm_resource_manager_daemon.c +++ b/src/daemon/mm_resource_manager_daemon.c @@ -199,10 +199,10 @@ static int set_signal_handlers(void) sigemptyset(&sa_ignore.sa_mask); sa_ignore.sa_flags = 0; - return sigaction(SIGTERM, &sa_term, &rm_term_old_action) != -1 - && sigaction(SIGINT, &sa_ignore, &rm_int_old_action) != -1 - && sigaction(SIGQUIT, &sa_ignore, &rm_quit_old_action) != -1 - && sigaction(SIGHUP, &sa_reload_conf, &rm_hup_old_action) != -1; + return sigaction(SIGTERM, &sa_term, &rm_term_old_action) != -1 && + sigaction(SIGINT, &sa_ignore, &rm_int_old_action) != -1 && + sigaction(SIGQUIT, &sa_ignore, &rm_quit_old_action) != -1 && + sigaction(SIGHUP, &sa_reload_conf, &rm_hup_old_action) != -1; } static void terminate_handler(int signo) diff --git a/src/daemon/mm_resource_manager_daemon_conf.c b/src/daemon/mm_resource_manager_daemon_conf.c index 3a9855a..3d46b8c 100644 --- a/src/daemon/mm_resource_manager_daemon_conf.c +++ b/src/daemon/mm_resource_manager_daemon_conf.c @@ -97,7 +97,7 @@ gboolean mm_resource_manager_reload_conf(void) return TRUE; } -mm_resource_manager_conf_s* mm_resource_manager_get_conf(void) +mm_resource_manager_conf_s *mm_resource_manager_get_conf(void) { return &mm_resource_manager_conf; } diff --git a/src/daemon/mm_resource_manager_daemon_conf.h b/src/daemon/mm_resource_manager_daemon_conf.h index d926cb0..7534665 100644 --- a/src/daemon/mm_resource_manager_daemon_conf.h +++ b/src/daemon/mm_resource_manager_daemon_conf.h @@ -35,6 +35,6 @@ typedef struct { } mm_resource_manager_conf_s; gboolean mm_resource_manager_reload_conf(void); -mm_resource_manager_conf_s* mm_resource_manager_get_conf(void); +mm_resource_manager_conf_s *mm_resource_manager_get_conf(void); #endif /* __MM_RESOURCE_MANAGER_DAEMON_CONF__ */ diff --git a/src/daemon/mm_resource_manager_daemon_priv.c b/src/daemon/mm_resource_manager_daemon_priv.c index 975ff52..5090d16 100644 --- a/src/daemon/mm_resource_manager_daemon_priv.c +++ b/src/daemon/mm_resource_manager_daemon_priv.c @@ -215,21 +215,21 @@ mm_resource_manager_error_e _mmrm_dmn_commit(mm_resource_manager_id id, mm_resource_manager_dmn_res_request_s *increases = NULL; GArray *cb_requests; - MM_RM_RETVM_IF(manager == NULL, - MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER, + MM_RM_RETVM_IF(manager == NULL, MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER, "Resource manager #%"PRIu64" doesn't exist", _mm_rm_hash64(id)); MM_RM_RETVM_IF( - (releases == NULL || releases[0].type == MM_RESOURCE_MANAGER_NO_RES) - && (acquires == NULL || acquires[0].type == MM_RESOURCE_MANAGER_NO_RES), + (releases == NULL || releases[0].type == MM_RESOURCE_MANAGER_NO_RES) && + (acquires == NULL || acquires[0].type == MM_RESOURCE_MANAGER_NO_RES), MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER, "Commit request is empty"); ret = __check_release_requests(manager, releases); - if (ret != MM_RESOURCE_MANAGER_ERROR_NONE) - return ret; + MM_RM_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE, ret, + "check_release_requests is failed [0x%x]", ret); + increases = __create_increase_requests(releases, acquires); - if (increases == NULL) - return MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION; + MM_RM_RETVM_IF(increases == NULL, MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION, + "create_increase_requests is failed"); ret = __check_increase_requests(manager, increases); if (ret != MM_RESOURCE_MANAGER_ERROR_NONE) { @@ -334,8 +334,8 @@ static mm_resource_manager_error_e __check_release_requests(mm_resource_manager_ return MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER; } } else { - for (i = 0; i < manager->resources[type]->parts->len - && ((mm_resource_manager_res_volume*)manager->resources[type]->parts->data)[i] + for (i = 0; i < manager->resources[type]->parts->len && + ((mm_resource_manager_res_volume*)manager->resources[type]->parts->data)[i] != requests->volume; i++); if (i == manager->resources[type]->parts->len) { MM_RM_ERROR("Part of %s of volume %d is not acquired", type_s, requests->volume); @@ -366,8 +366,8 @@ static mm_resource_manager_dmn_res_request_s *__create_increase_requests( for (; acquires->type != MM_RESOURCE_MANAGER_NO_RES; acquires++) { - if ((resources[acquires->type] > 0 || resources[acquires->type] == MM_RESOURCE_MANAGER_RES_VOLUME_FULL) - && acquires->volume == MM_RESOURCE_MANAGER_RES_VOLUME_FULL) { + if ((resources[acquires->type] > 0 || resources[acquires->type] == MM_RESOURCE_MANAGER_RES_VOLUME_FULL) && + acquires->volume == MM_RESOURCE_MANAGER_RES_VOLUME_FULL) { MM_RM_ERROR("The client tries to acquire %s by part and fully at once", _mm_resource_manager_get_res_str(acquires->type)); return NULL; @@ -415,11 +415,11 @@ static mm_resource_manager_error_e __check_increase_requests(mm_resource_manager mm_resource_manager_res_volume remaining_volume; mm_resource_manager_dmn_p i_man; mm_resource_manager_conf_s *conf = mm_resource_manager_get_conf(); - gboolean resource_conflict = FALSE; int i, j, len; mm_resource_manager_res_type_e type = MM_RESOURCE_MANAGER_RES_TYPE_MAX; MM_RM_RETVM_IF(conf == NULL, MM_RESOURCE_MANAGER_ERROR_NONE, "conf is null"); + MM_RM_RETVM_IF(manager == NULL, MM_RESOURCE_MANAGER_ERROR_NONE, "manager is null"); MM_RM_RETVM_IF(requests == NULL, MM_RESOURCE_MANAGER_ERROR_NONE, "requests is null"); len = managers->len; @@ -428,8 +428,8 @@ static mm_resource_manager_error_e __check_increase_requests(mm_resource_manager type = requests->type; const char *type_s = _mm_resource_manager_get_res_str(type); - MM_RM_RETVM_IF(type < MM_RESOURCE_MANAGER_RES_TYPE_VIDEO_DECODER - || type >= MM_RESOURCE_MANAGER_RES_TYPE_MAX, + MM_RM_RETVM_IF(type < MM_RESOURCE_MANAGER_RES_TYPE_VIDEO_DECODER || + type >= MM_RESOURCE_MANAGER_RES_TYPE_MAX, MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER, "wrong type %d", type); MM_RM_RETVM_IF(manager->resources[type] == NULL, @@ -441,14 +441,13 @@ static mm_resource_manager_error_e __check_increase_requests(mm_resource_manager for (i = 0; i < len; i++) { i_man = (mm_resource_manager_dmn_p)managers->pdata[i]; - if (i_man != manager && conf->priority[i_man->app_class] > conf->priority[manager->app_class] - && i_man->resources[type]->is_acquired) { + if (i_man != manager && conf->priority[i_man->app_class] > conf->priority[manager->app_class] && + i_man->resources[type]->is_acquired) { if (i_man->resources[type]->parts) { if (requests->volume == MM_RESOURCE_MANAGER_RES_VOLUME_FULL) { requests->priority_error = TRUE; - resource_conflict = TRUE; MM_RM_DEBUG("Resource conflict. Full volume is requested, but only part is available"); - break; + return MM_RESOURCE_MANAGER_ERROR_LOW_PRIORITY; } else { for (j = 0; j < i_man->resources[type]->parts->len; j++) remaining_volume -= g_array_index(i_man->resources[type]->parts, @@ -456,29 +455,21 @@ static mm_resource_manager_error_e __check_increase_requests(mm_resource_manager if (remaining_volume < requests->volume) { requests->priority_error = TRUE; - resource_conflict = TRUE; MM_RM_DEBUG("Resource conflict. %d of %s are available, but %d required", remaining_volume, type_s, requests->volume); - break; + return MM_RESOURCE_MANAGER_ERROR_LOW_PRIORITY; } } } else { requests->priority_error = TRUE; - resource_conflict = TRUE; MM_RM_DEBUG("Resource conflict. %s is already acquired fully", type_s); - break; + return MM_RESOURCE_MANAGER_ERROR_LOW_PRIORITY; } } } } - if (resource_conflict) { - MM_RM_DEBUG("There is resource conflict"); - return MM_RESOURCE_MANAGER_ERROR_LOW_PRIORITY; - } else { - MM_RM_DEBUG("type %d", type); - return MM_RESOURCE_MANAGER_ERROR_NONE; - } + return MM_RESOURCE_MANAGER_ERROR_NONE; } static void __sync_increase_acquire_requests(mm_resource_manager_dmn_res_request_s *increases, @@ -539,8 +530,8 @@ static GArray *__handle_acquire_requests(mm_resource_manager_dmn_p manager, for (i = 0; i < managers->len; i++) { i_man = (mm_resource_manager_dmn_p)managers->pdata[i]; - if (!i_man->resources[type]->is_acquired - ||conf->priority[i_man->app_class] >conf->priority[manager->app_class]) { + if (!i_man->resources[type]->is_acquired || + conf->priority[i_man->app_class] >conf->priority[manager->app_class]) { i_man->resources[type]->is_acquired = TRUE; if (conf->max_instance[type] > 0) res_count[type]++; @@ -584,8 +575,8 @@ static GArray *__handle_acquire_requests(mm_resource_manager_dmn_p manager, i_man = (mm_resource_manager_dmn_p)managers->pdata[i]; res = i_man->resources[type]->parts; - if (!i_man->resources[type]->is_acquired || res - || conf->priority[i_man->app_class] > conf->priority[manager->app_class]) { + if (!i_man->resources[type]->is_acquired || res || + conf->priority[i_man->app_class] > conf->priority[manager->app_class]) { if (conf->volume_would_be_checked[type] && conf->max_volume[type] >= 0 && !res) { conf->max_volume[type] -= volume; @@ -684,10 +675,11 @@ static void __handle_release_callbacks(GArray *requests) MM_RM_HASH64(id); type = request->type; volume = request->volume; - MM_RM_DEBUG("Sending release callback to RM #%"PRIu64" for %s of volume %d", - id, _mm_resource_manager_get_res_str(type), volume); + MM_RM_INFO("Sending release callback to [man %p] RM #%"PRIu64" for %s of volume %d", + request->manager, id, _mm_resource_manager_get_res_str(type), volume); + _mmrm_dmn_dbus_release_callback(id, type, volume); - if (__wait_for_release_cb_sync(request->manager->id)) + if (__wait_for_release_cb_sync(id)) MM_RM_DEBUG("Release callback sync success"); else MM_RM_ERROR("Wait for release callback sync failed"); @@ -790,9 +782,9 @@ static gboolean __wait_for_release_cb_sync(mm_resource_manager_id id) default: read_size = read(sync.fd, &recv_id, sizeof(recv_id)); if (read_size == sizeof(recv_id)) { - ret = id == recv_id; + ret = id == _mm_rm_hash64(recv_id); if (ret == FALSE) - MM_RM_ERROR("Sync is received from wrong client #%"PRIu64, recv_id); + MM_RM_ERROR("Sync is received from wrong client #%"PRIu64, id); /* * Wait POLLHUP to avoid situation when client sent last sync * through the pipe, but not already closed the pipe handle and diff --git a/src/lib/mm_resource_manager_priv.c b/src/lib/mm_resource_manager_priv.c index a69a3c2..e9f2152 100644 --- a/src/lib/mm_resource_manager_priv.c +++ b/src/lib/mm_resource_manager_priv.c @@ -529,8 +529,8 @@ void __mm_resource_manager_release_callback(mm_resource_manager_s *handle, __mm_resources_unlock(handle); if (release_all) { - if (_mm_resource_manager_mark_all_for_release(handle) == MM_RESOURCE_MANAGER_ERROR_NONE - && _mm_resource_manager_commit(handle) == MM_RESOURCE_MANAGER_ERROR_NONE) { + if (_mm_resource_manager_mark_all_for_release(handle) == MM_RESOURCE_MANAGER_ERROR_NONE && + _mm_resource_manager_commit(handle) == MM_RESOURCE_MANAGER_ERROR_NONE) { MM_RM_DEBUG("All resources are released after release cb"); } else { MM_RM_ERROR("Resources cannot be released after release cb"); @@ -590,9 +590,9 @@ static int __check_resource(mm_resource_manager_s *rm, if (volume > 0) { for (i = 0; i < rm->resources->len; i++) { i_res = (mm_resource_manager_res_p) rm->resources->pdata[i]; - if (i_res->type == type && i_res->state != MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE - && (i_res->volume == MM_RESOURCE_MANAGER_RES_VOLUME_FULL - || (local_volume -= i_res->volume) < volume)) { + if (i_res->type == type && i_res->state != MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE && + (i_res->volume == MM_RESOURCE_MANAGER_RES_VOLUME_FULL || + (local_volume -= i_res->volume) < volume)) { MM_RM_ERROR("Requested volume %d exceeds remaining local volume %d", volume, i_res->volume == MM_RESOURCE_MANAGER_RES_VOLUME_FULL ? 0 : local_volume); @@ -944,7 +944,6 @@ static void __dbus_release_callback(MMResourceManager *object, guint64 arg_id, { mm_resource_manager_s *handle; mm_resource_manager_id handle_id; - gboolean unlock = TRUE; int i; __mm_resource_handles_lock(); @@ -953,37 +952,34 @@ static void __dbus_release_callback(MMResourceManager *object, guint64 arg_id, handle = (mm_resource_manager_s*)handles->pdata[i]; handle_id = handle->id; MM_RM_HASH64(handle_id); + if (handle->dbus_proxy == object && handle_id == arg_id) { __mm_resource_manager_release_callback(handle, arg_id, arg_resource_type, arg_volume); - unlock = FALSE; - break; + return; } } - if (unlock) - __mm_resource_handles_unlock(); + __mm_resource_handles_unlock(); } static void __dbus_status_callback(MMResourceManager *object, gint arg_status) { mm_resource_manager_s *handle; - gboolean unlock = TRUE; int i; MM_RM_INFO("status callback status %d", arg_status); __mm_resource_handles_lock(); + for (i = 0; i < handles->len; i++) { handle = (mm_resource_manager_s*)handles->pdata[i]; if (handle->dbus_proxy == object) { __mm_resource_manager_status_callback(handle, arg_status); - unlock = FALSE; - break; + return; } } - if (unlock) - __mm_resource_handles_unlock(); + __mm_resource_handles_unlock(); } static gpointer __dispatcher_thread(gpointer user_data) -- 2.7.4