Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1402946 > unrolled thread

[PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil governor

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil governor Viresh Kumar <viresh.kumar@linaro.org> - 2016-05-18 14:40 +0200
    Re: [PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil governor "Rafael J. Wysocki" <rafael@kernel.org> - 2016-05-18 23:10 +0200
      Re: [PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil  governor Viresh Kumar <viresh.kumar@linaro.org> - 2016-05-19 04:20 +0200
        Re: [PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil governor "Rafael J. Wysocki" <rafael@kernel.org> - 2016-05-19 13:50 +0200

#1402946 — [PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil governor

These macros can be used by governors which don't use the common
governor code present in cpufreq_governor.c and should be moved to the
relevant header.

Now that they are getting moved to the right header file, reuse them in
schedutil governor as well (that required rename of show/store
routines).

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq_governor.h | 8 --------
 include/linux/cpufreq.h            | 8 ++++++++
 kernel/sched/cpufreq_schedutil.c   | 8 ++++----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
index 34eb214b6d57..e2bc5281d3b7 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -70,14 +70,6 @@ static ssize_t show_##file_name						\
 	return sprintf(buf, "%u\n", dbs_data->file_name);		\
 }
 
-#define gov_attr_ro(_name)						\
-static struct governor_attr _name =					\
-__ATTR(_name, 0444, show_##_name, NULL)
-
-#define gov_attr_rw(_name)						\
-static struct governor_attr _name =					\
-__ATTR(_name, 0644, show_##_name, store_##_name)
-
 /* Common to all CPUs of a policy */
 struct policy_dbs_info {
 	struct cpufreq_policy *policy;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 4e81e08db752..fe4c48b0fa40 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -501,6 +501,14 @@ struct gov_attr_set {
 	int usage_count;
 };
 
+#define gov_attr_ro(_name)						\
+static struct governor_attr _name =					\
+__ATTR(_name, 0444, show_##_name, NULL)
+
+#define gov_attr_rw(_name)						\
+static struct governor_attr _name =					\
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
 /* sysfs ops for cpufreq governors */
 extern const struct sysfs_ops governor_sysfs_ops;
 
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 14c4aa25cc45..4035765deafc 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -243,15 +243,15 @@ static inline struct sugov_tunables *to_sugov_tunables(struct gov_attr_set *attr
 	return container_of(attr_set, struct sugov_tunables, attr_set);
 }
 
-static ssize_t rate_limit_us_show(struct gov_attr_set *attr_set, char *buf)
+static ssize_t show_rate_limit_us(struct gov_attr_set *attr_set, char *buf)
 {
 	struct sugov_tunables *tunables = to_sugov_tunables(attr_set);
 
 	return sprintf(buf, "%u\n", tunables->rate_limit_us);
 }
 
-static ssize_t rate_limit_us_store(struct gov_attr_set *attr_set, const char *buf,
-				   size_t count)
+static ssize_t store_rate_limit_us(struct gov_attr_set *attr_set,
+				   const char *buf, size_t count)
 {
 	struct sugov_tunables *tunables = to_sugov_tunables(attr_set);
 	struct sugov_policy *sg_policy;
@@ -268,7 +268,7 @@ static ssize_t rate_limit_us_store(struct gov_attr_set *attr_set, const char *bu
 	return count;
 }
 
-static struct governor_attr rate_limit_us = __ATTR_RW(rate_limit_us);
+gov_attr_rw(rate_limit_us);
 
 static struct attribute *sugov_attributes[] = {
 	&rate_limit_us.attr,
-- 
2.7.1.410.g6faf27b

[toc] | [next] | [standalone]

#1403287

On Wed, May 18, 2016 at 2:25 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> These macros can be used by governors which don't use the common
> governor code present in cpufreq_governor.c and should be moved to the
> relevant header.
>
> Now that they are getting moved to the right header file, reuse them in
> schedutil governor as well (that required rename of show/store
> routines).

I'm not sure what the benefit is to be honest.

It adds one level of indirection to the definition of rate_limit_us,
but why is that better?

[toc] | [prev] | [next] | [standalone]

#1403395 — Re: [PATCH 5/6] cpufreq: Reuse gov_attr_* macros in schedutil governor

On 18-05-16, 23:01, Rafael J. Wysocki wrote:
> On Wed, May 18, 2016 at 2:25 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > These macros can be used by governors which don't use the common
> > governor code present in cpufreq_governor.c and should be moved to the
> > relevant header.
> >
> > Now that they are getting moved to the right header file, reuse them in
> > schedutil governor as well (that required rename of show/store
> > routines).
> 
> I'm not sure what the benefit is to be honest.
> 
> It adds one level of indirection to the definition of rate_limit_us,
> but why is that better?

I agree.

I did it because I am required to use these macros for the
interactive-governor and have to move them to cpufreq.h anyway.

Now that we have to move them out, I thought that we should perhaps
use them for schedutil as well. This would look more relevant to
schedutil once it has more tunables instead of just one.

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1403673

On Thu, May 19, 2016 at 4:13 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18-05-16, 23:01, Rafael J. Wysocki wrote:
>> On Wed, May 18, 2016 at 2:25 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> > These macros can be used by governors which don't use the common
>> > governor code present in cpufreq_governor.c and should be moved to the
>> > relevant header.
>> >
>> > Now that they are getting moved to the right header file, reuse them in
>> > schedutil governor as well (that required rename of show/store
>> > routines).
>>
>> I'm not sure what the benefit is to be honest.
>>
>> It adds one level of indirection to the definition of rate_limit_us,
>> but why is that better?
>
> I agree.
>
> I did it because I am required to use these macros for the
> interactive-governor and have to move them to cpufreq.h anyway.
>
> Now that we have to move them out, I thought that we should perhaps
> use them for schedutil as well. This would look more relevant to
> schedutil once it has more tunables instead of just one.

OK, then let's defer this change until we have interactive in the tree
(which still may or may not happen).

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web