2016-11-03 20:47:17

by Markus Mayer

[permalink] [raw]
Subject: [PATCH] cpufreq: stats: clear statistics

From: Markus Mayer <[email protected]>

Allow cpufreq statistics to be cleared by writing anything to
/sys/.../cpufreq/stats/reset. Reading this new sysfs entry returns
nothing.

Resetting the statistics can be useful in a test environment (test
governor, retrieve stats, reset stats, test other governor, etc.). This
feature is not meant for production use.

Signed-off-by: Markus Mayer <[email protected]>
---
drivers/cpufreq/Kconfig | 10 ++++++++++
drivers/cpufreq/cpufreq_stats.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index d8b164a..97a458e 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -45,6 +45,16 @@ config CPU_FREQ_STAT_DETAILS

If in doubt, say N.

+config CPU_FREQ_STAT_RESET
+ bool "Allow reset of CPU frequency transition statistics"
+ depends on CPU_FREQ_STAT
+ help
+ If enabled, writing to /sys/[...]/cpufreq/stats/reset will reset the
+ current CPUfreq statistics. This is primarily meant for testing. It
+ should not be enabled on a production system.
+
+ If in doubt, say N.
+
choice
prompt "Default CPUFreq governor"
default CPU_FREQ_DEFAULT_GOV_USERSPACE if ARM_SA1100_CPUFREQ || ARM_SA1110_CPUFREQ
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index 06d3abd..e4e1e3e 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -111,6 +111,35 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
cpufreq_freq_attr_ro(trans_table);
#endif

+#ifdef CONFIG_CPU_FREQ_STAT_RESET
+static void cpufreq_stats_clear_table(struct cpufreq_policy *policy)
+{
+ struct cpufreq_stats *stats = policy->stats;
+ unsigned int count = stats->max_state;
+
+ memset(stats->time_in_state, 0, count * sizeof(u64));
+#ifdef CONFIG_CPU_FREQ_STAT_DETAILS
+ memset(stats->trans_table, 0, count * count * sizeof(int));
+#endif
+ stats->last_time = get_jiffies_64();
+ stats->total_trans = 0;
+}
+
+static ssize_t show_reset(struct cpufreq_policy *policy, char *buf)
+{
+ buf[0] = '\0';
+ return 0;
+}
+
+static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf,
+ size_t count)
+{
+ cpufreq_stats_clear_table(policy);
+ return count;
+}
+cpufreq_freq_attr_rw(reset);
+#endif
+
cpufreq_freq_attr_ro(total_trans);
cpufreq_freq_attr_ro(time_in_state);

