Messages in this thread Patch in this message | | | Date | Wed, 11 May 2011 16:56:34 +0900 | From | Hidetoshi Seto <> | Subject | Re: [patch 13/15] sched: add exports tracking cfs bandwidth control statistics |
| |
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.
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:
| |