linux-kernel - Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch

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] [day] [month] [year] [list]

Message-ID: <20170530145946.v3n44cx56flhbi5l@e106622-lin>
Date:   Tue, 30 May 2017 15:59:46 +0100
From:   Juri Lelli <juri.lelli@....com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Dietmar Eggemann <dietmar.eggemann@....com>,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        peterz@...radead.org, vincent.guittot@...aro.org,
        robh+dt@...nel.org, mark.rutland@....com, linux@....linux.org.uk,
        sudeep.holla@....com, lorenzo.pieralisi@....com,
        catalin.marinas@....com, will.deacon@....com,
        morten.rasmussen@....com, broonie@...nel.org
Subject: Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers
 arch_topology interfaces

On 29/05/17 12:46, Dietmar Eggemann wrote:
> On 05/29/2017 11:58 AM, Greg KH wrote:
> > On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
> > > Hi Greg,
> > > 
> > > On 05/26/2017 08:36 PM, Greg KH wrote:
> > > > On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> > > > > Hi,
> > > > > 
> > > > > On 25/05/17 15:18, Greg KH wrote:
> > > > > > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > > 
> > > [...]
> > > 
> > > > > > But this is all really topology stuff, right?  Why use "capacity" at
> > > > > > all:
> > > > > > 	topology_normalize_cpu()
> > > > > > 	topology_parse_cpu()
> > > > > > 	topology_scale_cpu()
> > > > > > 	topology_set_scale()
> > > > > > ?
> > > > > > 
> > > > > > It's always best to put the "subsystem" name first, we have a bad
> > > > > > history of getting this wrong in the past by putting the verb first, not
> > > > > > the noun.
> > > > > > 
> > > > > 

[...]

> > > > 
> > > > Oh, and drop "capacity" please :)
> > > 

[...]

> > 
> > I think that if you are creating an api that the scheduler will use, you
> > need to ask the scheduler maintainers/developers what they want to see
> > here, as that would be up to them, not me...
> 
> The scheduler API exists already. It is arch_scale_cpu_capacity() and
> arch_scale_freq_capacity() in kernel/sched/sched.h. An arch is able to
> overwrite these two functions by defining them (since commit 8cd5601c5060
> and dfbca41f3479):
> 
> #define arch_scale_cpu_capacity 'arch implementation of capacity scaling by
> micro-architectural + max frequency (OPPmax)'
> 
> #define arch_scale_freq_capacity 'arch implementation of capacity scaling by
> 'frequency ((OPPmin..OPPmax)'
> 
> There is no naming convention from the scheduler side on these functions
> though. They should just express what they're doing, scaling capacity by
> something.
> 

So, discussing this naming with Morten off-line we seemed actually to
agree that the following might adhere even better to what the functions
actually do:

 topology_parse_cpu_capacity() - it parses the raw capacity-dmips-mhz
 values from DT; so it seems OK to leave capacity in the name here

 topology_set_cpu_scale() - it sets the per_cpu cpu_scale variable; so
 this name seems saner that the one with "_capacity"

 topology_get_cpu_scale() - dual of the previous one

 topology_normalize_cpu_scale() - it normalizes cpu_scale variables
 across the system CPUs (and calls topology_set_cpu_scale() to set the
 normalized values)

Greg, does this approack look saner to you as well?
If yes, I'll send a new version of the set shortly.

And, as I already said on IRC, apologies for this naming fight. :)

Thanks,

- Juri