* [PATCH 0/4] cpufreq: Record stats with fast-switching
@ 2020-09-02 7:24 Viresh Kumar
2020-09-02 7:24 ` [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition() Viresh Kumar
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Viresh Kumar @ 2020-09-02 7:24 UTC (permalink / raw)
To: Rafael Wysocki, Ben Segall, Dietmar Eggemann, Ingo Molnar,
Juri Lelli, Mel Gorman, Peter Zijlstra, Steven Rostedt,
Vincent Guittot, Viresh Kumar
Cc: linux-pm, Lukasz Luba, cristian.marussi, sudeep.holla, linux-kernel
Hi,
We disabled recording cpufreq stats when fast switching was introduced
to the cpufreq core as the cpufreq stats required to take a spinlock and
that can't be allowed (for performance reasons) on scheduler's hot path.
Here is an attempt to get rid of the lock and bring back the support.
--
Viresh
Viresh Kumar (4):
cpufreq: stats: Defer stats update to
cpufreq_stats_record_transition()
cpufreq: stats: Remove locking
cpufreq: stats: Enable stats for fast-switch as well
cpufreq: Move traces and update to policy->cur to cpufreq core
drivers/cpufreq/cpufreq.c | 16 +++++-
drivers/cpufreq/cpufreq_stats.c | 87 ++++++++++++++++++++------------
kernel/sched/cpufreq_schedutil.c | 12 +----
3 files changed, 72 insertions(+), 43 deletions(-)
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()
2020-09-02 7:24 [PATCH 0/4] cpufreq: Record stats with fast-switching Viresh Kumar
@ 2020-09-02 7:24 ` Viresh Kumar
2020-09-11 10:11 ` peterz
2020-09-15 10:04 ` Lukasz Luba
2020-09-02 7:24 ` [PATCH 2/4] cpufreq: stats: Remove locking Viresh Kumar
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Viresh Kumar @ 2020-09-02 7:24 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar
Cc: linux-pm, Vincent Guittot, Lukasz Luba, cristian.marussi,
sudeep.holla, linux-kernel
In order to prepare for lock-less stats update, add support to defer any
updates to it until cpufreq_stats_record_transition() is called.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq_stats.c | 75 ++++++++++++++++++++++++---------
1 file changed, 56 insertions(+), 19 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index 94d959a8e954..fdf9e8556a49 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -22,17 +22,22 @@ struct cpufreq_stats {
spinlock_t lock;
unsigned int *freq_table;
unsigned int *trans_table;
+
+ /* Deferred reset */
+ atomic_t reset_pending;
+ unsigned long long reset_time;
};
-static void cpufreq_stats_update(struct cpufreq_stats *stats)
+static void cpufreq_stats_update(struct cpufreq_stats *stats,
+ unsigned long long time)
{
unsigned long long cur_time = get_jiffies_64();
- stats->time_in_state[stats->last_index] += cur_time - stats->last_time;
+ stats->time_in_state[stats->last_index] += cur_time - time;
stats->last_time = cur_time;
}
-static void cpufreq_stats_clear_table(struct cpufreq_stats *stats)
+static void cpufreq_stats_reset_table(struct cpufreq_stats *stats)
{
unsigned int count = stats->max_state;
@@ -41,42 +46,67 @@ static void cpufreq_stats_clear_table(struct cpufreq_stats *stats)
memset(stats->trans_table, 0, count * count * sizeof(int));
stats->last_time = get_jiffies_64();
stats->total_trans = 0;
+
+ /* Adjust for the time elapsed since reset was requested */
+ atomic_set(&stats->reset_pending, 0);
+ cpufreq_stats_update(stats, stats->reset_time);
spin_unlock(&stats->lock);
}
static ssize_t show_total_trans(struct cpufreq_policy *policy, char *buf)
{
- return sprintf(buf, "%d\n", policy->stats->total_trans);
+ struct cpufreq_stats *stats = policy->stats;
+
+ if (atomic_read(&stats->reset_pending))
+ return sprintf(buf, "%d\n", 0);
+ else
+ return sprintf(buf, "%d\n", stats->total_trans);
}
cpufreq_freq_attr_ro(total_trans);
static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf)
{
struct cpufreq_stats *stats = policy->stats;
+ bool pending = atomic_read(&stats->reset_pending);
+ unsigned long long time;
ssize_t len = 0;
int i;
if (policy->fast_switch_enabled)
return 0;
- spin_lock(&stats->lock);
- cpufreq_stats_update(stats);
- spin_unlock(&stats->lock);
-
for (i = 0; i < stats->state_num; i++) {
+ if (pending) {
+ if (i == stats->last_index)
+ time = get_jiffies_64() - stats->reset_time;
+ else
+ time = 0;
+ } else {
+ time = stats->time_in_state[i];
+ if (i == stats->last_index)
+ time += get_jiffies_64() - stats->last_time;
+ }
+
len += sprintf(buf + len, "%u %llu\n", stats->freq_table[i],
- (unsigned long long)
- jiffies_64_to_clock_t(stats->time_in_state[i]));
+ jiffies_64_to_clock_t(time));
}
return len;
}
cpufreq_freq_attr_ro(time_in_state);
+/* We don't care what is written to the attribute */
static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf,
size_t count)
{
- /* We don't care what is written to the attribute. */
- cpufreq_stats_clear_table(policy->stats);
+ struct cpufreq_stats *stats = policy->stats;
+
+ /*
+ * Defer resetting of stats to cpufreq_stats_record_transition() to
+ * avoid races.
+ */
+ atomic_set(&stats->reset_pending, 1);
+ stats->reset_time = get_jiffies_64();
+
return count;
}
cpufreq_freq_attr_wo(reset);
@@ -84,8 +114,9 @@ cpufreq_freq_attr_wo(reset);
static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
{
struct cpufreq_stats *stats = policy->stats;
+ bool pending = atomic_read(&stats->reset_pending);
ssize_t len = 0;
- int i, j;
+ int i, j, count;
if (policy->fast_switch_enabled)
return 0;
@@ -113,8 +144,13 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
for (j = 0; j < stats->state_num; j++) {
if (len >= PAGE_SIZE)
break;
- len += scnprintf(buf + len, PAGE_SIZE - len, "%9u ",
- stats->trans_table[i*stats->max_state+j]);
+
+ if (pending)
+ count = 0;
+ else
+ count = stats->trans_table[i * stats->max_state + j];
+
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%9u ", count);
}
if (len >= PAGE_SIZE)
break;
@@ -228,10 +264,11 @@ void cpufreq_stats_record_transition(struct cpufreq_policy *policy,
struct cpufreq_stats *stats = policy->stats;
int old_index, new_index;
- if (!stats) {
- pr_debug("%s: No stats found\n", __func__);
+ if (!stats)
return;
- }
+
+ if (atomic_read(&stats->reset_pending))
+ cpufreq_stats_reset_table(stats);
old_index = stats->last_index;
new_index = freq_table_get_index(stats, new_freq);
@@ -241,7 +278,7 @@ void cpufreq_stats_record_transition(struct cpufreq_policy *policy,
return;
spin_lock(&stats->lock);
- cpufreq_stats_update(stats);
+ cpufreq_stats_update(stats, stats->last_time);
stats->last_index = new_index;
stats->trans_table[old_index * stats->max_state + new_index]++;
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] cpufreq: stats: Remove locking
2020-09-02 7:24 [PATCH 0/4] cpufreq: Record stats with fast-switching Viresh Kumar
2020-09-02 7:24 ` [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition() Viresh Kumar
@ 2020-09-02 7:24 ` Viresh Kumar
2020-09-02 7:24 ` [PATCH 3/4] cpufreq: stats: Enable stats for fast-switch as well Viresh Kumar
2020-09-02 7:24 ` [PATCH 4/4] cpufreq: Move traces and update to policy->cur to cpufreq core Viresh Kumar
3 siblings, 0 replies; 10+ messages in thread
From: Viresh Kumar @ 2020-09-02 7:24 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar
Cc: linux-pm, Vincent Guittot, Lukasz Luba, cristian.marussi,
sudeep.holla, linux-kernel
The locking isn't required anymore as stats can get updated only from
one place at a time. Remove it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq_stats.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index fdf9e8556a49..d86ea9744649 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -19,7 +19,6 @@ struct cpufreq_stats {
unsigned int state_num;
unsigned int last_index;
u64 *time_in_state;
- spinlock_t lock;
unsigned int *freq_table;
unsigned int *trans_table;
@@ -41,7 +40,6 @@ static void cpufreq_stats_reset_table(struct cpufreq_stats *stats)
{
unsigned int count = stats->max_state;
- spin_lock(&stats->lock);
memset(stats->time_in_state, 0, count * sizeof(u64));
memset(stats->trans_table, 0, count * count * sizeof(int));
stats->last_time = get_jiffies_64();
@@ -50,7 +48,6 @@ static void cpufreq_stats_reset_table(struct cpufreq_stats *stats)
/* Adjust for the time elapsed since reset was requested */
atomic_set(&stats->reset_pending, 0);
cpufreq_stats_update(stats, stats->reset_time);
- spin_unlock(&stats->lock);
}
static ssize_t show_total_trans(struct cpufreq_policy *policy, char *buf)
@@ -244,7 +241,6 @@ void cpufreq_stats_create_table(struct cpufreq_policy *policy)
stats->state_num = i;
stats->last_time = get_jiffies_64();
stats->last_index = freq_table_get_index(stats, policy->cur);
- spin_lock_init(&stats->lock);
policy->stats = stats;
ret = sysfs_create_group(&policy->kobj, &stats_attr_group);
@@ -277,11 +273,9 @@ void cpufreq_stats_record_transition(struct cpufreq_policy *policy,
if (old_index == -1 || new_index == -1 || old_index == new_index)
return;
- spin_lock(&stats->lock);
cpufreq_stats_update(stats, stats->last_time);
stats->last_index = new_index;
stats->trans_table[old_index * stats->max_state + new_index]++;
stats->total_trans++;
- spin_unlock(&stats->lock);
}
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] cpufreq: stats: Enable stats for fast-switch as well
2020-09-02 7:24 [PATCH 0/4] cpufreq: Record stats with fast-switching Viresh Kumar
2020-09-02 7:24 ` [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition() Viresh Kumar
2020-09-02 7:24 ` [PATCH 2/4] cpufreq: stats: Remove locking Viresh Kumar
@ 2020-09-02 7:24 ` Viresh Kumar
2020-09-02 7:24 ` [PATCH 4/4] cpufreq: Move traces and update to policy->cur to cpufreq core Viresh Kumar
3 siblings, 0 replies; 10+ messages in thread
From: Viresh Kumar @ 2020-09-02 7:24 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar
Cc: linux-pm, Vincent Guittot, Lukasz Luba, cristian.marussi,
sudeep.holla, linux-kernel
Now that all the blockers are gone for enabling stats in fast-switching
case, enable it.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 6 +++++-
drivers/cpufreq/cpufreq_stats.c | 6 ------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 47aa90f9a7c2..d5fe64e96be9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2057,8 +2057,12 @@ unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
{
target_freq = clamp_val(target_freq, policy->min, policy->max);
+ target_freq = cpufreq_driver->fast_switch(policy, target_freq);
- return cpufreq_driver->fast_switch(policy, target_freq);
+ if (target_freq)
+ cpufreq_stats_record_transition(policy, target_freq);
+
+ return target_freq;
}
EXPORT_SYMBOL_GPL(cpufreq_driver_fast_switch);
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index d86ea9744649..06b5ee12f3b2 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -69,9 +69,6 @@ static ssize_t show_time_in_state(struct cpufreq_policy *policy, char *buf)
ssize_t len = 0;
int i;
- if (policy->fast_switch_enabled)
- return 0;
-
for (i = 0; i < stats->state_num; i++) {
if (pending) {
if (i == stats->last_index)
@@ -115,9 +112,6 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
ssize_t len = 0;
int i, j, count;
- if (policy->fast_switch_enabled)
- return 0;
-
len += scnprintf(buf + len, PAGE_SIZE - len, " From : To\n");
len += scnprintf(buf + len, PAGE_SIZE - len, " : ");
for (i = 0; i < stats->state_num; i++) {
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] cpufreq: Move traces and update to policy->cur to cpufreq core
2020-09-02 7:24 [PATCH 0/4] cpufreq: Record stats with fast-switching Viresh Kumar
` (2 preceding siblings ...)
2020-09-02 7:24 ` [PATCH 3/4] cpufreq: stats: Enable stats for fast-switch as well Viresh Kumar
@ 2020-09-02 7:24 ` Viresh Kumar
3 siblings, 0 replies; 10+ messages in thread
From: Viresh Kumar @ 2020-09-02 7:24 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Ingo Molnar, Peter Zijlstra,
Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Mel Gorman
Cc: linux-pm, Lukasz Luba, cristian.marussi, sudeep.holla, linux-kernel
The cpufreq core handles the updates to policy->cur and recording of
cpufreq trace events for all the governors except schedutil's fast
switch case.
Move that as well to cpufreq core for consistency and readability.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 14 ++++++++++++--
kernel/sched/cpufreq_schedutil.c | 12 +-----------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d5fe64e96be9..bc930f6ecff6 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2056,11 +2056,21 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier);
unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
{
+ int cpu;
+
target_freq = clamp_val(target_freq, policy->min, policy->max);
target_freq = cpufreq_driver->fast_switch(policy, target_freq);
- if (target_freq)
- cpufreq_stats_record_transition(policy, target_freq);
+ if (!target_freq)
+ return 0;
+
+ policy->cur = target_freq;
+ cpufreq_stats_record_transition(policy, target_freq);
+
+ if (trace_cpu_frequency_enabled()) {
+ for_each_cpu(cpu, policy->cpus)
+ trace_cpu_frequency(target_freq, cpu);
+ }
return target_freq;
}
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index e39008242cf4..28f6d1ad608b 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -115,21 +115,11 @@ static void sugov_fast_switch(struct sugov_policy *sg_policy, u64 time,
unsigned int next_freq)
{
struct cpufreq_policy *policy = sg_policy->policy;
- int cpu;
if (!sugov_update_next_freq(sg_policy, time, next_freq))
return;
- next_freq = cpufreq_driver_fast_switch(policy, next_freq);
- if (!next_freq)
- return;
-
- policy->cur = next_freq;
-
- if (trace_cpu_frequency_enabled()) {
- for_each_cpu(cpu, policy->cpus)
- trace_cpu_frequency(next_freq, cpu);
- }
+ cpufreq_driver_fast_switch(policy, next_freq);
}
static void sugov_deferred_update(struct sugov_policy *sg_policy, u64 time,
--
2.25.0.rc1.19.g042ed3e048af
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()
2020-09-02 7:24 ` [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition() Viresh Kumar
@ 2020-09-11 10:11 ` peterz
2020-09-11 11:35 ` Viresh Kumar
2020-09-15 10:04 ` Lukasz Luba
1 sibling, 1 reply; 10+ messages in thread
From: peterz @ 2020-09-11 10:11 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, Lukasz Luba,
cristian.marussi, sudeep.holla, linux-kernel
On Wed, Sep 02, 2020 at 12:54:41PM +0530, Viresh Kumar wrote:
> + atomic_t reset_pending;
> + atomic_set(&stats->reset_pending, 0);
> + if (atomic_read(&stats->reset_pending))
> + bool pending = atomic_read(&stats->reset_pending);
> + atomic_set(&stats->reset_pending, 1);
> + bool pending = atomic_read(&stats->reset_pending);
> + if (atomic_read(&stats->reset_pending))
What do you think atomic_t is doing for you?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()
2020-09-11 10:11 ` peterz
@ 2020-09-11 11:35 ` Viresh Kumar
2020-09-11 12:16 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Viresh Kumar @ 2020-09-11 11:35 UTC (permalink / raw)
To: peterz
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, Lukasz Luba,
cristian.marussi, sudeep.holla, linux-kernel
On 11-09-20, 12:11, peterz@infradead.org wrote:
> On Wed, Sep 02, 2020 at 12:54:41PM +0530, Viresh Kumar wrote:
> > + atomic_t reset_pending;
>
> > + atomic_set(&stats->reset_pending, 0);
> > + if (atomic_read(&stats->reset_pending))
> > + bool pending = atomic_read(&stats->reset_pending);
> > + atomic_set(&stats->reset_pending, 1);
> > + bool pending = atomic_read(&stats->reset_pending);
> > + if (atomic_read(&stats->reset_pending))
>
> What do you think atomic_t is doing for you?
I was trying to avoid races while two writes are going in parallel,
but obviously as this isn't a RMW operation, it won't result in
anything for me.
Maybe what I should be doing is just READ_ONCE()/WRITE_ONCE()? So the
other side doesn't see any intermediate value that was never meant to
be set/read ?
--
viresh
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()
2020-09-11 11:35 ` Viresh Kumar
@ 2020-09-11 12:16 ` Rafael J. Wysocki
0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2020-09-11 12:16 UTC (permalink / raw)
To: Viresh Kumar
Cc: Peter Zijlstra, Rafael Wysocki, Linux PM, Vincent Guittot,
Lukasz Luba, cristian.marussi, Sudeep Holla,
Linux Kernel Mailing List
On Fri, Sep 11, 2020 at 1:36 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 11-09-20, 12:11, peterz@infradead.org wrote:
> > On Wed, Sep 02, 2020 at 12:54:41PM +0530, Viresh Kumar wrote:
> > > + atomic_t reset_pending;
> >
> > > + atomic_set(&stats->reset_pending, 0);
> > > + if (atomic_read(&stats->reset_pending))
> > > + bool pending = atomic_read(&stats->reset_pending);
> > > + atomic_set(&stats->reset_pending, 1);
> > > + bool pending = atomic_read(&stats->reset_pending);
> > > + if (atomic_read(&stats->reset_pending))
> >
> > What do you think atomic_t is doing for you?
>
> I was trying to avoid races while two writes are going in parallel,
> but obviously as this isn't a RMW operation, it won't result in
> anything for me.
>
> Maybe what I should be doing is just READ_ONCE()/WRITE_ONCE()? So the
> other side doesn't see any intermediate value that was never meant to
> be set/read ?
If the value in question is a pointer or an int (or equivalent),
READ_ONCE()/WRITE_ONCE() should be sufficient, and should be used at
least as a matter of annotation of the sensitive code IMO.
IIRC, atomic_set() and atomic_read() are pretty much the same as
WRITE_ONCE() and READ_ONCE(), respectively, anyway.
Cheers!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()
2020-09-02 7:24 ` [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition() Viresh Kumar
2020-09-11 10:11 ` peterz
@ 2020-09-15 10:04 ` Lukasz Luba
2020-09-16 5:56 ` Viresh Kumar
1 sibling, 1 reply; 10+ messages in thread
From: Lukasz Luba @ 2020-09-15 10:04 UTC (permalink / raw)
To: Viresh Kumar, Rafael Wysocki
Cc: linux-pm, Vincent Guittot, cristian.marussi, sudeep.holla, linux-kernel
Hi Viresh,
On 9/2/20 8:24 AM, Viresh Kumar wrote:
> In order to prepare for lock-less stats update, add support to defer any
> updates to it until cpufreq_stats_record_transition() is called.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq_stats.c | 75 ++++++++++++++++++++++++---------
> 1 file changed, 56 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> index 94d959a8e954..fdf9e8556a49 100644
> --- a/drivers/cpufreq/cpufreq_stats.c
> +++ b/drivers/cpufreq/cpufreq_stats.c
> @@ -22,17 +22,22 @@ struct cpufreq_stats {
Would it be possible to move this structure in the
linux/cpufreq.h header? Any subsystem could have access to it,
like to the cpuidle stats.
Apart from that (and the comment regarding the 'atomic_t' field)
I don't see any issues.
Regards,
Lukasz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()
2020-09-15 10:04 ` Lukasz Luba
@ 2020-09-16 5:56 ` Viresh Kumar
0 siblings, 0 replies; 10+ messages in thread
From: Viresh Kumar @ 2020-09-16 5:56 UTC (permalink / raw)
To: Lukasz Luba
Cc: Rafael Wysocki, linux-pm, Vincent Guittot, cristian.marussi,
sudeep.holla, linux-kernel
On 15-09-20, 11:04, Lukasz Luba wrote:
> Hi Viresh,
>
> On 9/2/20 8:24 AM, Viresh Kumar wrote:
> > In order to prepare for lock-less stats update, add support to defer any
> > updates to it until cpufreq_stats_record_transition() is called.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/cpufreq_stats.c | 75 ++++++++++++++++++++++++---------
> > 1 file changed, 56 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> > index 94d959a8e954..fdf9e8556a49 100644
> > --- a/drivers/cpufreq/cpufreq_stats.c
> > +++ b/drivers/cpufreq/cpufreq_stats.c
> > @@ -22,17 +22,22 @@ struct cpufreq_stats {
>
> Would it be possible to move this structure in the
> linux/cpufreq.h header? Any subsystem could have access to it,
> like to the cpuidle stats.
Hmm, I am not sure why we should be doing it. In case of cpuidle many
parts of the kernel are playing with cpuidle code, like drivers/idle/,
drivers/cpuidle, etc.
Something should land in include/ only if you want others to use it,
but in case of cpufreq no one should be using cpufreq stats.
So unless you have a real case where that might be beneficial, I am
going to keep it as is.
> Apart from that (and the comment regarding the 'atomic_t' field)
> I don't see any issues.
Thanks.
--
viresh
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-16 5:56 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 7:24 [PATCH 0/4] cpufreq: Record stats with fast-switching Viresh Kumar
2020-09-02 7:24 ` [PATCH 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition() Viresh Kumar
2020-09-11 10:11 ` peterz
2020-09-11 11:35 ` Viresh Kumar
2020-09-11 12:16 ` Rafael J. Wysocki
2020-09-15 10:04 ` Lukasz Luba
2020-09-16 5:56 ` Viresh Kumar
2020-09-02 7:24 ` [PATCH 2/4] cpufreq: stats: Remove locking Viresh Kumar
2020-09-02 7:24 ` [PATCH 3/4] cpufreq: stats: Enable stats for fast-switch as well Viresh Kumar
2020-09-02 7:24 ` [PATCH 4/4] cpufreq: Move traces and update to policy->cur to cpufreq core Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).