* [PATCH v2 1/2] sched: don't account throttle time for empty groups @ 2023-06-12 23:27 Josh Don 2023-06-12 23:27 ` [PATCH v2 2/2] sched: add throttled time stat for throttled children Josh Don 2023-06-15 11:38 ` [tip: sched/core] sched: Don't account throttle time for empty groups tip-bot2 for Josh Don 0 siblings, 2 replies; 6+ messages in thread From: Josh Don @ 2023-06-12 23:27 UTC (permalink / raw) To: Tejun Heo, Zefan Li, Johannes Weiner, Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider, cgroups, linux-kernel, Xiangling Kong, Josh Don It is easy for a cfs_rq to become throttled even when it has no enqueued entities (for example, if we have just put_prev()'d the last runnable task of the cfs_rq, and the cfs_rq is out of quota). Avoid accounting this time towards total throttle time, since it otherwise falsely inflates the stats. Note that the dequeue path is special, since we normally disallow migrations when a task is in a throttled hierarchy (see throttled_lb_pair()). Signed-off-by: Josh Don <joshdon@google.com> --- kernel/sched/fair.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 48b6f0ca13ac..ddd5dc18b238 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4873,8 +4873,14 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) if (cfs_rq->nr_running == 1) { check_enqueue_throttle(cfs_rq); - if (!throttled_hierarchy(cfs_rq)) + if (!throttled_hierarchy(cfs_rq)) { list_add_leaf_cfs_rq(cfs_rq); + } else { +#ifdef CONFIG_CFS_BANDWIDTH + if (!cfs_rq->throttled_clock) + cfs_rq->throttled_clock = rq_clock(rq_of(cfs_rq)); +#endif + } } } @@ -5480,7 +5486,9 @@ static bool throttle_cfs_rq(struct cfs_rq *cfs_rq) * throttled-list. rq->lock protects completion. */ cfs_rq->throttled = 1; - cfs_rq->throttled_clock = rq_clock(rq); + SCHED_WARN_ON(cfs_rq->throttled_clock); + if (cfs_rq->nr_running) + cfs_rq->throttled_clock = rq_clock(rq); return true; } @@ -5498,7 +5506,10 @@ void unthrottle_cfs_rq(struct cfs_rq *cfs_rq) update_rq_clock(rq); raw_spin_lock(&cfs_b->lock); - cfs_b->throttled_time += rq_clock(rq) - cfs_rq->throttled_clock; + if (cfs_rq->throttled_clock) { + cfs_b->throttled_time += rq_clock(rq) - cfs_rq->throttled_clock; + cfs_rq->throttled_clock = 0; + } list_del_rcu(&cfs_rq->throttled_list); raw_spin_unlock(&cfs_b->lock); -- 2.41.0.162.gfafddb0af9-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] sched: add throttled time stat for throttled children 2023-06-12 23:27 [PATCH v2 1/2] sched: don't account throttle time for empty groups Josh Don @ 2023-06-12 23:27 ` Josh Don 2023-06-15 11:38 ` [tip: sched/core] sched: Add " tip-bot2 for Josh Don 2023-06-19 17:53 ` [PATCH v2 2/2] sched: add " Michal Koutný 2023-06-15 11:38 ` [tip: sched/core] sched: Don't account throttle time for empty groups tip-bot2 for Josh Don 1 sibling, 2 replies; 6+ messages in thread From: Josh Don @ 2023-06-12 23:27 UTC (permalink / raw) To: Tejun Heo, Zefan Li, Johannes Weiner, Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider, cgroups, linux-kernel, Xiangling Kong, Josh Don We currently export the total throttled time for cgroups that are given a bandwidth limit. This patch extends this accounting to also account the total time that each children cgroup has been throttled. This is useful to understand the degree to which children have been affected by the throttling control. Children which are not runnable during the entire throttled period, for example, will not show any self-throttling time during this period. Expose this in a new interface, 'cpu.stat.local', which is similar to how non-hierarchical events are accounted in 'memory.events.local'. Signed-off-by: Josh Don <joshdon@google.com> --- v2: - moved export to new cpu.stat.local file, per Tejun's recommendation include/linux/cgroup-defs.h | 2 ++ kernel/cgroup/cgroup.c | 34 ++++++++++++++++++++++++++++ kernel/sched/core.c | 44 +++++++++++++++++++++++++++++++++++++ kernel/sched/fair.c | 21 +++++++++++++++++- kernel/sched/sched.h | 2 ++ 5 files changed, 102 insertions(+), 1 deletion(-) diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h index 8a0d5466c7be..ae20dbb885d6 100644 --- a/include/linux/cgroup-defs.h +++ b/include/linux/cgroup-defs.h @@ -661,6 +661,8 @@ struct cgroup_subsys { void (*css_rstat_flush)(struct cgroup_subsys_state *css, int cpu); int (*css_extra_stat_show)(struct seq_file *seq, struct cgroup_subsys_state *css); + int (*css_local_stat_show)(struct seq_file *seq, + struct cgroup_subsys_state *css); int (*can_attach)(struct cgroup_taskset *tset); void (*cancel_attach)(struct cgroup_taskset *tset); diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index b26ae200abef..eafbdb58ee81 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -3726,6 +3726,36 @@ static int cpu_stat_show(struct seq_file *seq, void *v) return ret; } +static int __maybe_unused cgroup_local_stat_show(struct seq_file *seq, + struct cgroup *cgrp, int ssid) +{ + struct cgroup_subsys *ss = cgroup_subsys[ssid]; + struct cgroup_subsys_state *css; + int ret; + + if (!ss->css_local_stat_show) + return 0; + + css = cgroup_tryget_css(cgrp, ss); + if (!css) + return 0; + + ret = ss->css_local_stat_show(seq, css); + css_put(css); + return ret; +} + +static int cpu_local_stat_show(struct seq_file *seq, void *v) +{ + struct cgroup __maybe_unused *cgrp = seq_css(seq)->cgroup; + int ret = 0; + +#ifdef CONFIG_CGROUP_SCHED + ret = cgroup_local_stat_show(seq, cgrp, cpu_cgrp_id); +#endif + return ret; +} + #ifdef CONFIG_PSI static int cgroup_io_pressure_show(struct seq_file *seq, void *v) { @@ -5276,6 +5306,10 @@ static struct cftype cgroup_base_files[] = { .name = "cpu.stat", .seq_show = cpu_stat_show, }, + { + .name = "cpu.stat.local", + .seq_show = cpu_local_stat_show, + }, { } /* terminate */ }; diff --git a/kernel/sched/core.c b/kernel/sched/core.c index a68d1276bab0..02e1a1a78bd0 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -11103,6 +11103,27 @@ static int cpu_cfs_stat_show(struct seq_file *sf, void *v) return 0; } + +static u64 throttled_time_self(struct task_group *tg) +{ + int i; + u64 total = 0; + + for_each_possible_cpu(i) { + total += READ_ONCE(tg->cfs_rq[i]->throttled_clock_self_time); + } + + return total; +} + +static int cpu_cfs_local_stat_show(struct seq_file *sf, void *v) +{ + struct task_group *tg = css_tg(seq_css(sf)); + + seq_printf(sf, "throttled_time %llu\n", throttled_time_self(tg)); + + return 0; +} #endif /* CONFIG_CFS_BANDWIDTH */ #endif /* CONFIG_FAIR_GROUP_SCHED */ @@ -11179,6 +11200,10 @@ static struct cftype cpu_legacy_files[] = { .name = "stat", .seq_show = cpu_cfs_stat_show, }, + { + .name = "stat.local", + .seq_show = cpu_cfs_local_stat_show, + }, #endif #ifdef CONFIG_RT_GROUP_SCHED { @@ -11235,6 +11260,24 @@ static int cpu_extra_stat_show(struct seq_file *sf, return 0; } +static int cpu_local_stat_show(struct seq_file *sf, + struct cgroup_subsys_state *css) +{ +#ifdef CONFIG_CFS_BANDWIDTH + { + struct task_group *tg = css_tg(css); + u64 throttled_self_usec; + + throttled_self_usec = throttled_time_self(tg); + do_div(throttled_self_usec, NSEC_PER_USEC); + + seq_printf(sf, "throttled_usec %llu\n", + throttled_self_usec); + } +#endif + return 0; +} + #ifdef CONFIG_FAIR_GROUP_SCHED static u64 cpu_weight_read_u64(struct cgroup_subsys_state *css, struct cftype *cft) @@ -11413,6 +11456,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { .css_released = cpu_cgroup_css_released, .css_free = cpu_cgroup_css_free, .css_extra_stat_show = cpu_extra_stat_show, + .css_local_stat_show = cpu_local_stat_show, #ifdef CONFIG_RT_GROUP_SCHED .can_attach = cpu_cgroup_can_attach, #endif diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index ddd5dc18b238..606885fc67be 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4877,8 +4877,12 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) list_add_leaf_cfs_rq(cfs_rq); } else { #ifdef CONFIG_CFS_BANDWIDTH + struct rq *rq = rq_of(cfs_rq); + if (!cfs_rq->throttled_clock) - cfs_rq->throttled_clock = rq_clock(rq_of(cfs_rq)); + cfs_rq->throttled_clock = rq_clock(rq); + if (!cfs_rq->throttled_clock_self) + cfs_rq->throttled_clock_self = rq_clock(rq); #endif } } @@ -5385,6 +5389,17 @@ static int tg_unthrottle_up(struct task_group *tg, void *data) list_add_leaf_cfs_rq(cfs_rq); } + if (cfs_rq->throttled_clock_self) { + u64 delta = rq_clock(rq) - cfs_rq->throttled_clock_self; + + cfs_rq->throttled_clock_self = 0; + + if (SCHED_WARN_ON((s64)delta < 0)) + delta = 0; + + cfs_rq->throttled_clock_self_time += delta; + } + return 0; } @@ -5400,6 +5415,10 @@ static int tg_throttle_down(struct task_group *tg, void *data) } cfs_rq->throttle_count++; + SCHED_WARN_ON(cfs_rq->throttled_clock_self); + if (cfs_rq->nr_running) + cfs_rq->throttled_clock_self = rq_clock(rq); + return 0; } diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 678446251c35..1d4c2434ec9b 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -642,6 +642,8 @@ struct cfs_rq { u64 throttled_clock; u64 throttled_clock_pelt; u64 throttled_clock_pelt_time; + u64 throttled_clock_self; + u64 throttled_clock_self_time; int throttled; int throttle_count; struct list_head throttled_list; -- 2.41.0.162.gfafddb0af9-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip: sched/core] sched: Add throttled time stat for throttled children 2023-06-12 23:27 ` [PATCH v2 2/2] sched: add throttled time stat for throttled children Josh Don @ 2023-06-15 11:38 ` tip-bot2 for Josh Don 2023-06-19 17:53 ` [PATCH v2 2/2] sched: add " Michal Koutný 1 sibling, 0 replies; 6+ messages in thread From: tip-bot2 for Josh Don @ 2023-06-15 11:38 UTC (permalink / raw) To: linux-tip-commits; +Cc: Josh Don, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: 06f5576bf513824b40d535d6b872c0265bb44c0d Gitweb: https://git.kernel.org/tip/06f5576bf513824b40d535d6b872c0265bb44c0d Author: Josh Don <joshdon@google.com> AuthorDate: Mon, 12 Jun 2023 16:27:48 -07:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Thu, 15 Jun 2023 13:28:20 +02:00 sched: Add throttled time stat for throttled children We currently export the total throttled time for cgroups that are given a bandwidth limit. This patch extends this accounting to also account the total time that each children cgroup has been throttled. This is useful to understand the degree to which children have been affected by the throttling control. Children which are not runnable during the entire throttled period, for example, will not show any self-throttling time during this period. Expose this in a new interface, 'cpu.stat.local', which is similar to how non-hierarchical events are accounted in 'memory.events.local'. Signed-off-by: Josh Don <joshdon@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20230612232748.3948659-2-joshdon@google.com --- include/linux/cgroup-defs.h | 2 ++- kernel/cgroup/cgroup.c | 34 ++++++++++++++++++++++++++++- kernel/sched/core.c | 44 ++++++++++++++++++++++++++++++++++++- kernel/sched/fair.c | 21 ++++++++++++++++- kernel/sched/sched.h | 2 ++- 5 files changed, 102 insertions(+), 1 deletion(-) diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h index 8a0d546..ae20dbb 100644 --- a/include/linux/cgroup-defs.h +++ b/include/linux/cgroup-defs.h @@ -661,6 +661,8 @@ struct cgroup_subsys { void (*css_rstat_flush)(struct cgroup_subsys_state *css, int cpu); int (*css_extra_stat_show)(struct seq_file *seq, struct cgroup_subsys_state *css); + int (*css_local_stat_show)(struct seq_file *seq, + struct cgroup_subsys_state *css); int (*can_attach)(struct cgroup_taskset *tset); void (*cancel_attach)(struct cgroup_taskset *tset); diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index b26ae20..eafbdb5 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -3726,6 +3726,36 @@ static int cpu_stat_show(struct seq_file *seq, void *v) return ret; } +static int __maybe_unused cgroup_local_stat_show(struct seq_file *seq, + struct cgroup *cgrp, int ssid) +{ + struct cgroup_subsys *ss = cgroup_subsys[ssid]; + struct cgroup_subsys_state *css; + int ret; + + if (!ss->css_local_stat_show) + return 0; + + css = cgroup_tryget_css(cgrp, ss); + if (!css) + return 0; + + ret = ss->css_local_stat_show(seq, css); + css_put(css); + return ret; +} + +static int cpu_local_stat_show(struct seq_file *seq, void *v) +{ + struct cgroup __maybe_unused *cgrp = seq_css(seq)->cgroup; + int ret = 0; + +#ifdef CONFIG_CGROUP_SCHED + ret = cgroup_local_stat_show(seq, cgrp, cpu_cgrp_id); +#endif + return ret; +} + #ifdef CONFIG_PSI static int cgroup_io_pressure_show(struct seq_file *seq, void *v) { @@ -5276,6 +5306,10 @@ static struct cftype cgroup_base_files[] = { .name = "cpu.stat", .seq_show = cpu_stat_show, }, + { + .name = "cpu.stat.local", + .seq_show = cpu_local_stat_show, + }, { } /* terminate */ }; diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ac38225..962a8d0 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -11140,6 +11140,27 @@ static int cpu_cfs_stat_show(struct seq_file *sf, void *v) return 0; } + +static u64 throttled_time_self(struct task_group *tg) +{ + int i; + u64 total = 0; + + for_each_possible_cpu(i) { + total += READ_ONCE(tg->cfs_rq[i]->throttled_clock_self_time); + } + + return total; +} + +static int cpu_cfs_local_stat_show(struct seq_file *sf, void *v) +{ + struct task_group *tg = css_tg(seq_css(sf)); + + seq_printf(sf, "throttled_time %llu\n", throttled_time_self(tg)); + + return 0; +} #endif /* CONFIG_CFS_BANDWIDTH */ #endif /* CONFIG_FAIR_GROUP_SCHED */ @@ -11216,6 +11237,10 @@ static struct cftype cpu_legacy_files[] = { .name = "stat", .seq_show = cpu_cfs_stat_show, }, + { + .name = "stat.local", + .seq_show = cpu_cfs_local_stat_show, + }, #endif #ifdef CONFIG_RT_GROUP_SCHED { @@ -11272,6 +11297,24 @@ static int cpu_extra_stat_show(struct seq_file *sf, return 0; } +static int cpu_local_stat_show(struct seq_file *sf, + struct cgroup_subsys_state *css) +{ +#ifdef CONFIG_CFS_BANDWIDTH + { + struct task_group *tg = css_tg(css); + u64 throttled_self_usec; + + throttled_self_usec = throttled_time_self(tg); + do_div(throttled_self_usec, NSEC_PER_USEC); + + seq_printf(sf, "throttled_usec %llu\n", + throttled_self_usec); + } +#endif + return 0; +} + #ifdef CONFIG_FAIR_GROUP_SCHED static u64 cpu_weight_read_u64(struct cgroup_subsys_state *css, struct cftype *cft) @@ -11450,6 +11493,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { .css_released = cpu_cgroup_css_released, .css_free = cpu_cgroup_css_free, .css_extra_stat_show = cpu_extra_stat_show, + .css_local_stat_show = cpu_local_stat_show, #ifdef CONFIG_RT_GROUP_SCHED .can_attach = cpu_cgroup_can_attach, #endif diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index b56c86c..819efad 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4877,8 +4877,12 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) list_add_leaf_cfs_rq(cfs_rq); } else { #ifdef CONFIG_CFS_BANDWIDTH + struct rq *rq = rq_of(cfs_rq); + if (!cfs_rq->throttled_clock) - cfs_rq->throttled_clock = rq_clock(rq_of(cfs_rq)); + cfs_rq->throttled_clock = rq_clock(rq); + if (!cfs_rq->throttled_clock_self) + cfs_rq->throttled_clock_self = rq_clock(rq); #endif } } @@ -5385,6 +5389,17 @@ static int tg_unthrottle_up(struct task_group *tg, void *data) list_add_leaf_cfs_rq(cfs_rq); } + if (cfs_rq->throttled_clock_self) { + u64 delta = rq_clock(rq) - cfs_rq->throttled_clock_self; + + cfs_rq->throttled_clock_self = 0; + + if (SCHED_WARN_ON((s64)delta < 0)) + delta = 0; + + cfs_rq->throttled_clock_self_time += delta; + } + return 0; } @@ -5400,6 +5415,10 @@ static int tg_throttle_down(struct task_group *tg, void *data) } cfs_rq->throttle_count++; + SCHED_WARN_ON(cfs_rq->throttled_clock_self); + if (cfs_rq->nr_running) + cfs_rq->throttled_clock_self = rq_clock(rq); + return 0; } diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 556496c..46f8a01 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -636,6 +636,8 @@ struct cfs_rq { u64 throttled_clock; u64 throttled_clock_pelt; u64 throttled_clock_pelt_time; + u64 throttled_clock_self; + u64 throttled_clock_self_time; int throttled; int throttle_count; struct list_head throttled_list; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] sched: add throttled time stat for throttled children 2023-06-12 23:27 ` [PATCH v2 2/2] sched: add throttled time stat for throttled children Josh Don 2023-06-15 11:38 ` [tip: sched/core] sched: Add " tip-bot2 for Josh Don @ 2023-06-19 17:53 ` Michal Koutný 2023-06-20 18:28 ` Josh Don 1 sibling, 1 reply; 6+ messages in thread From: Michal Koutný @ 2023-06-19 17:53 UTC (permalink / raw) To: Josh Don Cc: Tejun Heo, Zefan Li, Johannes Weiner, Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider, cgroups, linux-kernel, Xiangling Kong [-- Attachment #1: Type: text/plain, Size: 646 bytes --] On Mon, Jun 12, 2023 at 04:27:48PM -0700, Josh Don <joshdon@google.com> wrote: > We currently export the total throttled time for cgroups that are given > a bandwidth limit. I assume you refer to cpu.stat:throttled_usec (from struct cfs_bandwidth) -- notice that the value is not properly hierarchical despite v2 filename. > This patch extends this accounting to also account the total time that > each children cgroup has been throttled. IIUC, this is visible on inner-nodes cpu cgroups (i.e. with no tasks)? IOW, wouldn't you get the intended information if hierarchical summing was added/fixed for cpu.stat:throttled_usec? Thanks, Michal [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] sched: add throttled time stat for throttled children 2023-06-19 17:53 ` [PATCH v2 2/2] sched: add " Michal Koutný @ 2023-06-20 18:28 ` Josh Don 0 siblings, 0 replies; 6+ messages in thread From: Josh Don @ 2023-06-20 18:28 UTC (permalink / raw) To: Michal Koutný Cc: Tejun Heo, Zefan Li, Johannes Weiner, Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman, Daniel Bristot de Oliveira, Valentin Schneider, cgroups, linux-kernel, Xiangling Kong Hi Michal, On Mon, Jun 19, 2023 at 10:53 AM Michal Koutný <mkoutny@suse.com> wrote: > > On Mon, Jun 12, 2023 at 04:27:48PM -0700, Josh Don <joshdon@google.com> wrote: > > We currently export the total throttled time for cgroups that are given > > a bandwidth limit. > > I assume you refer to cpu.stat:throttled_usec (from struct > cfs_bandwidth) -- notice that the value is not properly hierarchical > despite v2 filename. > > > This patch extends this accounting to also account the total time that > > each children cgroup has been throttled. > > IIUC, this is visible on inner-nodes cpu cgroups (i.e. with no tasks)? > > IOW, wouldn't you get the intended information if hierarchical summing > was added/fixed for cpu.stat:throttled_usec? It isn't currently hierarchical in the sense that the inner-nodes don't themselves account their throttled time, but the summation at the top is still correct. This patch is intended to close the gap. I suppose your question here is why not simply make the existing throttled_usec export properly hierarchical, and avoid the extra stat export here. I think it might be useful to still expose a non-hierarchical metric indicating the throttled time due to the group's own configured limit; the accounting can look strange with nested bandwidth limits. Not strongly opposed to the idea, but your hierarchical accounting proposal is essentially what this patch adds. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip: sched/core] sched: Don't account throttle time for empty groups 2023-06-12 23:27 [PATCH v2 1/2] sched: don't account throttle time for empty groups Josh Don 2023-06-12 23:27 ` [PATCH v2 2/2] sched: add throttled time stat for throttled children Josh Don @ 2023-06-15 11:38 ` tip-bot2 for Josh Don 1 sibling, 0 replies; 6+ messages in thread From: tip-bot2 for Josh Don @ 2023-06-15 11:38 UTC (permalink / raw) To: linux-tip-commits; +Cc: Josh Don, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: eed673d4f701e17b8f887dbcbfd3b4bcc53503ec Gitweb: https://git.kernel.org/tip/eed673d4f701e17b8f887dbcbfd3b4bcc53503ec Author: Josh Don <joshdon@google.com> AuthorDate: Mon, 12 Jun 2023 16:27:47 -07:00 Committer: Peter Zijlstra <peterz@infradead.org> CommitterDate: Thu, 15 Jun 2023 13:28:19 +02:00 sched: Don't account throttle time for empty groups It is easy for a cfs_rq to become throttled even when it has no enqueued entities (for example, if we have just put_prev()'d the last runnable task of the cfs_rq, and the cfs_rq is out of quota). Avoid accounting this time towards total throttle time, since it otherwise falsely inflates the stats. Note that the dequeue path is special, since we normally disallow migrations when a task is in a throttled hierarchy (see throttled_lb_pair()). Signed-off-by: Josh Don <joshdon@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20230612232748.3948659-1-joshdon@google.com --- kernel/sched/fair.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 6189d1a..b56c86c 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -4873,8 +4873,14 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) if (cfs_rq->nr_running == 1) { check_enqueue_throttle(cfs_rq); - if (!throttled_hierarchy(cfs_rq)) + if (!throttled_hierarchy(cfs_rq)) { list_add_leaf_cfs_rq(cfs_rq); + } else { +#ifdef CONFIG_CFS_BANDWIDTH + if (!cfs_rq->throttled_clock) + cfs_rq->throttled_clock = rq_clock(rq_of(cfs_rq)); +#endif + } } } @@ -5480,7 +5486,9 @@ done: * throttled-list. rq->lock protects completion. */ cfs_rq->throttled = 1; - cfs_rq->throttled_clock = rq_clock(rq); + SCHED_WARN_ON(cfs_rq->throttled_clock); + if (cfs_rq->nr_running) + cfs_rq->throttled_clock = rq_clock(rq); return true; } @@ -5498,7 +5506,10 @@ void unthrottle_cfs_rq(struct cfs_rq *cfs_rq) update_rq_clock(rq); raw_spin_lock(&cfs_b->lock); - cfs_b->throttled_time += rq_clock(rq) - cfs_rq->throttled_clock; + if (cfs_rq->throttled_clock) { + cfs_b->throttled_time += rq_clock(rq) - cfs_rq->throttled_clock; + cfs_rq->throttled_clock = 0; + } list_del_rcu(&cfs_rq->throttled_list); raw_spin_unlock(&cfs_b->lock); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-20 18:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-12 23:27 [PATCH v2 1/2] sched: don't account throttle time for empty groups Josh Don 2023-06-12 23:27 ` [PATCH v2 2/2] sched: add throttled time stat for throttled children Josh Don 2023-06-15 11:38 ` [tip: sched/core] sched: Add " tip-bot2 for Josh Don 2023-06-19 17:53 ` [PATCH v2 2/2] sched: add " Michal Koutný 2023-06-20 18:28 ` Josh Don 2023-06-15 11:38 ` [tip: sched/core] sched: Don't account throttle time for empty groups tip-bot2 for Josh Don
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®