ACPI / Processor: add sysfs support for low power idle
Subsystems (2)
PATCH v2 1/1 ACPI / Processor: add sysfs support for low power idle
Add support to expose idle statistics maintained by platform to userspace via sysfs in addition to other data of interest from each LPI(Low Power Idle) state.
LPI described in section 8.4.4 of ACPI spec 6.1 provides different methods to obtain idle statistics maintained by the platform. These show a granular view of how each of the LPI state is being used at different level of hierarchy. sysfs data is exposed at each level in the hierarchy by creating a directory named 'lpi' at each level and the LPI state information is presented under it. Below is the representation of LPI information at one such level in the hierarchy
.../ACPI00XX: XX/lpi
|-> state0 | |-> desc | |-> time | |-> usage | |-> latency | |-> min_residency | <<more states>>
ACPI00XX can be ACPI0007(processor) or ACPI0010(processor container)
stateX contains information related to a specific LPI state defined in the LPI ACPI tables.
---
v2
* Add Documentation/ABI/testing/sysfs-acpi-lpi
v1
* Drop flags, arch_flags and summary_stats field (Sudeep)
* Create sysfs entries after we know that LPI will be used (Sudeep & Rafael)
Documentation/ABI/testing/sysfs-acpi-lpi | 47 +++++ drivers/acpi/processor_idle.c | 343 ++++++++++++++++++++++++++++++- include/acpi/processor.h | 14 ++ 3 files changed, 402 insertions(+), 2 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-acpi-lpi
Documentation/ABI/testing/sysfs-acpi-lpi new file +47 -0
@@ -0,0 +1,47 @@ +What: LPI sysfs interface for processor devices and containers +Date: 13-July-2017 +KernelVersion: v4.13 +Contact: [email protected] +Description: LPI(Low Power Idle) described in section 8.4.4 of ACPI spec 6.1 + provides different methods to obtain idle statistics maintained + by the platform. These show a granular view of how each of the + LPI state is being used at different level of processor hierarchy. + sysfs data is exposed at each level in the hierarchy by creating + a directory named 'lpi' at each level and the LPI state + information is presented under it. Below is the representation + of LPI information at one such level in the hierarchy. + + .../ACPI00XX: YY/lpi + |-> state0 + | |-> desc + | |-> time + | |-> usage + | |-> latency + | |-> min_residency + | + |-> state1 + | |-> desc + | |-> time + | |-> usage + | |-> latency + | |-> min_residency + | + <<more states>> + + ACPI00XX can be ACPI0007(processor device) or ACPI0010(processor + container) and the sysfs nodes for these ACPI devices can be + found under /sys/devices/LNXSYSTM:00 + + stateX contains information related to a specific local LPI + state defined in the LPI ACPI tables under a owning hierarchy + node(ACPI0007 or ACPI0010). + + desc - Description/Name for the local LPI state + latency - Worst case wake up latency(in micro seconds) for the + owning hierarchy node to exit from this local idle state + min_residency - Time in microseconds after which a state becomes + more energy effecient than a shallower state + time - Total time spent(in micro seconds) by the owning + hierarchy node in this local idle state + usage - Number of times the owning hierarchy node entered this + local power state
drivers/acpi/processor_idle.c +341 -2
384 lines not on this page · raw message
include/acpi/processor.h +14 -0
@@ -79,6 +79,19 @@ struct acpi_lpi_state {u8 index;u8 entry_method;char desc[ACPI_CX_DESC_LEN ]; +struct acpi_generic_address res_cntr; +struct acpi_generic_address usage_cntr; +}; + +struct acpi_lpi_sysfs_state { +struct acpi_lpi_state lpi_state; +struct kobject kobj; +}; + +struct acpi_lpi_sysfs_data { +u8 state_count; +struct kobject kobj; +struct acpi_lpi_sysfs_state *sysfs_states; };struct acpi_processor_power {
@@ -88,6 +101,7 @@ struct acpi_processor_power {struct acpi_lpi_state lpi_states[ACPI_PROCESSOR_MAX_POWER ]; };int timer_broadcast_on_state; +struct acpi_lpi_sysfs_data *lpi_sysfs_data; };/* Performance Management */
Signature · 3 lines
Qualcomm Datacenter Technologies on behalf of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Documentation/ABI/testing/sysfs-acpi-lpi
Quoted 46 lines · Prashanth Prakash
Add support to expose idle statistics maintained by platform to userspace via sysfs in addition to other data of interest from each LPI(Low Power Idle) state.
LPI described in section 8.4.4 of ACPI spec 6.1 provides different methods to obtain idle statistics maintained by the platform. These show a granular view of how each of the LPI state is being used at different level of hierarchy. sysfs data is exposed at each level in the hierarchy by creating a directory named 'lpi' at each level and the LPI state information is presented under it. Below is the representation of LPI information at one such level in the hierarchy
.../ACPI00XX: XX/lpi
|-> state0 | |-> desc | |-> time | |-> usage | |-> latency | |-> min_residency | <<more states>>ACPI00XX can be ACPI0007(processor) or ACPI0010(processor container)
stateX contains information related to a specific LPI state defined in the LPI ACPI tables.
Signed-off-by Prashanth Prakash <[email protected]>---
v2
* AddDocumentation/ABI/testing/sysfs-acpi-lpi
v1
* Drop flags,arch_flagsandsummary_statsfield (Sudeep)
* Create sysfs entries after we know that LPI will be used (Sudeep & Rafael)Documentation/ABI/testing/sysfs-acpi-lpi | 47 +++++ drivers/acpi/processor_idle.c | 343 ++++++++++++++++++++++++++++++- include/acpi/processor.h | 14 ++ 3 files changed, 402 insertions(+), 2 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-acpi-lpiDocumentation/ABI/testing/sysfs-acpi-lpi new file +0
-0
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
But from Documentation/ABI/README, I see
testing/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...
which makes me worry. Since the use for this is purely for debug or optimization purposes, I still prefer simple single file debugfs entry. I still can't digest the fact that reading single file is time consuming as we are not using this interface at runtime IIUC. i.e. statistic are collected and analyzed offline.
But if Rafael is fine with this, that's fine.
Signature · 2 lines
Regards, Sudeep
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or optimization purposes, I still prefer simple single file debugfs entry. I still can't digest the fact that reading single file is time consuming as we are not using this interface at runtime IIUC. i.e. statistic are collected and analyzed offline.
These fields has the same utility/use-cases as the usage & time fields in cpuidle sysfs, but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the summary_stats file was removed and went with one value per file.
--
Thanks,
Prashanth
Quoted 11 lines · Prashanth
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.
Fair enough.
Quoted 10 lines
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...
Quoted 5 lines
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or
optimization purposes, I still prefer simple single file debugfs entry.
I still can't digest the fact that reading single file is time consuming
as we are not using this interface at runtime IIUC. i.e. statistic are
collected and analyzed offline.>> These fields has the same utility/use-cases as the usage & time
fields in cpuidle sysfs,
Quoted 3 lines
but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
I completely agree with that and that's not the argument.
Quoted 6 lines
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the
summary_statsfile was removed and went with one value per file.
You are contradicting yourself above :). You say the argument you made is weak :) but still went ahead and dropped single debugfs file vs the standard per entry sysfs file which is an ABI.
Since we already have CPUIdle sysfs which is an ABI, I am really not sure if we need another set of ABI files which are used only for debug and optimization purposes. Why is single debugfs file not sufficient ?
As hardware evolves, most of the platforms can't provide these information accurately. So if we are trying to address a problem which is short-lived and on very small class of platforms, I would avoid creating a new ABI for it. That's my main argument against this interface instead go with debugfs entry. That's my opinion though.
Signature · 2 lines
Regards, Sudeep
Quoted 42 lines · Sudeep Holla
Quoted 11 lines · Prashanth
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.Fair enough.
Quoted 10 lines
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...Quoted 5 lines
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or
optimization purposes, I still prefer simple single file debugfs entry.
I still can't digest the fact that reading single file is time consuming
as we are not using this interface at runtime IIUC. i.e. statistic are
collected and analyzed offline.>> These fields has the same utility/use-cases as the usage & timefields in cpuidle sysfs,
Quoted 3 lines
but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
I completely agree with that and that's not the argument.
Quoted 6 lines
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the
summary_statsfile was removed and went with one value per file.
Quoted 3 lines
You are contradicting yourself above :). You say the argument you made is weak :) but still went ahead and dropped single debugfs file vs the standard per entry sysfs file which is an ABI.
To clarify, the first RFC patch had a sysfs entry called summary_stats which provided all the stats for a specific node in hierarchy via a single file. The argument for having such a single file was weak. :) There was never a debugfs file to be dropped in first place.
Quoted 4 lines
Since we already have CPUIdle sysfs which is an ABI, I am really not sure if we need another set of ABI files which are used only for debug and optimization purposes. Why is single debugfs file not sufficient ?
The consumers for this data are not all kernel developers. We will have other engineers looking at this data for power/performance optimizations and would be nice to give them a consistent interface.
Quoted 5 lines
As hardware evolves, most of the platforms can't provide these information accurately. So if we are trying to address a problem which is short-lived and on very small class of platforms, I would avoid creating a new ABI for it. That's my main argument against this interface instead go with debugfs entry. That's my opinion though.
I would like to think of it in terms of ACPI spec rather than a subset of platforms. If it is part of spec there should be no reason to speculate on which platform may or may not implement it. We implement to the spec and ideally platform designers should ideally do the same.
Since sysfs vs debugfs is quite debatable :) , I will wait for Rafael's inputs.
--
Thanks,
Prashanth
Quoted 44 lines · Prashanth
Quoted 42 lines · Sudeep Holla
Quoted 11 lines · Prashanth
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.Fair enough.
Quoted 10 lines
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...Quoted 5 lines
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or
optimization purposes, I still prefer simple single file debugfs entry.
I still can't digest the fact that reading single file is time consuming
as we are not using this interface at runtime IIUC. i.e. statistic are
collected and analyzed offline.>> These fields has the same utility/use-cases as the usage & timefields in cpuidle sysfs,
Quoted 3 lines
but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
I completely agree with that and that's not the argument.
Quoted 6 lines
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the
summary_statsfile was removed and went with one value per file.
Quoted 9 lines
Quoted 3 lines
You are contradicting yourself above :). You say the argument you made is weak :) but still went ahead and dropped single debugfs file vs the standard per entry sysfs file which is an ABI.
To clarify, the first RFC patch had a sysfs entry called
summary_statswhich provided all the stats for a specific node in hierarchy via a single file. The argument for having such a single file was weak. :) There was never a debugfs file to be dropped in first place.
You are right, it was me who keep suggesting debugfs file as a replacement for your summary_stats sysfs file. Sorry for the confusion but I still insist debugfs :)
Quoted 8 lines
Quoted 4 lines
Since we already have CPUIdle sysfs which is an ABI, I am really not sure if we need another set of ABI files which are used only for debug and optimization purposes. Why is single debugfs file not sufficient ?
The consumers for this data are not all kernel developers. We will have other engineers looking at this data for power/performance optimizations and would be nice to give them a consistent interface.
Yes that's the case with any sysfs file users and that's why we can't break ABI once we expose.
Quoted 10 lines
Quoted 5 lines
As hardware evolves, most of the platforms can't provide these information accurately. So if we are trying to address a problem which is short-lived and on very small class of platforms, I would avoid creating a new ABI for it. That's my main argument against this interface instead go with debugfs entry. That's my opinion though.
I would like to think of it in terms of ACPI spec rather than a subset of platforms. If it is part of spec there should be no reason to speculate on which platform may or may not implement it. We implement to the spec and ideally platform designers should ideally do the same. >
Since sysfs vs debugfs is quite debatable :) , I will wait for Rafael's inputs.
Sure.
Signature · 2 lines
Regards, Sudeep
Hi Rafael,
Quoted 60 lines · Sudeep Holla
Quoted 42 lines · Prashanth
Quoted 41 lines · Sudeep Holla
Quoted 11 lines · Prashanth
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.Fair enough.
Quoted 10 lines
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...Quoted 5 lines
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or
optimization purposes, I still prefer simple single file debugfs entry.
I still can't digest the fact that reading single file is time consuming
as we are not using this interface at runtime IIUC. i.e. statistic are
collected and analyzed offline.>> These fields has the same utility/use-cases as the usage & timefields in cpuidle sysfs,
Quoted 3 lines
but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
I completely agree with that and that's not the argument.
Quoted 6 lines
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the
summary_statsfile was removed and went with one value per file.Quoted 8 lines
Quoted 3 lines
You are contradicting yourself above :). You say the argument you made is weak :) but still went ahead and dropped single debugfs file vs the standard per entry sysfs file which is an ABI.
To clarify, the first RFC patch had a sysfs entry called
summary_statswhich provided all the stats for a specific node in hierarchy via a single file. The argument for having such a single file was weak. :) There was never a debugfs file to be dropped in first place.You are right, it was me who keep suggesting debugfs file as a replacement for your
summary_statssysfs file. Sorry for the confusion but I still insist debugfs :)Quoted 4 lines
Quoted 4 lines
Since we already have CPUIdle sysfs which is an ABI, I am really not sure if we need another set of ABI files which are used only for debug and optimization purposes. Why is single debugfs file not sufficient ?
Quoted 16 lines
Quoted 3 lines
The consumers for this data are not all kernel developers. We will have other engineers looking at this data for power/performance optimizations and would be nice to give them a consistent interface.
Yes that's the case with any sysfs file users and that's why we can't break ABI once we expose.
Quoted 10 lines
Quoted 5 lines
As hardware evolves, most of the platforms can't provide these information accurately. So if we are trying to address a problem which is short-lived and on very small class of platforms, I would avoid creating a new ABI for it. That's my main argument against this interface instead go with debugfs entry. That's my opinion though.
I would like to think of it in terms of ACPI spec rather than a subset of platforms. If it is part of spec there should be no reason to speculate on which platform may or may not implement it. We implement to the spec and ideally platform designers should ideally do the same. >
Since sysfs vs debugfs is quite debatable :) , I will wait for Rafael's inputs.
Any inputs on this?
--
Thanks,
Prashanth
Quoted 74 lines · Prashanth
Hi Rafael,
Quoted 71 lines · Sudeep Holla
Quoted 42 lines · Prashanth
Quoted 41 lines · Sudeep Holla
Quoted 11 lines · Prashanth
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.Fair enough.
Quoted 10 lines
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...Quoted 5 lines
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or
optimization purposes, I still prefer simple single file debugfs entry.
I still can't digest the fact that reading single file is time consuming
as we are not using this interface at runtime IIUC. i.e. statistic are
collected and analyzed offline.>> These fields has the same utility/use-cases as the usage & timefields in cpuidle sysfs,
Quoted 3 lines
but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
I completely agree with that and that's not the argument.
Quoted 6 lines
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the
summary_statsfile was removed and went with one value per file.Quoted 8 lines
Quoted 3 lines
You are contradicting yourself above :). You say the argument you made is weak :) but still went ahead and dropped single debugfs file vs the standard per entry sysfs file which is an ABI.
To clarify, the first RFC patch had a sysfs entry called
summary_statswhich provided all the stats for a specific node in hierarchy via a single file. The argument for having such a single file was weak. :) There was never a debugfs file to be dropped in first place.You are right, it was me who keep suggesting debugfs file as a replacement for your
summary_statssysfs file. Sorry for the confusion but I still insist debugfs :)Quoted 7 lines
Quoted 4 lines
Since we already have CPUIdle sysfs which is an ABI, I am really not sure if we need another set of ABI files which are used only for debug and optimization purposes. Why is single debugfs file not sufficient ?
The consumers for this data are not all kernel developers. We will have other engineers looking at this data for power/performance optimizations and would be nice to give them a consistent interface.
Yes that's the case with any sysfs file users and that's why we can't break ABI once we expose.
Quoted 5 lines
Quoted 5 lines
As hardware evolves, most of the platforms can't provide these information accurately. So if we are trying to address a problem which is short-lived and on very small class of platforms, I would avoid creating a new ABI for it. That's my main argument against this interface instead go with debugfs entry. That's my opinion though.
Quoted 6 lines
Quoted 5 lines
Quoted 5 lines
I would like to think of it in terms of ACPI spec rather than a subset of platforms. If it is part of spec there should be no reason to speculate on which platform may or may not implement it. We implement to the spec and ideally platform designers should ideally do the same. >
Since sysfs vs debugfs is quite debatable :) , I will wait for Rafael's inputs.Any inputs on this?
Sorry for the slow response, but I'm still unconvinced about this.
I generally don't like residency ("time" in your terminology, which is quite confusing BTW) / usage information in sysfs under ACPI device nodes, as it belongs to cpuidle rather.
I know that it is easy to expose it this way, but can't we do better?
Also, I'd use file names following the spec language. min_residency is fine, but "latency" would better be called "wakeup_latency" and why use "desc" rather than "state_name"? Plust exposing "Enabled Parent State" might be useful.
Thanks,
Rafael
Quoted 79 lines · Rafael J. Wysocki
Quoted 78 lines · Prashanth
Hi Rafael,
Quoted 75 lines · Sudeep Holla
Quoted 42 lines · Prashanth
Quoted 41 lines · Sudeep Holla
Quoted 11 lines · Prashanth
Hi Sudeep,
Quoted 4 lines · Sudeep Holla
Sorry for the delay, I initial thought having this ABI under testing is fine as I really don't want any *real* user space programs to depend on this for various reasons stated in earlier threads, e.g. h/w auto promotable states, accuracy of the stats, ..etc
<sorry for repeating this part>
These fields are optional, so if there is no reliable way to keep track of stats, platform can choose not to expose it. If a platform is exposing inaccurate stats via this ACPI interface, it is breaking the spec.Fair enough.
Quoted 10 lines
Quoted 10 lines
But from
Documentation/ABI/README, I seetesting/
This directory documents interfaces that are felt to be stable, as the main development of this interface has been completed. The interface can be changed to add new features, but the current interface will not break by doing this, unless grave errors or security problems are found in them. User space programs can start to rely on these interfaces,...Quoted 5 lines
Quoted 5 lines
which makes me worry. Since the use for this is purely for debug or
optimization purposes, I still prefer simple single file debugfs entry.
I still can't digest the fact that reading single file is time consuming
as we are not using this interface at runtime IIUC. i.e. statistic are
collected and analyzed offline.>> These fields has the same utility/use-cases as the usage & timefields in cpuidle sysfs,
Quoted 3 lines
but provides more granularity - idle stats for different levels hierarchy and accurate idle stats for states that require platform co-ordination.
I completely agree with that and that's not the argument.
Quoted 6 lines
The argument for having a single sysfs file per node was that reading individual files might get expensive to get a snapshot(not the other way around). But, that argument was weak as we typically read these only in debug settings and not that often during runtime. So, the
summary_statsfile was removed and went with one value per file.Quoted 8 lines
Quoted 3 lines
You are contradicting yourself above :). You say the argument you made is weak :) but still went ahead and dropped single debugfs file vs the standard per entry sysfs file which is an ABI.
To clarify, the first RFC patch had a sysfs entry called
summary_statswhich provided all the stats for a specific node in hierarchy via a single file. The argument for having such a single file was weak. :) There was never a debugfs file to be dropped in first place.You are right, it was me who keep suggesting debugfs file as a replacement for your
summary_statssysfs file. Sorry for the confusion but I still insist debugfs :)Quoted 7 lines
Quoted 4 lines
Since we already have CPUIdle sysfs which is an ABI, I am really not sure if we need another set of ABI files which are used only for debug and optimization purposes. Why is single debugfs file not sufficient ?
The consumers for this data are not all kernel developers. We will have other engineers looking at this data for power/performance optimizations and would be nice to give them a consistent interface.
Yes that's the case with any sysfs file users and that's why we can't break ABI once we expose.
Quoted 10 lines
Quoted 5 lines
As hardware evolves, most of the platforms can't provide these information accurately. So if we are trying to address a problem which is short-lived and on very small class of platforms, I would avoid creating a new ABI for it. That's my main argument against this interface instead go with debugfs entry. That's my opinion though.
I would like to think of it in terms of ACPI spec rather than a subset of platforms. If it is part of spec there should be no reason to speculate on which platform may or may not implement it. We implement to the spec and ideally platform designers should ideally do the same. >
Since sysfs vs debugfs is quite debatable :) , I will wait for Rafael's inputs.
Quoted 8 lines
Quoted 1 line
Any inputs on this?
Sorry for the slow response, but I'm still unconvinced about this.
I generally don't like residency ("time" in your terminology, which is quite confusing BTW) / usage information in sysfs under ACPI device nodes, as it belongs to cpuidle rather.
I know that it is easy to expose it this way, but can't we do better?
Last time I went though the code, I didn't find a good way to expose this outside the ACPI hierarchy primarily because i couldn't find a good substitute to represent the processor hierarchy.
I will take a look at it again.
Quoted 4 lines
Also, I'd use file names following the spec language.
min_residencyis fine, but "latency" would better be called "wakeup_latency" and why use "desc" rather than "state_name"? Plust exposing "Enabled Parent State" might be useful.
I will switch the names to spec language on next version of this patch.
Thanks for your feedback!
--
Regards,
Prashanth