[lkml]   [2014]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFC PATCHC 1/3] cpuidle: encapsulate power info in a separate structure
On Fri, 28 Mar 2014, Daniel Lezcano wrote:

> Hi Nicolas,
> thanks for reviewing the patchset.
> On 03/28/2014 07:17 PM, Nicolas Pitre wrote:
> > On Fri, 28 Mar 2014, Daniel Lezcano wrote:
> >
> >> The scheduler needs some information from cpuidle to know the timing for a
> >> specific idle state a cpu is.
> >>
> >> This patch creates a separate structure to group the cpuidle power info in
> >> order to share it with the scheduler. It improves the encapsulation of the
> >> code.
> >
> > Having cpuidle_power as a structure name, or worse, 'power' as a struct
> > member, is a really bad choice.
> Yes, I was asking myself if this name was a good choice or not. I
> assumed 'power' could have been a good name because 'target_residency'
> is a time conversion of the power needed to enter this state.

Still, that's something the casual reviewer might not know.

And we ought to be careful when talking about power as well. By
definition, power means energy transferred per unit of time. Sometimes
we tend to say 'power' when we actually mean 'energy'. With more "power
aware" work going into the scheduler, it is better to disambiguate those

> > Amongst the fields this struct
> > contains, only 1 out of 3 is about power. The word "power" is already
> > abused quite significantly to mean too many different things already.
> >
> > I'd suggest something inspired by your own patch log message i.e.
> > 'struct cpuidle_info' instead, and use 'info' as a field name within
> > struct cpuidle_state. Having 'params" instead of "info" could be a good
> > alternative too, although slightly longer.
> Hmm 'info' or 'param' sound too vague. What about:
> cpuidle_attr
> or
> cpuidle_property

As you wish. As long as it isn't 'power'.


 \ /
  Last update: 2014-03-29 01:21    [W:0.052 / U:4.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site