From: Chanwoo Choi <cw00.choi@samsung.com>
Date: Tue, 21 Feb 2023 08:51:04 +0000 (+0900)
Subject: pass: resmon: Move private pass_resmon structure for encapsulation
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1a68612788096258d9361460d3e90af116365c6b;p=platform%2Fcore%2Fsystem%2Fpass.git

pass: resmon: Move private pass_resmon structure for encapsulation

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..e9579dd 100644
--- 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);
@@ -940,6 +947,8 @@ int pass_parser_get_each_resource_config(struct pass_resource *res, const char *
 
 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
--- 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
--- 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
--- a/src/pass/pass-resmon.c
+++ b/src/pass/pass-resmon.c
@@ -43,6 +43,21 @@
 #include "pass-resmon.h"
 #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
  */
@@ -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
--- 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
--- 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
--- 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
--- 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
--- 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
--- 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
 )