2011-05-07 06:32:42

by Paul Turner

[permalink] [raw]
Subject: [patch 13/15] sched: add exports tracking cfs bandwidth control statistics

From: Nikhil Rao <[email protected]>

This change introduces statistics exports for the cpu sub-system, these are
added through the use of a stat file similar to that exported by other
subsystems.

The following exports are included:

nr_periods: number of periods in which execution occurred
nr_throttled: the number of periods above in which execution was throttle
throttled_time: cumulative wall-time that any cpus have been throttled for
this group

Signed-off-by: Nikhil Rao <[email protected]>
Signed-off-by: Paul Turner <[email protected]>
Signed-off-by: Bharata B Rao <[email protected]>
---
kernel/sched.c | 22 ++++++++++++++++++++++
kernel/sched_fair.c | 9 +++++++++
2 files changed, 31 insertions(+)

Index: tip/kernel/sched.c
===================================================================
--- tip.orig/kernel/sched.c
+++ tip/kernel/sched.c
@@ -260,6 +260,10 @@ struct cfs_bandwidth {
struct hrtimer period_timer;
struct list_head throttled_cfs_rq;

+ /* statistics */
+ int nr_periods, nr_throttled;
+ u64 throttled_time;
+
#endif
};

@@ -395,6 +399,7 @@ struct cfs_rq {
u64 runtime_expires;
s64 runtime_remaining;

+ u64 throttled_timestamp;
int throttled, throttle_count;
struct list_head throttled_list;
#endif
@@ -9517,6 +9522,19 @@ int sched_cfs_consistent_handler(struct

return ret;
}
+
+static int cpu_stats_show(struct cgroup *cgrp, struct cftype *cft,
+ struct cgroup_map_cb *cb)
+{
+ struct task_group *tg = cgroup_tg(cgrp);
+ struct cfs_bandwidth *cfs_b = tg_cfs_bandwidth(tg);
+
+ cb->fill(cb, "nr_periods", cfs_b->nr_periods);
+ cb->fill(cb, "nr_throttled", cfs_b->nr_throttled);
+ cb->fill(cb, "throttled_time", cfs_b->throttled_time);
+
+ return 0;
+}
#endif /* CONFIG_CFS_BANDWIDTH */
#endif /* CONFIG_FAIR_GROUP_SCHED */

@@ -9563,6 +9581,10 @@ static struct cftype cpu_files[] = {
.read_u64 = cpu_cfs_period_read_u64,
.write_u64 = cpu_cfs_period_write_u64,
},
+ {
+ .name = "stat",
+ .read_map = cpu_stats_show,
+ },
#endif
#ifdef CONFIG_RT_GROUP_SCHED
{
Index: tip/kernel/sched_fair.c
===================================================================
--- tip.orig/kernel/sched_fair.c
+++ tip/kernel/sched_fair.c
@@ -1459,6 +1459,7 @@ static void throttle_cfs_rq(struct cfs_r
rq->nr_running += task_delta;

cfs_rq->throttled = 1;
+ cfs_rq->throttled_timestamp = rq->clock;
raw_spin_lock(&cfs_b->lock);
list_add_tail_rcu(&cfs_rq->throttled_list, &cfs_b->throttled_cfs_rq);
raw_spin_unlock(&cfs_b->lock);
@@ -1514,8 +1515,10 @@ static void unthrottle_cfs_rq(struct cfs

cfs_rq->throttled = 0;
raw_spin_lock(&cfs_b->lock);
+ cfs_b->throttled_time += rq->clock - cfs_rq->throttled_timestamp;
list_del_rcu(&cfs_rq->throttled_list);
raw_spin_unlock(&cfs_b->lock);
+ cfs_rq->throttled_timestamp = 0;

update_rq_clock(rq);
/* don't include throttled window for load statistics */
@@ -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:


2011-05-10 07:28:08

by Hidetoshi Seto

[permalink] [raw]
Subject: Re: [patch 13/15] sched: add exports tracking cfs bandwidth control statistics

(2011/05/03 18:28), Paul Turner wrote:
> From: Nikhil Rao <[email protected]>
>
> This change introduces statistics exports for the cpu sub-system, these are
> added through the use of a stat file similar to that exported by other
> subsystems.
>
> The following exports are included:
>
> nr_periods: number of periods in which execution occurred
> nr_throttled: the number of periods above in which execution was throttle
> throttled_time: cumulative wall-time that any cpus have been throttled for
> this group
>
> Signed-off-by: Nikhil Rao <[email protected]>
> Signed-off-by: Paul Turner <[email protected]>
> Signed-off-by: Bharata B Rao <[email protected]>
> ---
> kernel/sched.c | 22 ++++++++++++++++++++++
> kernel/sched_fair.c | 9 +++++++++
> 2 files changed, 31 insertions(+)
>
> Index: tip/kernel/sched.c
> ===================================================================
> --- tip.orig/kernel/sched.c
> +++ tip/kernel/sched.c
> @@ -260,6 +260,10 @@ struct cfs_bandwidth {
> struct hrtimer period_timer;
> struct list_head throttled_cfs_rq;
>
> + /* statistics */
> + int nr_periods, nr_throttled;
> + u64 throttled_time;
> +
> #endif
> };
>

Nit: blank line?

Reviewed-by: Hidetoshi Seto <[email protected]>

Thanks,
H.Seto

2011-05-11 16:46:31

by Hidetoshi Seto

[permalink] [raw]
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:

2011-05-11 16:14:18

by Paul Turner

[permalink] [raw]
Subject: Re: [patch 13/15] sched: add exports tracking cfs bandwidth control statistics

On Wed, May 11, 2011 at 12:56 AM, Hidetoshi Seto
<[email protected]> 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:
>
>
>