Messages in this thread | | | Subject | Re: [PATCH v2 1/3] devfreq: change time stats to 64-bit | From | Kamil Konieczny <> | Date | Thu, 5 Dec 2019 10:58:58 +0100 |
| |
Hi,
On 05.12.2019 01:27, Chanwoo Choi wrote: > On 12/5/19 12:00 AM, Kamil Konieczny wrote: >> Change time stats counting to bigger type by using 64-bit jiffies. >> This will make devfreq stats code look similar to cpufreq stats and >> prevents overflow (for HZ = 1000 after 49.7 days). >> >> Signed-off-by: Kamil Konieczny <k.konieczny@samsung.com> >> Acked-by: Chanwoo Choi <cw00.choi@samsung.com> >> --- >> Changes in v2: >> added Acked-by, rebased on linux-next >> >> drivers/devfreq/devfreq.c | 14 +++++++------- >> include/linux/devfreq.h | 4 ++-- >> 2 files changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index bdeb4189c978..0e2030403e4a 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -199,10 +199,10 @@ static int set_freq_table(struct devfreq *devfreq) >> int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >> { >> int lev, prev_lev, ret = 0; >> - unsigned long cur_time; >> + unsigned long long cur_time; > > It looks better to use 'u64' instead of 'unsigned long long'. > Because get_jiffies_u64 has 'u64' return type.
You are right, I will change this and send v3.
>> >> lockdep_assert_held(&devfreq->lock); >> - cur_time = jiffies; >> + cur_time = get_jiffies_64(); >> >> /* Immediately exit if previous_freq is not initialized yet. */ >> if (!devfreq->previous_freq) >> @@ -525,7 +525,7 @@ void devfreq_monitor_resume(struct devfreq *devfreq) >> msecs_to_jiffies(devfreq->profile->polling_ms)); >> >> out_update: >> - devfreq->last_stat_updated = jiffies; >> + devfreq->last_stat_updated = get_jiffies_64(); >> devfreq->stop_polling = false; >> >> if (devfreq->profile->get_cur_freq && >> @@ -748,7 +748,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> >> devfreq->time_in_state = devm_kcalloc(&devfreq->dev, >> devfreq->profile->max_state, >> - sizeof(unsigned long), >> + sizeof(*devfreq->time_in_state), >> GFP_KERNEL); >> if (!devfreq->time_in_state) { >> mutex_unlock(&devfreq->lock); >> @@ -756,7 +756,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >> goto err_devfreq; >> } >> >> - devfreq->last_stat_updated = jiffies; >> + devfreq->last_stat_updated = get_jiffies_64(); >> >> srcu_init_notifier_head(&devfreq->transition_notifier_list); >> >> @@ -1470,8 +1470,8 @@ static ssize_t trans_stat_show(struct device *dev, >> for (j = 0; j < max_state; j++) >> len += sprintf(buf + len, "%10u", >> devfreq->trans_table[(i * max_state) + j]); >> - len += sprintf(buf + len, "%10u\n", >> - jiffies_to_msecs(devfreq->time_in_state[i])); >> + len += sprintf(buf + len, "%10llu\n", (u64) >> + jiffies64_to_msecs(devfreq->time_in_state[i])); >> } >> >> len += sprintf(buf + len, "Total transition : %u\n", >> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >> index 2bae9ed3c783..b81a86e47fb9 100644 >> --- a/include/linux/devfreq.h >> +++ b/include/linux/devfreq.h >> @@ -174,8 +174,8 @@ struct devfreq { >> /* information for device frequency transition */ >> unsigned int total_trans; >> unsigned int *trans_table; >> - unsigned long *time_in_state; >> - unsigned long last_stat_updated; >> + u64 *time_in_state; >> + unsigned long long last_stat_updated; > > ditto. 'unsigned long long' -> 'u64'.
Yes, will change this too.
>> struct srcu_notifier_head transition_notifier_list; >> }; >>
-- Best regards, Kamil Konieczny Samsung R&D Institute Poland
| |