Re: [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
On 09/06/2018 07:09 AM, Quentin Perret wrote: Hi Dietmar, On Wednesday 05 Sep 2018 at 23:56:43 (-0700), Dietmar Eggemann wrote: On 08/20/2018 02:44 AM, Quentin Perret wrote: Expose the Energy Model (read-only) of all performance domains in sysfs for convenience. To do so, add a kobject to the CPU subsystem under the umbrella of which a kobject for each performance domain is attached. The resulting hierarchy is as follows for a platform with two performance domains for example: /sys/devices/system/cpu/energy_model ├── pd0 │ ├── cost │ ├── cpus │ ├── frequency │ └── power cpus (cpumask of the perf domain), frequency (OPP's of the perf domain) and power (values at those OPP's) are somehow easy to grasp, cost is definitely not. You have this nice description in em_pd_energy() what cost actually is. IMHO, might be worth repeating this at least in the patch header here. Hmm, this patch introduces the sysfs interface, not the 'cost' field itself. As long as 'cost' is documented in the patch that introduces it we should be good no ? I mean this patch header tells you _where_ the fields of the structure are exposed. _What_ the structure is all about is a different story. Mmmh, so maybe a EAS related documentation file explaining this interface as well, which can be introduced later is the solution here? I'm just not 100% convinced that those cost values are self-explanatory like the other three items: root@h960:~# ls /sys/devices/system/cpu/energy_model/pd0/ cost cpus frequency power root@h960:~# cat /sys/devices/system/cpu/energy_model/pd0/* 96 129 163 201 245 0-3 533000 999000 1402000 1709000 1844000 28 70 124 187 245 But yeah, in any case, a reminder shouldn't hurt I guess, if you really want one :-) Nothing which should hold this patch-set back though.
Re: [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
On 09/06/2018 07:09 AM, Quentin Perret wrote: Hi Dietmar, On Wednesday 05 Sep 2018 at 23:56:43 (-0700), Dietmar Eggemann wrote: On 08/20/2018 02:44 AM, Quentin Perret wrote: Expose the Energy Model (read-only) of all performance domains in sysfs for convenience. To do so, add a kobject to the CPU subsystem under the umbrella of which a kobject for each performance domain is attached. The resulting hierarchy is as follows for a platform with two performance domains for example: /sys/devices/system/cpu/energy_model ├── pd0 │ ├── cost │ ├── cpus │ ├── frequency │ └── power cpus (cpumask of the perf domain), frequency (OPP's of the perf domain) and power (values at those OPP's) are somehow easy to grasp, cost is definitely not. You have this nice description in em_pd_energy() what cost actually is. IMHO, might be worth repeating this at least in the patch header here. Hmm, this patch introduces the sysfs interface, not the 'cost' field itself. As long as 'cost' is documented in the patch that introduces it we should be good no ? I mean this patch header tells you _where_ the fields of the structure are exposed. _What_ the structure is all about is a different story. Mmmh, so maybe a EAS related documentation file explaining this interface as well, which can be introduced later is the solution here? I'm just not 100% convinced that those cost values are self-explanatory like the other three items: root@h960:~# ls /sys/devices/system/cpu/energy_model/pd0/ cost cpus frequency power root@h960:~# cat /sys/devices/system/cpu/energy_model/pd0/* 96 129 163 201 245 0-3 533000 999000 1402000 1709000 1844000 28 70 124 187 245 But yeah, in any case, a reminder shouldn't hurt I guess, if you really want one :-) Nothing which should hold this patch-set back though.
Re: [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
Hi Dietmar, On Wednesday 05 Sep 2018 at 23:56:43 (-0700), Dietmar Eggemann wrote: > On 08/20/2018 02:44 AM, Quentin Perret wrote: > > Expose the Energy Model (read-only) of all performance domains in sysfs > > for convenience. To do so, add a kobject to the CPU subsystem under the > > umbrella of which a kobject for each performance domain is attached. > > > > The resulting hierarchy is as follows for a platform with two > > performance domains for example: > > > > /sys/devices/system/cpu/energy_model > > ├── pd0 > > │ ├── cost > > │ ├── cpus > > │ ├── frequency > > │ └── power > > cpus (cpumask of the perf domain), frequency (OPP's of the perf domain) and > power (values at those OPP's) are somehow easy to grasp, cost is definitely > not. > > You have this nice description in em_pd_energy() what cost actually is. > IMHO, might be worth repeating this at least in the patch header here. Hmm, this patch introduces the sysfs interface, not the 'cost' field itself. As long as 'cost' is documented in the patch that introduces it we should be good no ? I mean this patch header tells you _where_ the fields of the structure are exposed. _What_ the structure is all about is a different story. But yeah, in any case, a reminder shouldn't hurt I guess, if you really want one :-) Thanks, Quentin
Re: [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
Hi Dietmar, On Wednesday 05 Sep 2018 at 23:56:43 (-0700), Dietmar Eggemann wrote: > On 08/20/2018 02:44 AM, Quentin Perret wrote: > > Expose the Energy Model (read-only) of all performance domains in sysfs > > for convenience. To do so, add a kobject to the CPU subsystem under the > > umbrella of which a kobject for each performance domain is attached. > > > > The resulting hierarchy is as follows for a platform with two > > performance domains for example: > > > > /sys/devices/system/cpu/energy_model > > ├── pd0 > > │ ├── cost > > │ ├── cpus > > │ ├── frequency > > │ └── power > > cpus (cpumask of the perf domain), frequency (OPP's of the perf domain) and > power (values at those OPP's) are somehow easy to grasp, cost is definitely > not. > > You have this nice description in em_pd_energy() what cost actually is. > IMHO, might be worth repeating this at least in the patch header here. Hmm, this patch introduces the sysfs interface, not the 'cost' field itself. As long as 'cost' is documented in the patch that introduces it we should be good no ? I mean this patch header tells you _where_ the fields of the structure are exposed. _What_ the structure is all about is a different story. But yeah, in any case, a reminder shouldn't hurt I guess, if you really want one :-) Thanks, Quentin
Re: [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
On 08/20/2018 02:44 AM, Quentin Perret wrote: Expose the Energy Model (read-only) of all performance domains in sysfs for convenience. To do so, add a kobject to the CPU subsystem under the umbrella of which a kobject for each performance domain is attached. The resulting hierarchy is as follows for a platform with two performance domains for example: /sys/devices/system/cpu/energy_model ├── pd0 │ ├── cost │ ├── cpus │ ├── frequency │ └── power cpus (cpumask of the perf domain), frequency (OPP's of the perf domain) and power (values at those OPP's) are somehow easy to grasp, cost is definitely not. You have this nice description in em_pd_energy() what cost actually is. IMHO, might be worth repeating this at least in the patch header here. [...]
Re: [PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
On 08/20/2018 02:44 AM, Quentin Perret wrote: Expose the Energy Model (read-only) of all performance domains in sysfs for convenience. To do so, add a kobject to the CPU subsystem under the umbrella of which a kobject for each performance domain is attached. The resulting hierarchy is as follows for a platform with two performance domains for example: /sys/devices/system/cpu/energy_model ├── pd0 │ ├── cost │ ├── cpus │ ├── frequency │ └── power cpus (cpumask of the perf domain), frequency (OPP's of the perf domain) and power (values at those OPP's) are somehow easy to grasp, cost is definitely not. You have this nice description in em_pd_energy() what cost actually is. IMHO, might be worth repeating this at least in the patch header here. [...]
[PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
Expose the Energy Model (read-only) of all performance domains in sysfs
for convenience. To do so, add a kobject to the CPU subsystem under the
umbrella of which a kobject for each performance domain is attached.
The resulting hierarchy is as follows for a platform with two
performance domains for example:
/sys/devices/system/cpu/energy_model
├── pd0
│ ├── cost
│ ├── cpus
│ ├── frequency
│ └── power
└── pd4
├── cost
├── cpus
├── frequency
└── power
In this implementation, the kobject abstraction is only used as a
convenient way of exposing data to sysfs. However, it could also be
used in the future to allocate and release performance domains in a more
dynamic way using reference counting.
Cc: Peter Zijlstra
Cc: "Rafael J. Wysocki"
Signed-off-by: Quentin Perret
---
include/linux/energy_model.h | 1 +
kernel/power/energy_model.c | 90
2 files changed, 91 insertions(+)
diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index b89b5596c976..5cfdfa25bf0f 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -19,6 +19,7 @@ struct em_cap_state {
struct em_perf_domain {
struct em_cap_state *table; /* Capacity states, in ascending order. */
int nr_cap_states;
+ struct kobject kobj;
unsigned long cpus[0]; /* CPUs of the frequency domain. */
};
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 2aa6f94343d0..a25d23d73d4e 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -23,6 +23,82 @@ static DEFINE_PER_CPU(struct em_perf_domain *, em_data);
*/
static DEFINE_MUTEX(em_pd_mutex);
+static struct kobject *em_kobject;
+
+/* Getters for the attributes of em_perf_domain objects */
+struct em_pd_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct em_perf_domain *pd, char *buf);
+ ssize_t (*store)(struct em_perf_domain *pd, const char *buf, size_t s);
+};
+
+#define EM_ATTR_LEN 13
+#define show_table_attr(_attr) \
+static ssize_t show_##_attr(struct em_perf_domain *pd, char *buf) \
+{ \
+ ssize_t cnt = 0; \
+ int i; \
+ for (i = 0; i < pd->nr_cap_states; i++) { \
+ if (cnt >= (ssize_t) (PAGE_SIZE / sizeof(char) \
+ - (EM_ATTR_LEN + 2))) \
+ goto out; \
+ cnt += scnprintf([cnt], EM_ATTR_LEN + 1, "%lu ", \
+pd->table[i]._attr); \
+ } \
+out: \
+ cnt += sprintf([cnt], "\n"); \
+ return cnt; \
+}
+
+show_table_attr(power);
+show_table_attr(frequency);
+show_table_attr(cost);
+
+static ssize_t show_cpus(struct em_perf_domain *pd, char *buf)
+{
+ return sprintf(buf, "%*pbl\n", cpumask_pr_args(to_cpumask(pd->cpus)));
+}
+
+#define pd_attr(_name) em_pd_##_name##_attr
+#define define_pd_attr(_name) static struct em_pd_attr pd_attr(_name) = \
+ __ATTR(_name, 0444, show_##_name, NULL)
+
+define_pd_attr(power);
+define_pd_attr(frequency);
+define_pd_attr(cost);
+define_pd_attr(cpus);
+
+static struct attribute *em_pd_default_attrs[] = {
+ _attr(power).attr,
+ _attr(frequency).attr,
+ _attr(cost).attr,
+ _attr(cpus).attr,
+ NULL
+};
+
+#define to_pd(k) container_of(k, struct em_perf_domain, kobj)
+#define to_pd_attr(a) container_of(a, struct em_pd_attr, attr)
+
+static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
+{
+ struct em_perf_domain *pd = to_pd(kobj);
+ struct em_pd_attr *pd_attr = to_pd_attr(attr);
+ ssize_t ret;
+
+ ret = pd_attr->show(pd, buf);
+
+ return ret;
+}
+
+static const struct sysfs_ops em_pd_sysfs_ops = {
+ .show = show,
+};
+
+static struct kobj_type ktype_em_pd = {
+ .sysfs_ops = _pd_sysfs_ops,
+ .default_attrs = em_pd_default_attrs,
+};
+
static struct em_perf_domain *em_create_pd(cpumask_t *span, int nr_states,
struct em_data_callback *cb)
{
@@ -102,6 +178,11 @@ static struct em_perf_domain *em_create_pd(cpumask_t
*span, int nr_states,
pd->nr_cap_states = nr_states;
cpumask_copy(to_cpumask(pd->cpus), span);
+ ret = kobject_init_and_add(>kobj, _em_pd, em_kobject,
+ "pd%u", cpu);
+ if (ret)
+ pr_err("pd%d: failed kobject_init_and_add(): %d\n", cpu, ret);
+
return pd;
free_cs_table:
@@ -155,6 +236,15 @@ int em_register_perf_domain(cpumask_t *span, unsigned int
nr_states,
*/
mutex_lock(_pd_mutex);
+ if (!em_kobject) {
+ em_kobject = kobject_create_and_add("energy_model",
+ _subsys.dev_root->kobj);
+ if (!em_kobject) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+ }
+
[PATCH v6 04/14] PM / EM: Expose the Energy Model in sysfs
Expose the Energy Model (read-only) of all performance domains in sysfs
for convenience. To do so, add a kobject to the CPU subsystem under the
umbrella of which a kobject for each performance domain is attached.
The resulting hierarchy is as follows for a platform with two
performance domains for example:
/sys/devices/system/cpu/energy_model
├── pd0
│ ├── cost
│ ├── cpus
│ ├── frequency
│ └── power
└── pd4
├── cost
├── cpus
├── frequency
└── power
In this implementation, the kobject abstraction is only used as a
convenient way of exposing data to sysfs. However, it could also be
used in the future to allocate and release performance domains in a more
dynamic way using reference counting.
Cc: Peter Zijlstra
Cc: "Rafael J. Wysocki"
Signed-off-by: Quentin Perret
---
include/linux/energy_model.h | 1 +
kernel/power/energy_model.c | 90
2 files changed, 91 insertions(+)
diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index b89b5596c976..5cfdfa25bf0f 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -19,6 +19,7 @@ struct em_cap_state {
struct em_perf_domain {
struct em_cap_state *table; /* Capacity states, in ascending order. */
int nr_cap_states;
+ struct kobject kobj;
unsigned long cpus[0]; /* CPUs of the frequency domain. */
};
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 2aa6f94343d0..a25d23d73d4e 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -23,6 +23,82 @@ static DEFINE_PER_CPU(struct em_perf_domain *, em_data);
*/
static DEFINE_MUTEX(em_pd_mutex);
+static struct kobject *em_kobject;
+
+/* Getters for the attributes of em_perf_domain objects */
+struct em_pd_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct em_perf_domain *pd, char *buf);
+ ssize_t (*store)(struct em_perf_domain *pd, const char *buf, size_t s);
+};
+
+#define EM_ATTR_LEN 13
+#define show_table_attr(_attr) \
+static ssize_t show_##_attr(struct em_perf_domain *pd, char *buf) \
+{ \
+ ssize_t cnt = 0; \
+ int i; \
+ for (i = 0; i < pd->nr_cap_states; i++) { \
+ if (cnt >= (ssize_t) (PAGE_SIZE / sizeof(char) \
+ - (EM_ATTR_LEN + 2))) \
+ goto out; \
+ cnt += scnprintf([cnt], EM_ATTR_LEN + 1, "%lu ", \
+pd->table[i]._attr); \
+ } \
+out: \
+ cnt += sprintf([cnt], "\n"); \
+ return cnt; \
+}
+
+show_table_attr(power);
+show_table_attr(frequency);
+show_table_attr(cost);
+
+static ssize_t show_cpus(struct em_perf_domain *pd, char *buf)
+{
+ return sprintf(buf, "%*pbl\n", cpumask_pr_args(to_cpumask(pd->cpus)));
+}
+
+#define pd_attr(_name) em_pd_##_name##_attr
+#define define_pd_attr(_name) static struct em_pd_attr pd_attr(_name) = \
+ __ATTR(_name, 0444, show_##_name, NULL)
+
+define_pd_attr(power);
+define_pd_attr(frequency);
+define_pd_attr(cost);
+define_pd_attr(cpus);
+
+static struct attribute *em_pd_default_attrs[] = {
+ _attr(power).attr,
+ _attr(frequency).attr,
+ _attr(cost).attr,
+ _attr(cpus).attr,
+ NULL
+};
+
+#define to_pd(k) container_of(k, struct em_perf_domain, kobj)
+#define to_pd_attr(a) container_of(a, struct em_pd_attr, attr)
+
+static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
+{
+ struct em_perf_domain *pd = to_pd(kobj);
+ struct em_pd_attr *pd_attr = to_pd_attr(attr);
+ ssize_t ret;
+
+ ret = pd_attr->show(pd, buf);
+
+ return ret;
+}
+
+static const struct sysfs_ops em_pd_sysfs_ops = {
+ .show = show,
+};
+
+static struct kobj_type ktype_em_pd = {
+ .sysfs_ops = _pd_sysfs_ops,
+ .default_attrs = em_pd_default_attrs,
+};
+
static struct em_perf_domain *em_create_pd(cpumask_t *span, int nr_states,
struct em_data_callback *cb)
{
@@ -102,6 +178,11 @@ static struct em_perf_domain *em_create_pd(cpumask_t
*span, int nr_states,
pd->nr_cap_states = nr_states;
cpumask_copy(to_cpumask(pd->cpus), span);
+ ret = kobject_init_and_add(>kobj, _em_pd, em_kobject,
+ "pd%u", cpu);
+ if (ret)
+ pr_err("pd%d: failed kobject_init_and_add(): %d\n", cpu, ret);
+
return pd;
free_cs_table:
@@ -155,6 +236,15 @@ int em_register_perf_domain(cpumask_t *span, unsigned int
nr_states,
*/
mutex_lock(_pd_mutex);
+ if (!em_kobject) {
+ em_kobject = kobject_create_and_add("energy_model",
+ _subsys.dev_root->kobj);
+ if (!em_kobject) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+ }
+