@@ -120,6 +149,9 @@ static struct attribute *default_attrs[] = {
#ifdef CONFIG_CPU_FREQ_STAT_DETAILS
&trans_table.attr,
#endif
+#ifdef CONFIG_CPU_FREQ_STAT_RESET
+ &reset.attr,
+#endif
NULL
};
static struct attribute_group stats_attr_group = {
--
2.7.4


2016-11-04 00:29:01

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: stats: clear statistics

On Thu, Nov 3, 2016 at 9:46 PM, Markus Mayer <[email protected]> wrote:
> From: Markus Mayer <[email protected]>
>
> Allow cpufreq statistics to be cleared by writing anything to
> /sys/.../cpufreq/stats/reset. Reading this new sysfs entry returns
> nothing.
>
> Resetting the statistics can be useful in a test environment (test
> governor, retrieve stats, reset stats, test other governor, etc.). This
> feature is not meant for production use.
>
> Signed-off-by: Markus Mayer <[email protected]>
> ---
> drivers/cpufreq/Kconfig | 10 ++++++++++
> drivers/cpufreq/cpufreq_stats.c | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index d8b164a..97a458e 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -45,6 +45,16 @@ config CPU_FREQ_STAT_DETAILS
>
> If in doubt, say N.
>
> +config CPU_FREQ_STAT_RESET
> + bool "Allow reset of CPU frequency transition statistics"
> + depends on CPU_FREQ_STAT
> + help
> + If enabled, writing to /sys/[...]/cpufreq/stats/reset will reset the
> + current CPUfreq statistics. This is primarily meant for testing. It
> + should not be enabled on a production system.
> +
> + If in doubt, say N.
> +
> choice
> prompt "Default CPUFreq governor"
> default CPU_FREQ_DEFAULT_GOV_USERSPACE if ARM_SA1100_CPUFREQ || ARM_SA1110_CPUFREQ
> diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> index 06d3abd..e4e1e3e 100644
> --- a/drivers/cpufreq/cpufreq_stats.c
> +++ b/drivers/cpufreq/cpufreq_stats.c
> @@ -111,6 +111,35 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> cpufreq_freq_attr_ro(trans_table);
> #endif
>
> +#ifdef CONFIG_CPU_FREQ_STAT_RESET
> +static void cpufreq_stats_clear_table(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_stats *stats = policy->stats;
> + unsigned int count = stats->max_state;
> +
> + memset(stats->time_in_state, 0, count * sizeof(u64));
> +#ifdef CONFIG_CPU_FREQ_STAT_DETAILS
> + memset(stats->trans_table, 0, count * count * sizeof(int));
> +#endif
> + stats->last_time = get_jiffies_64();
> + stats->total_trans = 0;
> +}
> +
> +static ssize_t show_reset(struct cpufreq_policy *policy, char *buf)
> +{
> + buf[0] = '\0';
> + return 0;
> +}
> +
> +static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf,
> + size_t count)
> +{
> + cpufreq_stats_clear_table(policy);
> + return count;
> +}
> +cpufreq_freq_attr_rw(reset);
> +#endif
> +
> cpufreq_freq_attr_ro(total_trans);
> cpufreq_freq_attr_ro(time_in_state);
>
> @@ -120,6 +149,9 @@ static struct attribute *default_attrs[] = {
> #ifdef CONFIG_CPU_FREQ_STAT_DETAILS
> &trans_table.attr,
> #endif
> +#ifdef CONFIG_CPU_FREQ_STAT_RESET
> + &reset.attr,
> +#endif
> NULL
> };
> static struct attribute_group stats_attr_group = {
> --

What would be wrong with adding this unconditionally?

Thanks,
Rafael

2016-11-04 02:12:12

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: stats: clear statistics

On 04-11-16, 01:28, Rafael J. Wysocki wrote:
> On Thu, Nov 3, 2016 at 9:46 PM, Markus Mayer <[email protected]> wrote:
> > From: Markus Mayer <[email protected]>
> >
> > Allow cpufreq statistics to be cleared by writing anything to
> > /sys/.../cpufreq/stats/reset. Reading this new sysfs entry returns
> > nothing.
> >
> > Resetting the statistics can be useful in a test environment (test
> > governor, retrieve stats, reset stats, test other governor, etc.). This
> > feature is not meant for production use.
> >
> > Signed-off-by: Markus Mayer <[email protected]>
> > ---
> > drivers/cpufreq/Kconfig | 10 ++++++++++
> > drivers/cpufreq/cpufreq_stats.c | 32 ++++++++++++++++++++++++++++++++
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> > index d8b164a..97a458e 100644
> > --- a/drivers/cpufreq/Kconfig
> > +++ b/drivers/cpufreq/Kconfig
> > @@ -45,6 +45,16 @@ config CPU_FREQ_STAT_DETAILS
> >
> > If in doubt, say N.
> >
> > +config CPU_FREQ_STAT_RESET
> > + bool "Allow reset of CPU frequency transition statistics"
> > + depends on CPU_FREQ_STAT
> > + help
> > + If enabled, writing to /sys/[...]/cpufreq/stats/reset will reset the
> > + current CPUfreq statistics. This is primarily meant for testing. It
> > + should not be enabled on a production system.
> > +
> > + If in doubt, say N.
> > +
> > choice
> > prompt "Default CPUFreq governor"
> > default CPU_FREQ_DEFAULT_GOV_USERSPACE if ARM_SA1100_CPUFREQ || ARM_SA1110_CPUFREQ
> > diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
> > index 06d3abd..e4e1e3e 100644
> > --- a/drivers/cpufreq/cpufreq_stats.c
> > +++ b/drivers/cpufreq/cpufreq_stats.c
> > @@ -111,6 +111,35 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
> > cpufreq_freq_attr_ro(trans_table);
> > #endif
> >
> > +#ifdef CONFIG_CPU_FREQ_STAT_RESET
> > +static void cpufreq_stats_clear_table(struct cpufreq_policy *policy)
> > +{
> > + struct cpufreq_stats *stats = policy->stats;
> > + unsigned int count = stats->max_state;
> > +
> > + memset(stats->time_in_state, 0, count * sizeof(u64));
> > +#ifdef CONFIG_CPU_FREQ_STAT_DETAILS
> > + memset(stats->trans_table, 0, count * count * sizeof(int));
> > +#endif
> > + stats->last_time = get_jiffies_64();
> > + stats->total_trans = 0;
> > +}
> > +
> > +static ssize_t show_reset(struct cpufreq_policy *policy, char *buf)
> > +{
> > + buf[0] = '\0';
> > + return 0;
> > +}
> > +
> > +static ssize_t store_reset(struct cpufreq_policy *policy, const char *buf,
> > + size_t count)
> > +{
> > + cpufreq_stats_clear_table(policy);
> > + return count;
> > +}
> > +cpufreq_freq_attr_rw(reset);
> > +#endif
> > +
> > cpufreq_freq_attr_ro(total_trans);
> > cpufreq_freq_attr_ro(time_in_state);
> >
> > @@ -120,6 +149,9 @@ static struct attribute *default_attrs[] = {
> > #ifdef CONFIG_CPU_FREQ_STAT_DETAILS
> > &trans_table.attr,
> > #endif
> > +#ifdef CONFIG_CPU_FREQ_STAT_RESET
> > + &reset.attr,
> > +#endif
> > NULL
> > };
> > static struct attribute_group stats_attr_group = {
> > --
>
> What would be wrong with adding this unconditionally?

Absolutely and there is no need to have read permissions for the file. Just make
it write only.

--
viresh