On Thu, Mar 3, 2016 at 6:53 AM, Viresh Kumar [off-list ref] wrote:

On 02-03-16, 03:08, Rafael J. Wysocki wrote:

quoted

From: Rafael J. Wysocki <redacted>

In addition to fields representing governor tunables, struct dbs_data
contains some fields needed for the management of objects of that
type.  As it turns out, that part of struct dbs_data may be shared
with (future) governors that won't use the common code used by
"ondemand" and "conservative", so move it to a separate struct type
and modify the code using struct dbs_data to follow.

Signed-off-by: Rafael J. Wysocki <redacted>
---
 drivers/cpufreq/cpufreq_conservative.c |   15 +++--
 drivers/cpufreq/cpufreq_governor.c     |   90 ++++++++++++++++++++-------------
 drivers/cpufreq/cpufreq_governor.h     |   36 +++++++------
 drivers/cpufreq/cpufreq_ondemand.c     |   19 ++++--
 4 files changed, 97 insertions(+), 63 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================

--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h

@@ -41,6 +41,13 @@
 /* Ondemand Sampling types */
 enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE};

+struct gov_tunables {
+     struct kobject kobj;
+     struct list_head policy_list;
+     struct mutex update_lock;
+     int usage_count;
+};

Everything else looks fine, but I don't think that you have named it
properly. Every thing else present in struct dbs_data are tunables,
but not this. And so gov_tunables doesn't suit at all here..

So this is a totally bicycle shed discussion argument which makes it
seriously irritating.

Does it really matter so much how this structure is called?
Essentially, it is something to build your tunables structure around
and you can treat it as a counterpart of a C++ abstract class.  So the
name *does* make sense in that context.

That said, what about gov_attr_set?

Thanks,
Rafael