On Wednesday, August 23, 2017 6:04:42 PM CEST Prakash, Prashanth wrote:
> Hi Rafael,
>
> On 8/1/2017 11:34 AM, Sudeep Holla wrote:
> >
> > 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.
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
