diff mbox series

[RFC,3/5] thermal: support statistics table resizing at runtime

Message ID 20200408041917.2329-3-rui.zhang@intel.com (mailing list archive)
State Rejected
Delegated to: Zhang Rui
Headers show
Series [RFC,1/5] thermal: rename thermal_cooling_device_stats_update() | expand

Commit Message

Zhang, Rui April 8, 2020, 4:19 a.m. UTC 
Introduce thermal_cdev_stats_update_max() which can be used to update
the cooling device statistics table when maximum cooling state of a
cooling device is changed.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_sysfs.c | 65 ++++++++++++++++++++++++++-------
 include/linux/thermal.h         |  7 ++++
 2 files changed, 58 insertions(+), 14 deletions(-)

Comments

Takashi Iwai April 8, 2020, 9:45 a.m. UTC | #1 
On Wed, 08 Apr 2020 06:19:15 +0200,
Zhang Rui wrote:
> 
> Introduce thermal_cdev_stats_update_max() which can be used to update
> the cooling device statistics table when maximum cooling state of a
> cooling device is changed.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>

Just a couple of small nitpicking:

> @@ -787,6 +791,23 @@ void thermal_cdev_stats_update_cur(struct thermal_cooling_device *cdev,
>  	spin_unlock(&stats->lock);
>  }
>  
> +void thermal_cdev_stats_update_max(struct thermal_cooling_device *cdev)
> +{
> +	struct cooling_dev_stats *stats = cdev->stats;
> +	unsigned long cur_state, max_state;
> +
> +	if (!stats)
> +		return;
> +
> +	if (cdev->ops->get_max_state(cdev, &max_state))
> +		return;
> +
> +	if (cdev->ops->get_cur_state(cdev, &cur_state))
> +		return;
> +
> +	cooling_device_stats_resize(cdev, cur_state, max_state);
> +}

Don't we need to export this?

> @@ -946,7 +975,15 @@ static void cooling_device_stats_setup(struct thermal_cooling_device *cdev)
>  	cdev->stats = stats;
>  	spin_lock_init(&stats->lock);
>  
> -	ret = cooling_device_stats_resize(cdev);
> +	ret = cdev->ops->get_max_state(cdev, &max_state);
> +	if (ret)
> +		return;
> +
> +	/**
> +	 *  cooling device current state will be updated soon
> +	 *  during registration
> +	 **/

This comment style is confusing as if a kernel-doc.


thanks,

Takashi
Zhang, Rui April 9, 2020, 2:57 a.m. UTC | #2 
On Wed, 2020-04-08 at 11:45 +0200, Takashi Iwai wrote:
> On Wed, 08 Apr 2020 06:19:15 +0200,
> Zhang Rui wrote:
> > 
> > Introduce thermal_cdev_stats_update_max() which can be used to
> > update
> > the cooling device statistics table when maximum cooling state of a
> > cooling device is changed.
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> 
> Just a couple of small nitpicking:
> 
> > @@ -787,6 +791,23 @@ void thermal_cdev_stats_update_cur(struct
> > thermal_cooling_device *cdev,
> >  	spin_unlock(&stats->lock);
> >  }
> >  
> > +void thermal_cdev_stats_update_max(struct thermal_cooling_device
> > *cdev)
> > +{
> > +	struct cooling_dev_stats *stats = cdev->stats;
> > +	unsigned long cur_state, max_state;
> > +
> > +	if (!stats)
> > +		return;
> > +
> > +	if (cdev->ops->get_max_state(cdev, &max_state))
> > +		return;
> > +
> > +	if (cdev->ops->get_cur_state(cdev, &cur_state))
> > +		return;
> > +
> > +	cooling_device_stats_resize(cdev, cur_state, max_state);
> > +}
> 
> Don't we need to export this?

Oh, right, will fix this.
> 
> > @@ -946,7 +975,15 @@ static void cooling_device_stats_setup(struct
> > thermal_cooling_device *cdev)
> >  	cdev->stats = stats;
> >  	spin_lock_init(&stats->lock);
> >  
> > -	ret = cooling_device_stats_resize(cdev);
> > +	ret = cdev->ops->get_max_state(cdev, &max_state);
> > +	if (ret)
> > +		return;
> > +
> > +	/**
> > +	 *  cooling device current state will be updated soon
> > +	 *  during registration
> > +	 **/
> 
> This comment style is confusing as if a kernel-doc.
> 
will fix it in next version.

thanks,
rui
> 
> thanks,
> 
> Takashi
diff mbox series

Patch

diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 45cfc2746874..96e4a445952f 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -745,6 +745,10 @@  static const struct attribute_group *cooling_device_attr_groups[] = {
 };
 
 #ifdef CONFIG_THERMAL_STATISTICS
+static int cooling_device_stats_resize(struct thermal_cooling_device *cdev,
+					unsigned long cur_state,
+					unsigned long max_state);
+
 struct cooling_dev_stats {
 	spinlock_t lock;
 	unsigned int total_trans;
@@ -787,6 +791,23 @@  void thermal_cdev_stats_update_cur(struct thermal_cooling_device *cdev,
 	spin_unlock(&stats->lock);
 }
 
+void thermal_cdev_stats_update_max(struct thermal_cooling_device *cdev)
+{
+	struct cooling_dev_stats *stats = cdev->stats;
+	unsigned long cur_state, max_state;
+
+	if (!stats)
+		return;
+
+	if (cdev->ops->get_max_state(cdev, &max_state))
+		return;
+
+	if (cdev->ops->get_cur_state(cdev, &cur_state))
+		return;
+
+	cooling_device_stats_resize(cdev, cur_state, max_state);
+}
+
 static ssize_t total_trans_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
@@ -907,36 +928,44 @@  static const struct attribute_group cooling_device_stats_attr_group = {
 	.name = "stats"
 };
 
-static int cooling_device_stats_resize(struct thermal_cooling_device *cdev)
+static int cooling_device_stats_resize(struct thermal_cooling_device *cdev,
+					unsigned long cur_state,
+					unsigned long max_state)
 {
 	struct cooling_dev_stats *stats = cdev->stats;
-	unsigned long states;
-	int ret;
-
-	ret = cdev->ops->get_max_state(cdev, &states);
-	if (ret)
-		return ret;
+	unsigned long states = max_state + 1;
+	void *time_in_state, *trans_table;
 
-	states++; /* Total number of states is highest state + 1 */
+	if (stats->max_states == states)
+		return 0;
 
-	stats->time_in_state = kcalloc(states, sizeof(*stats->time_in_state), GFP_KERNEL);
-	if (!stats->time_in_state)
+	time_in_state = kcalloc(states, sizeof(*stats->time_in_state), GFP_KERNEL);
+	if (!time_in_state)
 		return -ENOMEM;
 
-	stats->trans_table = kcalloc(states, sizeof(*stats->trans_table) * states, GFP_KERNEL);
-	if (!stats->trans_table) {
-		kfree(stats->time_in_state);
+	trans_table = kcalloc(states, sizeof(*stats->trans_table) * states, GFP_KERNEL);
+	if (!trans_table) {
+		kfree(time_in_state);
 		return -ENOMEM;
 	}
 
+	spin_lock(&stats->lock);
+	kfree(stats->time_in_state);
+	kfree(stats->trans_table);
+	stats->time_in_state = time_in_state;
+	stats->trans_table = trans_table;
 	stats->last_time = ktime_get();
 	stats->max_states = states;
+	stats->state = cur_state;
+	stats->total_trans = 0;
+	spin_unlock(&stats->lock);
 
 	return 0;
 }
 static void cooling_device_stats_setup(struct thermal_cooling_device *cdev)
 {
 	struct cooling_dev_stats *stats;
+	unsigned long max_state;
 	int var, ret;
 
 	stats = kzalloc(sizeof(*stats), GFP_KERNEL);
@@ -946,7 +975,15 @@  static void cooling_device_stats_setup(struct thermal_cooling_device *cdev)
 	cdev->stats = stats;
 	spin_lock_init(&stats->lock);
 
-	ret = cooling_device_stats_resize(cdev);
+	ret = cdev->ops->get_max_state(cdev, &max_state);
+	if (ret)
+		return;
+
+	/**
+	 *  cooling device current state will be updated soon
+	 *  during registration
+	 **/
+	ret = cooling_device_stats_resize(cdev, 0, max_state);
 	if (ret) {
 		kfree(cdev->stats);
 		cdev->stats = NULL;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 126913c6a53b..cf3fad92f473 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -533,4 +533,11 @@  static inline void thermal_notify_framework(struct thermal_zone_device *tz,
 { }
 #endif /* CONFIG_THERMAL */
 
+#ifdef CONFIG_THERMAL_STATISTICS
+void thermal_cdev_stats_update_max(struct thermal_cooling_device *cdev);
+#else
+static inline void
+thermal_cdev_stats_update_max(struct thermal_cooling_device *cdev) {}
+#endif /* CONFIG_THERMAL_STATISTICS */
+
 #endif /* __THERMAL_H__ */