| Message ID | 20191204150018.5234-2-k.konieczny@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | devfreq: improve devfreq statistics counting | expand |
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. > > 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'. > > struct srcu_notifier_head transition_notifier_list; > }; >
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; >> }; >>
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; 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; struct srcu_notifier_head transition_notifier_list; };