linux-kernel - Re: [PATCHv4 2/2] powerpc: implement arch_scale_smt

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: <1266573672.1806.70.camel@laptop>
Date:	Fri, 19 Feb 2010 11:01:12 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Michael Neuling <mikey@...ling.org>
Cc:	Joel Schopp <jschopp@...tin.ibm.com>, Ingo Molnar <mingo@...e.hu>,
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	ego@...ibm.com
Subject: Re: [PATCHv4 2/2] powerpc: implement arch_scale_smt_power for
 Power7

On Fri, 2010-02-19 at 17:05 +1100, Michael Neuling wrote:

> >  include/linux/sched.h |    2 +-
> >  kernel/sched_fair.c   |   61 +++++++++++++++++++++++++++++++++++++++++++++--
> -
> >  2 files changed, 58 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 0eef87b..42fa5c6 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -849,7 +849,7 @@ enum cpu_idle_type {
> >  #define SD_POWERSAVINGS_BALANCE	0x0100	/* Balance for power savings */
> >  #define SD_SHARE_PKG_RESOURCES	0x0200	/* Domain members share cpu pkg
>  resources */
> >  #define SD_SERIALIZE		0x0400	/* Only a single load balancing instanc
> e */
> > -
> > +#define SD_ASYM_PACKING		0x0800
> 
> Would we eventually add this to SD_SIBLING_INIT in a arch specific hook,
> or is this ok to add it generically?

I'd think we'd want to keep that limited to architectures that actually
need it.

>  
> > +static int update_sd_pick_busiest(struct sched_domain *sd,
> > +	       			  struct sd_lb_stats *sds,
> > +				  struct sched_group *sg,
> > +			  	  struct sg_lb_stats *sgs)
> > +{
> > +	if (sgs->sum_nr_running > sgs->group_capacity)
> > +		return 1;
> > +
> > +	if (sgs->group_imb)
> > +		return 1;
> > +
> > +	if ((sd->flags & SD_ASYM_PACKING) && sgs->sum_nr_running) {
> > +		if (!sds->busiest)
> > +			return 1;
> > +
> > +		if (group_first_cpu(sds->busiest) < group_first_cpu(group))
> 
> "group" => "sg" here? (I get a compile error otherwise)

Oh, quite ;-)

> > +static int check_asym_packing(struct sched_domain *sd,
> > +				    struct sd_lb_stats *sds, 
> > +				    int cpu, unsigned long *imbalance)
> > +{
> > +	int i, cpu, busiest_cpu;
> 
> Redefining cpu here.  Looks like the cpu parameter is not really needed?

Seems that way indeed, I went back and forth a few times on the actual
implementation of this function (which started out live as a copy of
check_power_save_busiest_group), its amazing there were only these two
compile glitches ;-)

> > +
> > +	if (!(sd->flags & SD_ASYM_PACKING))
> > +		return 0;
> > +
> > +	if (!sds->busiest)
> > +		return 0;
> > +
> > +	i = 0;
> > +	busiest_cpu = group_first_cpu(sds->busiest);
> > +	for_each_cpu(cpu, sched_domain_span(sd)) {
> > +		i++;
> > +		if (cpu == busiest_cpu)
> > +			break;
> > +	}
> > +
> > +	if (sds->total_nr_running > i)
> > +		return 0;
> > +
> > +	*imbalance = sds->max_load;
> > +	return 1;
> > +}


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