linux-kernel - Re: [PATCH v3 05/13] sched: Introduce SD_ASYM_CPUCAPACITY sched

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: <20160815114308.GB3391@e105550-lin.cambridge.arm.com>
Date:	Mon, 15 Aug 2016 12:43:08 +0100
From:	Morten Rasmussen <morten.rasmussen@....com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	mingo@...hat.com, dietmar.eggemann@....com, yuyang.du@...el.com,
	vincent.guittot@...aro.org, mgalbraith@...e.de,
	sgurrappadi@...dia.com, freedom.tan@...iatek.com,
	keita.kobayashi.ym@...esas.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 05/13] sched: Introduce SD_ASYM_CPUCAPACITY
 sched_domain topology flag

On Mon, Aug 15, 2016 at 12:54:59PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 25, 2016 at 02:34:22PM +0100, Morten Rasmussen wrote:
> > @@ -6336,14 +6338,16 @@ static int sched_domains_curr_level;
> >   * SD_NUMA                - describes NUMA topologies
> >   * SD_SHARE_POWERDOMAIN   - describes shared power domain
> >   *
> > - * Odd one out:
> > + * Odd ones out:
> >   * SD_ASYM_PACKING        - describes SMT quirks
> > + * SD_ASYM_CPUCAPACITY    - describes mixed capacity topologies
> >   */
> 
> So I'm not sure the new CPUCAPACITY is 'odd'.
> 
> That said, the comment is very terse and doesn't explain why PACKING is
> odd.
> 
> IIRC the distinction is that the 'normal' ones only describe topology,
> while the ASYM_PACKING one also prescribes behaviour. It is odd in the
> way that it doesn't only describe things.
> 
> This ASYM_CPUCAPACITY otoh is purely descriptive, it doesn't prescribe
> how to deal with it.

I think I initially put it in as an 'odd' flag due to the somewhat
strange semantics in the previous versions, but now that it is fixed I
agree that it belongs together with purely descriptive flags.

> 
> Does something like so  clarify things?
> 
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6355,13 +6355,19 @@ static int sched_domains_curr_level;
>  /*
>   * SD_flags allowed in topology descriptions.
>   *
> - * SD_SHARE_CPUCAPACITY      - describes SMT topologies
> - * SD_SHARE_PKG_RESOURCES - describes shared caches
> - * SD_NUMA                - describes NUMA topologies
> - * SD_SHARE_POWERDOMAIN   - describes shared power domain
> + * These flags are purely descriptive of the topology and do not prescribe
> + * behaviour. Behaviour is artificial and mapped in the below sd_init()
> + * function:
>   *
> - * Odd one out:
> - * SD_ASYM_PACKING        - describes SMT quirks
> + *   SD_SHARE_CPUCAPACITY   - describes SMT topologies
> + *   SD_SHARE_PKG_RESOURCES - describes shared caches
> + *   SD_NUMA                - describes NUMA topologies
> + *   SD_SHARE_POWERDOMAIN   - describes shared power domain
> + *
> + * Odd one out, which beside describing the topology has a quirk also
> + * prescribes the desired behaviour that goes along with it:
> + *
> + *   SD_ASYM_PACKING        - describes SMT quirks
>   */
>  #define TOPOLOGY_SD_FLAGS		\
>  	(SD_SHARE_CPUCAPACITY |		\

I like it :)

Morten