linux-kernel - Re: [RFC PATCHC 1/3] cpuidle: encapsulate power info in a separate structure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.11.1403281950450.1571@knanqh.ubzr>
Date:	Fri, 28 Mar 2014 20:00:13 -0400 (EDT)
From:	Nicolas Pitre <nicolas.pitre@...aro.org>
To:	Daniel Lezcano <daniel.lezcano@...aro.org>
cc:	LKML <linux-kernel@...r.kernel.org>, mingo@...e.hu,
	Peter Zijlstra <peterz@...radead.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Alex Shi <alex.shi@...aro.org>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Morten Rasmussen <morten.rasmussen@....com>
Subject: Re: [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 
terms.

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


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/