Messages in this thread | | | From | Paul Turner <> | Date | Wed, 11 May 2011 02:09:05 -0700 | Subject | Re: [patch 13/15] sched: add exports tracking cfs bandwidth control statistics |
| |
On Wed, May 11, 2011 at 12:56 AM, Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> wrote: > Oops, I found an issue here. > > (2011/05/03 18:28), Paul Turner wrote: >> @@ -1628,6 +1631,12 @@ retry: >> raw_spin_unlock(&cfs_b->lock); >> goto retry; >> } >> + >> + /* update throttled stats */ >> + cfs_b->nr_periods += overrun; >> + if (throttled) >> + cfs_b->nr_throttled += overrun; >> + >> cfs_b->runtime = runtime; >> cfs_b->idle = idle; >> out_unlock: > > Quoting from patch 09/15: > > + if (!throttled || quota == RUNTIME_INF) > + goto out; > + idle = 0; > + > +retry: > + runtime = distribute_cfs_runtime(cfs_b, runtime, runtime_expires); > + > + raw_spin_lock(&cfs_b->lock); > + /* new new bandwidth may have been set */ > + if (unlikely(runtime_expires != cfs_b->runtime_expires)) > + goto out_unlock; > + /* > + * make sure no-one was throttled while we were handing out the new > + * runtime. > + */ > + if (runtime > 0 && !list_empty(&cfs_b->throttled_cfs_rq)) { > + raw_spin_unlock(&cfs_b->lock); > + goto retry; > + } > + cfs_b->runtime = runtime; > + cfs_b->idle = idle; > +out_unlock: > + raw_spin_unlock(&cfs_b->lock); > +out: > > Since we skip distributing runtime (by "goto out") when !throttled, > the new block inserted by this patch is passed only when throttled. > So I see that nr_periods and nr_throttled look the same. > > Maybe we should move this block up like followings. >
Yes, makes sense, incorporated -- thanks!
> Thanks, > H.Seto > > --- > --- a/kernel/sched_fair.c > +++ b/kernel/sched_fair.c > @@ -1620,6 +1620,12 @@ static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun) > idle = cfs_b->idle; > cfs_b->idle = 1; > } > + > + /* update throttled stats */ > + cfs_b->nr_periods += overrun; > + if (throttled) > + cfs_b->nr_throttled += overrun; > + > raw_spin_unlock(&cfs_b->lock); > > if (!throttled || quota == RUNTIME_INF) > @@ -1642,11 +1648,6 @@ retry: > goto retry; > } > > - /* update throttled stats */ > - cfs_b->nr_periods += overrun; > - if (throttled) > - cfs_b->nr_throttled += overrun; > - > cfs_b->runtime = runtime; > cfs_b->idle = idle; > out_unlock: > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |