Re: [PATCH v2] ACPI / Processor: add sysfs support for low power idle

[Sudeep Holla <sudeep.holla@arm.com>]

On 01/08/17 18:15, Prakash, Prashanth wrote:
> 
> On 8/1/2017 3:18 AM, Sudeep Holla wrote:
>>
>> On 31/07/17 19:18, Prakash, Prashanth wrote:
>>> Hi Sudeep,
>>>
>>> On 7/31/2017 10:25 AM, Sudeep Holla wrote:
>>>> 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.
>>
>>>> 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.>> 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.
>>>
>> I completely agree with that and that's not the argument.
>>
>>> 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.
>>>
>> 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.
> 

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 :)

>> 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.

>> 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.

-- 
Regards,
Sudeep