pass: resmon: Move private pass_resmon structure for encapsulation

The struct pass_resmon is private data structure and the other module
cannot access the data of struct pass_resmon directly. In order to
protect the data, move private pass_resmon structure into pass_resmon
module and then provide the proper getter/setter function to handle the
data.

Change-Id: Ic4d1d8d0d11294e173ba19cc08b50a459a878492
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c
index c429fa2..ae9fc90 100644 (file)
--- a/src/pass/pass-parser.c
+++ b/src/pass/pass-parser.c
@@ -45,6 +45,7 @@
 
 #include "pass.h"
 #include "pass-rescon.h"
+#include "pass-resmon.h"
 
 #define MAX_NUM                        255
 #define MIN_TIMEOUT_MS         200
@@ -887,6 +888,11 @@ int pass_parser_get_each_resource_config(struct pass_resource *res, const char *
        if (ret < 0)
                return ret;
 
+       /* Initialize the ResMon's data */
+       ret = pass_resmon_prepare(res);
+       if (ret < 0)
+               return ret;
+
        /* Initialize the CPUHP's data */
        cpuhp->pass_cpu_threshold = 0;
        cpuhp->up_threshold = 0;
@@ -904,7 +910,8 @@ int pass_parser_get_each_resource_config(struct pass_resource *res, const char *
        if (!obj) {
                _E("Failed to get json_object from %s (%s)\n",
                                path, json_util_get_last_err());
-               return -EINVAL;
+               ret = -EINVAL;
+               goto out_resmon;
        }
 
        ret = parse_each_resource(res, obj);
@@ -938,8 +945,14 @@ int pass_parser_get_each_resource_config(struct pass_resource *res, const char *
                goto out;
        }
 
+       json_object_put(obj);
+
+       return 0;
+
 out:
        json_object_put(obj);
+out_resmon:
+       pass_resmon_unprepare(res);
 
        return ret;
 }

diff --git a/src/pass/pass-resmon-internal.h b/src/pass/pass-resmon-internal.h
index 011bb81..0a23a3b 100644 (file)
--- a/src/pass/pass-resmon-internal.h
+++ b/src/pass/pass-resmon-internal.h
@@ -83,6 +83,9 @@ struct resmon_ops {
  *             uevent-based resource monitor.
  */
 struct resmon {
+       /** Instance of pass_resource */
+       struct pass_resource *res;
+
        /** Instance of PASS_MODULE_RESMON (Resource Monitor) module */
        struct pass_resmon *resmon;
 

diff --git a/src/pass/pass-resmon-source.c b/src/pass/pass-resmon-source.c
index 91807d5..2e21e9b 100644 (file)
--- a/src/pass/pass-resmon-source.c
+++ b/src/pass/pass-resmon-source.c
@@ -45,13 +45,16 @@
 static int resmon_thermal_init(struct resmon *monitor)
 {
        struct resmon_result_src_thermal *result;
-       struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res =
-                       container_of(resmon, struct pass_resource, resmon);
+       struct pass_resource *res;
+
+       if (!monitor || !monitor->res)
+               return -EINVAL;
 
        if (monitor->result)
                return 0;
 
+       res = monitor->res;
+
        result = calloc(1, sizeof(*result));
        if (!result)
                return -ENOMEM;
@@ -93,15 +96,18 @@ static int resmon_thermal_exit(struct resmon *monitor)
  */
 static int resmon_thermal_timer_handler(struct resmon *monitor, void *result)
 {
-       struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res =
-                       container_of(resmon, struct pass_resource, resmon);
+       struct pass_resource *res;
        struct resmon_result_src_thermal *thermal_result = result;
        int temp;
 
+       if (!monitor || !monitor->res)
+               return -EINVAL;
+
        if (!thermal_result)
                return -ENOMEM;
 
+       res = monitor->res;
+
        /* Get temperature of h/w resource */
        temp = pass_hal_get_temp(res);
        if (temp < 0)
@@ -123,15 +129,18 @@ static int resmon_thermal_timer_handler(struct resmon *monitor, void *result)
 static int resmon_thermal_uevent_handler(struct resmon *monitor, void *result,
                                                struct udev_device *dev)
 {
-       struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res =
-                       container_of(resmon, struct pass_resource, resmon);
+       struct pass_resource *res;
        struct resmon_result_src_thermal *thermal_result = result;
        int temp;
 
+       if (!monitor || !monitor->res)
+               return -EINVAL;
+
        if (!thermal_result)
                return -ENOMEM;
 
+       res = monitor->res;
+
        /* Get temperature of h/w resource */
        temp = pass_hal_get_temp(res);
        if (temp < 0)
@@ -259,17 +268,20 @@ static int resmon_cpuhp_exit(struct resmon *monitor)
  */
 static int resmon_cpuhp_timer_handler(struct resmon *monitor, void *result)
 {
-       struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res =
-                       container_of(resmon, struct pass_resource, resmon);
+       struct pass_resource *res;
        struct resmon_result_src_cpuhp *cpuhp_result = result;
        char str[BUFF_MAX];
        int i, j, ret;
        FILE *fp = NULL;
 
+       if (!monitor || !monitor->res)
+               return -EINVAL;
+
        if (!cpuhp_result)
                return -ENOMEM;
 
+       res = monitor->res;
+
        fp = fopen(res->config_data.path_load_table, "r");
        if (!fp)
                return -EIO;

diff --git a/src/pass/pass-resmon.c b/src/pass/pass-resmon.c
index f66f06a..516bd79 100644 (file)
--- a/src/pass/pass-resmon.c
+++ b/src/pass/pass-resmon.c
@@ -44,6 +44,21 @@
 #include "pass-resmon-internal.h"
 
 /**
+ * @brief      Represent PASS_MODULE_RESMON (Resource Monitor) module.
+ *             It should be always enabled in order to monitor h/w resources.
+ */
+struct pass_resmon {
+       /** State of PASS_MODULE_RESMON */
+       enum pass_state state;
+
+       /** List of required timer-based resource monitor */
+       GList *timer_list;
+
+       /** List of required uevent-based resource monitor */
+       GList *uevent_list;
+};
+
+/**
  * @brief      Global instance indicating udev
  */
 static struct udev *g_udev = NULL;
@@ -187,9 +202,7 @@ static int resmon_timer_delete(struct resmon *monitor)
 static gboolean resmon_timer_func(gpointer data)
 {
        struct resmon *monitor = data;
-       struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res
-               = container_of(resmon, struct pass_resource, resmon);;
+       struct pass_resource *res = monitor->res;
        int ret = 0;
 
        /* Collect resource data according to enum resmon_src_type */
@@ -238,10 +251,10 @@ void *pass_resmon_get_result(struct pass_resource *res, guint id)
 {
        struct resmon *monitor;
 
-       if (!res)
+       if (!res || !res->resmon)
                return NULL;
 
-       monitor = resmon_find_monitor(&res->resmon, RESMON_TIMER, id);
+       monitor = resmon_find_monitor(res->resmon, RESMON_TIMER, id);
        if (!monitor)
                return NULL;
 
@@ -270,10 +283,10 @@ int pass_resmon_register_timer(struct pass_resource *res,
        int res_type;
        int ret;
 
-       if (!res || timer_type == 0 || user_func == NULL)
+       if (!res || !res->resmon || timer_type == 0 || user_func == NULL)
                return -EINVAL;
 
-       resmon = &res->resmon;
+       resmon = res->resmon;
        res_type = res->config_data.res_type;
 
        /* Allocate the memory for resource monitor */
@@ -281,6 +294,7 @@ int pass_resmon_register_timer(struct pass_resource *res,
        if (!monitor)
                return -ENOMEM;
 
+       monitor->res = res;
        monitor->resmon = resmon;
        monitor->src_type = src_type;
        monitor->timer_type = timer_type;
@@ -319,10 +333,10 @@ int pass_resmon_unregister_timer(struct pass_resource *res, guint id)
        struct resmon *monitor;
        int ret;
 
-       if (!res || id == 0)
+       if (!res || !res->resmon || id == 0)
                return -EINVAL;
 
-       monitor = resmon_find_monitor(&res->resmon, RESMON_TIMER, id);
+       monitor = resmon_find_monitor(res->resmon, RESMON_TIMER, id);
        if (!monitor) {
                _E("failed to find monitor (res_type: %d, id: %d)\n",
                                        res->config_data.res_type, id);
@@ -354,11 +368,11 @@ int pass_resmon_update_timer_interval(struct pass_resource *res,
        struct resmon *monitor;
        int ret;
 
-       if (!res)
+       if (!res || !res->resmon)
                return -EINVAL;
 
        /* Find registered timer-based resmon */
-       monitor = resmon_find_monitor(&res->resmon, RESMON_TIMER, id);
+       monitor = resmon_find_monitor(res->resmon, RESMON_TIMER, id);
        if (!monitor) {
                _E("failed to find monitor (res_type:%d, id: %d)\n",
                                res->config_data.res_type, id);
@@ -385,9 +399,7 @@ int pass_resmon_update_timer_interval(struct pass_resource *res,
 static gboolean resmon_uevent_func(gint fd, GIOCondition cond, void *data)
 {
        struct resmon *monitor = data;
-       struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res
-               = container_of(resmon, struct pass_resource, resmon);;
+       struct pass_resource *res = monitor->res;
        struct udev_device *dev;
        int ret;
 
@@ -435,8 +447,7 @@ static int resmon_uevent_add(struct resmon *monitor)
        static int uevent_id = 0;
        struct udev_monitor *udev_monitor;
        struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res
-               = container_of(resmon, struct pass_resource, resmon);;
+       struct pass_resource *res = monitor->res;
        int ret;
 
        if (!monitor->ops) {
@@ -492,8 +503,7 @@ err_udev_monitor_fd:
 static int resmon_uevent_delete(struct resmon *monitor)
 {
        struct pass_resmon *resmon = monitor->resmon;
-       struct pass_resource *res
-               = container_of(resmon, struct pass_resource, resmon);;
+       struct pass_resource *res = monitor->res;
        int ret;
 
        /* Invoke the .exit of each RESMON_SRC_* source */
@@ -534,12 +544,12 @@ int pass_resmon_register_uevent(struct pass_resource *res,
        struct resmon *monitor;
        int ret;
 
-       if (!res || user_func == NULL) {
+       if (!res || !res->resmon || user_func == NULL) {
                _E("invalid parameter\n");
                return -EINVAL;
        }
 
-       resmon = &res->resmon;
+       resmon = res->resmon;
 
        /* Allocate the memory for resource monitor */
        monitor = calloc(1, sizeof(struct resmon));
@@ -582,12 +592,12 @@ int pass_resmon_unregister_uevent(struct pass_resource *res, guint id)
        struct resmon *monitor;
        int ret;
 
-       if (!res || id == 0) {
+       if (!res || !res->resmon || id == 0) {
                _E("invalid parameter\n");
                return -EINVAL;
        }
 
-       monitor = resmon_find_monitor(&res->resmon, RESMON_UEVENT, id);
+       monitor = resmon_find_monitor(res->resmon, RESMON_UEVENT, id);
        if (!monitor) {
                _E("failed to find monitor (res_name: %s, type: %d)\n",
                                        res->config_data.res_name, id);
@@ -609,6 +619,27 @@ int pass_resmon_unregister_uevent(struct pass_resource *res, guint id)
 
 }
 
+int pass_resmon_prepare(struct pass_resource *res)
+{
+       if (!res || res->resmon != NULL)
+               return -EINVAL;
+
+       res->resmon = calloc(1, sizeof(struct pass_resmon));
+       if (!res->resmon)
+               return -ENOMEM;
+
+       return 0;
+}
+
+void pass_resmon_unprepare(struct pass_resource *res)
+{
+       if (!res || res->resmon == NULL)
+               return;
+
+       free(res->resmon);
+       res->resmon = NULL;
+}
+
 /**
  * @brief      Initialize PASS_MODULE_RESMON(Resource Monitor) module
  * @param      [in] res Instance of h/w resource
@@ -618,12 +649,12 @@ int pass_resmon_init(struct pass_resource *res)
 {
        struct pass_resmon *resmon;
 
-       if (!res)
+       if (!res || !res->resmon)
                return -EINVAL;
-       if (res->resmon.state == PASS_ON)
+       if (res->resmon->state == PASS_ON)
                return -EINVAL;
 
-       resmon = &res->resmon;
+       resmon = res->resmon;
 
        resmon->timer_list = NULL;
 
@@ -651,16 +682,16 @@ int pass_resmon_init(struct pass_resource *res)
  * @param      [in] res Instance of h/w resource
  * @return     @c 0 on success, otherwise error value
  */
-int pass_resmon_exit(struct pass_resource *res)
+int pass_resmon_exit_and_unprepare(struct pass_resource *res)
 {
        struct pass_resmon *resmon;
 
-       if (!res)
+       if (!res || !res->resmon)
                return -EINVAL;
-       if (res->resmon.state == PASS_OFF)
+       if (res->resmon->state == PASS_OFF)
                return -EINVAL;
 
-       resmon = &res->resmon;
+       resmon = res->resmon;
 
        g_list_free(resmon->timer_list);
        resmon->timer_list = NULL;
@@ -676,5 +707,7 @@ int pass_resmon_exit(struct pass_resource *res)
 
        resmon->state = PASS_OFF;
 
+       pass_resmon_unprepare(res);
+
        return 0;
 }

diff --git a/src/pass/pass-resmon.h b/src/pass/pass-resmon.h
index 99d3984..d23381e 100644 (file)
--- a/src/pass/pass-resmon.h
+++ b/src/pass/pass-resmon.h
@@ -170,4 +170,31 @@ int pass_resmon_register_uevent(struct pass_resource *res,
 int pass_resmon_unregister_uevent(struct pass_resource *res,
                                enum resmon_src_type src_type);
 
+/**
+ * @brief      Prepare PASS_MODULE_RESMON(Resource Monitor) module
+ * @param      [in] res Instance of h/w resource
+ * @return     @c 0 on success, otherwise error value
+ */
+int pass_resmon_prepare(struct pass_resource *res);
+
+/**
+ * @brief      Unprepare PASS_MODULE_RESMON(Resource Monitor) module
+ * @param      [in] res Instance of h/w resource
+ */
+void pass_resmon_unprepare(struct pass_resource *res);
+
+/**
+ * @brief      Initialize PASS_MODULE_RESMON(Resource Monitor) module
+ * @param      [in] res Instance of h/w resource
+ * @return     @c 0 on success, otherwise error value
+ */
+int pass_resmon_init(struct pass_resource *res);
+
+/**
+ * @brief      Exit PASS_MODULE_RESMON(Resource Monitor) module
+ * @param      [in] res Instance of h/w resource
+ * @return     @c 0 on success, otherwise error value
+ */
+int pass_resmon_exit_and_unprepare(struct pass_resource *res);
+
 #endif /* __PASS_RESMON__ */

diff --git a/src/pass/pass.c b/src/pass/pass.c
index 4d6824a..fa6d2b4 100644 (file)
--- a/src/pass/pass.c
+++ b/src/pass/pass.c
@@ -44,6 +44,7 @@
 #include "pass-parser.h"
 #include "pass-hal.h"
 #include "pass-rescon.h"
+#include "pass-resmon.h"
 
 #define PASS_JSON_PATH                         "/hal/etc/pass/pass.json"
 
@@ -89,8 +90,6 @@ static uint64 supported_module[] = {
                                        | PASS_MODULE_THERMAL,
 };
 
-extern int pass_resmon_init(struct pass_resource *res);
-extern int pass_resmon_exit(struct pass_resource *res);
 extern int pass_cpuhp_init(struct pass_resource *res);
 extern int pass_cpuhp_exit(struct pass_resource *res);
 extern int pass_pmqos_init(struct pass_resource *res);
@@ -313,7 +312,7 @@ err_pmqos:
                        _E("cannot exit PASS CPUHP");
 err_cpuhp:
        if (is_supported_module(res, PASS_MODULE_RESMON))
-               if (pass_resmon_exit(res) < 0)
+               if (pass_resmon_exit_and_unprepare(res) < 0)
                        _E("cannot exit PASS Resource Monitor");
 err_resmon:
        if (is_supported_module(res, PASS_MODULE_RESCON))
@@ -370,7 +369,7 @@ static int pass_exit_resource(struct pass_resource *res)
         * (Resource Monitor) after called exit() function of modules.
         */
        if (is_supported_module(res, PASS_MODULE_RESMON)) {
-               ret = pass_resmon_exit(res);
+               ret = pass_resmon_exit_and_unprepare(res);
                if (ret < 0) {
                        _E("cannot exit PASS Resource Monitor");
                        return ret;

diff --git a/src/pass/pass.h b/src/pass/pass.h
index 35702fc..1a1d5c2 100644 (file)
--- a/src/pass/pass.h
+++ b/src/pass/pass.h
@@ -44,6 +44,7 @@
 #define PASS_LEVEL_COND_MAX    3
 
 struct pass_rescon;
+struct pass_resmon;
 struct pass_resource;
 struct pass_cpuhp_governor;
 
@@ -282,22 +283,6 @@ struct pass_scenario {
  *                   PASS Module                      *
  ******************************************************/
 
-
-/**
- * @brief      Represent PASS_MODULE_RESMON (Resource Monitor) module.
- *             It should be always enabled in order to monitor h/w resources.
- */
-struct pass_resmon {
-       /** State of PASS_MODULE_RESMON */
-       enum pass_state state;
-
-       /** List of required timer-based resource monitor */
-       GList *timer_list;
-
-       /** List of required uevent-based resource monitor */
-       GList *uevent_list;
-};
-
 /**
  * @brief      Represent PASS_MODULE_PMQOS (PMQoS) module. It may be enabled
  *             or disabled according to configuration.
@@ -588,7 +573,7 @@ struct pass_resource {
        /** Instance of PASS_MODULE_RESCON module */
        struct pass_rescon *rescon;
        /** Instance of PASS_MODULE_RESMON module */
-       struct pass_resmon resmon;
+       struct pass_resmon *resmon;
        /** Instance of PASS_MODULE_CPUHP module */
        struct pass_cpuhp cpuhp;
        /** Instance of PASS_MODULE_PMQOS module */

diff --git a/tests/haltest/CMakeLists.txt b/tests/haltest/CMakeLists.txt
index 1a1bcad..671e7be 100644 (file)
--- a/tests/haltest/CMakeLists.txt
+++ b/tests/haltest/CMakeLists.txt
@@ -4,6 +4,9 @@ SET(SRCS ${CMAKE_SOURCE_DIR}/src/pass/pass-hal.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-rescon.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-parser.c
        ${CMAKE_SOURCE_DIR}/src/util/common.c
+       ${CMAKE_SOURCE_DIR}/src/util/timer.c
+       ${CMAKE_SOURCE_DIR}/src/pass/pass-resmon.c
+       ${CMAKE_SOURCE_DIR}/src/pass/pass-resmon-source.c
 )
 
 INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR})
@@ -18,6 +21,7 @@ pkg_check_modules(gtest_pkgs REQUIRED
        gmock
        dlog
        json-c
+       libudev
        hal-api-power
 )
 

diff --git a/tests/integration-test/CMakeLists.txt b/tests/integration-test/CMakeLists.txt
index c0b9e64..2918a86 100644 (file)
--- a/tests/integration-test/CMakeLists.txt
+++ b/tests/integration-test/CMakeLists.txt
@@ -4,8 +4,11 @@ SET(SRCS ${CMAKE_SOURCE_DIR}/src/pass/pass-hal.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-rescon.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-parser.c
        ${CMAKE_SOURCE_DIR}/src/util/common.c
+       ${CMAKE_SOURCE_DIR}/src/util/timer.c
        ${CMAKE_SOURCE_DIR}/src/util/privilege.c
        ${CMAKE_SOURCE_DIR}/lib/resource-monitor/resource-monitor.c
+       ${CMAKE_SOURCE_DIR}/src/pass/pass-resmon.c
+       ${CMAKE_SOURCE_DIR}/src/pass/pass-resmon-source.c
 )
 
 INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR})
@@ -24,6 +27,7 @@ pkg_check_modules(gtest_pkgs REQUIRED
        hal-api-power
        cynara-client
        cynara-session
+       libudev
 )
 
 FOREACH(flag ${gtest_pkgs_CFLAGS})

diff --git a/tests/unittest/pass-hal-and-parser/CMakeLists.txt b/tests/unittest/pass-hal-and-parser/CMakeLists.txt
index 02a6388..531176d 100644 (file)
--- a/tests/unittest/pass-hal-and-parser/CMakeLists.txt
+++ b/tests/unittest/pass-hal-and-parser/CMakeLists.txt
@@ -6,9 +6,12 @@ SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CFLAGS} -Wall -Werror")
 
 SET(PASS_SRCS
        ${CMAKE_SOURCE_DIR}/src/util/common.c
+       ${CMAKE_SOURCE_DIR}/src/util/timer.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-hal.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-rescon.c
        ${CMAKE_SOURCE_DIR}/src/pass/pass-parser.c
+       ${CMAKE_SOURCE_DIR}/src/pass/pass-resmon.c
+       ${CMAKE_SOURCE_DIR}/src/pass/pass-resmon-source.c
 )
 
 AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR}/ PASS_UNITTEST_SRCS)
@@ -29,6 +32,7 @@ pkg_check_modules(pass_unittest_pkgs REQUIRED
        gmock
        dlog
        json-c
+       libudev
        hal-api-power
 )