lkml.org 
[lkml]   [2013]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 03/10] sched: Clean-up struct sd_lb_stat
On Sat, Aug 24, 2013 at 03:09:38AM -0700, Paul Turner wrote:
> On Mon, Aug 19, 2013 at 9:01 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > + struct sg_lb_stats *this, *busiest;
>
> "this" is a little confusing to read; mainly because elsewhere we've
> tied this to "this cpu" whereas the local sched group is arger. (Not
> to mention the obvious OOP-land overloading of "this->".)
>
> Perhaps %s/this/local/ for sg_lb_stat references? Including this_stat
> -> local_stat on sd_lb_stats?

fair enough, I'll edit the thing to be local.

> > @@ -4952,15 +4950,16 @@ find_busiest_group(struct lb_env *env)
> > * there is no imbalance between this and busiest group
> > * wrt to idle cpu's, it is balanced.
> > */
> > - if ((sds.this_idle_cpus <= sds.busiest_idle_cpus + 1) &&
> > - sds.busiest_nr_running <= sds.busiest_group_weight)
> > + if ((this->idle_cpus <= busiest->idle_cpus + 1) &&
> > + busiest->sum_nr_running <= busiest->group_weight)
>
> While we're improving readability: idle_cpus < busiest->idle_cpus ?

Right, took that.

> This check has always been a little oddly asymmetric in that:
> group_weight - sum_nr_running <= idle_cpus
>
> This allows the case where our group has pulled lots of work to one of
> its cpus, but not yet spread that out, but still keep trying to
> balance because idle_cpus is high.
>
> This is more food for thought since this patch is not changing functionality.

Right, I saw the same and made a note to look at it later. I suppose
later never happens though :/




\
 
 \ /
  Last update: 2013-08-26 14:21    [W:0.362 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site