Re: [PATCH v2 3/3] sched: clean-up struct sd_lb_stat

[Joonsoo Kim]

> > +	if (busiest->group_imb) {
> > +		busiest->sum_weighted_load =
> > +			min(busiest->sum_weighted_load, sds->sd_avg_load);
> 
> Right here we get confused as to why the total load is being compared
> against load per task (although you are changing it to load per task above).

Yes, you are right. I will add load_per_task to struct sg_lb_stats.


> > @@ -4771,12 +4763,13 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s
> >  	 * Be careful of negative numbers as they'll appear as very large values
> >  	 * with unsigned longs.
> >  	 */
> > -	max_pull = min(sds->max_load - sds->avg_load, load_above_capacity);
> > +	max_pull = min(busiest->avg_load - sds->sd_avg_load,
> > +						load_above_capacity);
> 
> This is ok, but readability wise max_load is much better. max_load
> signifies the maximum load per task on a group in this sd, and avg_load
> signifies the total load per task across the sd. You are checking if
> there is imbalance in the total load in the sd and try to even it out
> across the sd.  Here "busiest" does not convey this immediately.

You may be confused. max_load doesn't means the maximum load per task.
It means that the busiest group's load per cpu power. And here max doesn't
mean maximum load in this sd, instead, load of busiest sg. IMO,
busiest->avg_load convey proper meaning.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/