Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1462707 > unrolled thread

Re: [PATCH v3 05/13] sched: Introduce SD_ASYM_CPUCAPACITY sched_domain topology flag

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 05/13] sched: Introduce SD_ASYM_CPUCAPACITY  sched_domain topology flag Peter Zijlstra <peterz@infradead.org> - 2016-08-15 13:00 +0200
    Re: [PATCH v3 05/13] sched: Introduce SD_ASYM_CPUCAPACITY  sched_domain topology flag Morten Rasmussen <morten.rasmussen@arm.com> - 2016-08-15 13:50 +0200
    [tip:sched/core] sched/core: Clarify SD_flags comment tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2016-08-18 13:00 +0200

#1462707 — Re: [PATCH v3 05/13] sched: Introduce SD_ASYM_CPUCAPACITY sched_domain topology flag

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.

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 |		\

[toc] | [next] | [standalone]

#1462727

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

[toc] | [prev] | [next] | [standalone]

#1465131 — [tip:sched/core] sched/core: Clarify SD_flags comment

Commit-ID:  94f438c84e850570f28dd36588a0d7f73b991e44
Gitweb:     http://git.kernel.org/tip/94f438c84e850570f28dd36588a0d7f73b991e44
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 15 Aug 2016 12:54:59 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 11:26:52 +0200

sched/core: Clarify SD_flags comment

The SD_flags 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.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dietmar.eggemann@arm.com
Cc: freedom.tan@mediatek.com
Cc: keita.kobayashi.ym@renesas.com
Cc: mgalbraith@suse.de
Cc: sgurrappadi@nvidia.com
Cc: vincent.guittot@linaro.org
Cc: yuyang.du@intel.com
Link: http://lkml.kernel.org/r/20160815105459.GS6879@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3b6b23c..54fff81 100644
--- 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 |		\

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web