On Wed, 2010-01-20 at 14:04 -0600, Joel Schopp wrote:
> On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
> there is performance benefit to idling the higher numbered threads in
> the core.  
> 
> This patch implements arch_scale_smt_power to dynamically update smt
> thread power in these idle cases in order to prefer threads 0,1 over
> threads 2,3 within a core.
> 
> Signed-off-by: Joel Schopp <jsch...@austin.ibm.com>

So I'll leave Peter deal with the scheduler aspects and will focus on
details :-)

> ---
> Index: linux-2.6.git/arch/powerpc/kernel/smp.c
> ===================================================================
> --- linux-2.6.git.orig/arch/powerpc/kernel/smp.c
> +++ linux-2.6.git/arch/powerpc/kernel/smp.c
> @@ -617,3 +617,44 @@ void __cpu_die(unsigned int cpu)
>               smp_ops->cpu_die(cpu);
>  }
>  #endif
> +
> +static inline int thread_in_smt4core(int x)
> +{
> +  return x % 4;
> +}

Needs a whitespace here though I don't really like the above. Any reason
why you can't use the existing cpu_thread_in_core() ?

> +unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
> +{
> +  int cpu2;
> +  int idle_count = 0;
> +
> +  struct cpumask *cpu_map = sched_domain_span(sd);
> +
> +     unsigned long weight = cpumask_weight(cpu_map);
> +     unsigned long smt_gain = sd->smt_gain;

More whitespace damage above.

> +     if (cpu_has_feature(CPU_FTRS_POWER7) && weight == 4) {
> +             for_each_cpu(cpu2, cpu_map) {
> +                     if (idle_cpu(cpu2))
> +                             idle_count++;
> +             }

I'm not 100% sure about the use of the CPU feature above. First I wonder
if the right approach is to instead do something like

        if (!cpu_has_feature(...) !! weigth < 4)
                return default_scale_smt_power(sd, cpu);

Though we may be better off using a ppc_md. hook here to avoid
calculating the weight etc... on processors that don't need any
of that.

I also dislike your naming. I would suggest you change cpu_map to
sibling_map() and cpu2 to sibling (or just c). One thing I wonder is how
sure we are that sched_domain_span() is always going to give us the
threads btw ? If we introduce another sched domain level for NUMA
purposes can't we get confused ?

Also, how hot is this code path ?

> +             /* the following section attempts to tweak cpu power based
> +              * on current idleness of the threads dynamically at runtime
> +              */
> +             if (idle_count == 2 || idle_count == 3 || idle_count == 4) {

                if (idle_count > 1) ? :-)

> +                     if (thread_in_smt4core(cpu) == 0 ||
> +                         thread_in_smt4core(cpu) == 1) {

                        int thread = cpu_thread_in_core(cpu);
                        if (thread < 2)
                                ...

> +                             /* add 75 % to thread power */
> +                             smt_gain += (smt_gain >> 1) + (smt_gain >> 2);
> +                     } else {
> +                              /* subtract 75 % to thread power */
> +                             smt_gain = smt_gain >> 2;
> +                     }
> +             }
> +     }
> +     /* default smt gain is 1178, weight is # of SMT threads */
> +     smt_gain /= weight;
> +
> +     return smt_gain;

Cheers,
Ben.

> +}
> Index: linux-2.6.git/kernel/sched_features.h
> ===================================================================
> --- linux-2.6.git.orig/kernel/sched_features.h
> +++ linux-2.6.git/kernel/sched_features.h
> @@ -107,7 +107,7 @@ SCHED_FEAT(CACHE_HOT_BUDDY, 1)
>  /*
>   * Use arch dependent cpu power functions
>   */
> -SCHED_FEAT(ARCH_POWER, 0)
> +SCHED_FEAT(ARCH_POWER, 1)
>  
>  SCHED_FEAT(HRTICK, 0)
>  SCHED_FEAT(DOUBLE_TICK, 0)
> 


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to