* [PATCH] sched/fair: add comments for group_type and balancing at SD_NUMA level @ 2019-11-12 14:50 Vincent Guittot 2019-11-12 17:42 ` Valentin Schneider 2019-11-18 17:42 ` [tip: sched/core] sched/fair: Add " tip-bot2 for Vincent Guittot 0 siblings, 2 replies; 5+ messages in thread From: Vincent Guittot @ 2019-11-12 14:50 UTC (permalink / raw) To: linux-kernel, mingo, peterz, dietmar.eggemann, juri.lelli, rostedt, mgorman Cc: bsegall, Vincent Guittot Add comments to describe each state of goup_type and to add some details about the load balance at NUMA level. Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> --- kernel/sched/fair.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index c93d534..268e441 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6986,11 +6986,34 @@ enum fbq_type { regular, remote, all }; * group. see update_sd_pick_busiest(). */ enum group_type { + /* + * The group has spare capacity that can be used to process more work. + */ group_has_spare = 0, + /* + * The group is fully used and the tasks don't compete for more CPU + * cycles. Nevetheless, some tasks might wait before running. + */ group_fully_busy, + /* + * One task doesn't fit with CPU's capacity and must be migrated on a + * more powerful CPU. + */ group_misfit_task, + /* + * One local CPU with higher capacity is available and task should be + * migrated on it instead on current CPU. + */ group_asym_packing, + /* + * The tasks affinity prevents the scheduler to balance the load across + * the system. + */ group_imbalanced, + /* + * The CPU is overloaded and can't provide expected CPU cycles to all + * tasks. + */ group_overloaded }; @@ -8608,7 +8631,11 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s /* * Try to use spare capacity of local group without overloading it or - * emptying busiest + * emptying busiest. + * XXX Spreading tasks across numa nodes is not always the best policy + * and special cares should be taken for SD_NUMA domain level before + * spreading the tasks. For now, load_balance() fully relies on + * NUMA_BALANCING and fbq_classify_group/rq to overide the decision. */ if (local->group_type == group_has_spare) { if (busiest->group_type > group_fully_busy) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] sched/fair: add comments for group_type and balancing at SD_NUMA level 2019-11-12 14:50 [PATCH] sched/fair: add comments for group_type and balancing at SD_NUMA level Vincent Guittot @ 2019-11-12 17:42 ` Valentin Schneider 2019-11-18 13:34 ` [PATCH v2] " Ingo Molnar 2019-11-18 17:42 ` [tip: sched/core] sched/fair: Add " tip-bot2 for Vincent Guittot 1 sibling, 1 reply; 5+ messages in thread From: Valentin Schneider @ 2019-11-12 17:42 UTC (permalink / raw) To: Vincent Guittot, linux-kernel, mingo, peterz, dietmar.eggemann, juri.lelli, rostedt, mgorman Cc: bsegall Hi Vincent, On 12/11/2019 14:50, Vincent Guittot wrote: > Add comments to describe each state of goup_type and to add some details > about the load balance at NUMA level. > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> Suggestions/nits below. There's a bit of duplication with existing comments (e.g. the nice blob atop sg_imbalanced()), but I think it can't hurt to have the few extra lines you're introducing. --- diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index bfdcaf91b325..ec93ebd02352 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6955,28 +6955,26 @@ enum fbq_type { regular, remote, all }; * group. see update_sd_pick_busiest(). */ enum group_type { - /* - * The group has spare capacity that can be used to process more work. - */ + /* The group isn't significantly pressured and can be used to process more work */ group_has_spare = 0, /* * The group is fully used and the tasks don't compete for more CPU - * cycles. Nevetheless, some tasks might wait before running. + * cycles. Nevertheless, some tasks might wait before running. */ group_fully_busy, /* - * One task doesn't fit with CPU's capacity and must be migrated on a - * more powerful CPU. + * (SD_ASYM_CPUCAPACITY only) One task doesn't fit on its CPU's + * capacity and must be migrated to a CPU of higher capacity. */ group_misfit_task, /* - * One local CPU with higher capacity is available and task should be - * migrated on it instead on current CPU. + * (SD_ASYM_PACKING only) One local CPU with higher capacity is + * available and task should be migrated to it. */ group_asym_packing, /* - * The tasks affinity prevents the scheduler to balance the load across - * the system. + * The tasks affinity previously prevented the scheduler from balancing + * load across the system. */ group_imbalanced, /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] sched/fair: add comments for group_type and balancing at SD_NUMA level 2019-11-12 17:42 ` Valentin Schneider @ 2019-11-18 13:34 ` Ingo Molnar 2019-11-18 14:06 ` Valentin Schneider 0 siblings, 1 reply; 5+ messages in thread From: Ingo Molnar @ 2019-11-18 13:34 UTC (permalink / raw) To: Valentin Schneider Cc: Vincent Guittot, linux-kernel, mingo, peterz, dietmar.eggemann, juri.lelli, rostedt, mgorman, bsegall * Valentin Schneider <valentin.schneider@arm.com> wrote: > Hi Vincent, > > On 12/11/2019 14:50, Vincent Guittot wrote: > > Add comments to describe each state of goup_type and to add some details > > about the load balance at NUMA level. > > > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > > Suggestions/nits below. There's a bit of duplication with existing > comments (e.g. the nice blob atop sg_imbalanced()), but I think it can't > hurt to have the few extra lines you're introducing. > > --- > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index bfdcaf91b325..ec93ebd02352 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -6955,28 +6955,26 @@ enum fbq_type { regular, remote, all }; > * group. see update_sd_pick_busiest(). > */ > enum group_type { > - /* > - * The group has spare capacity that can be used to process more work. > - */ > + /* The group isn't significantly pressured and can be used to process more work */ > group_has_spare = 0, > /* > * The group is fully used and the tasks don't compete for more CPU > - * cycles. Nevetheless, some tasks might wait before running. > + * cycles. Nevertheless, some tasks might wait before running. > */ > group_fully_busy, > /* > - * One task doesn't fit with CPU's capacity and must be migrated on a > - * more powerful CPU. > + * (SD_ASYM_CPUCAPACITY only) One task doesn't fit on its CPU's > + * capacity and must be migrated to a CPU of higher capacity. > */ > group_misfit_task, > /* > - * One local CPU with higher capacity is available and task should be > - * migrated on it instead on current CPU. > + * (SD_ASYM_PACKING only) One local CPU with higher capacity is > + * available and task should be migrated to it. > */ > group_asym_packing, > /* > - * The tasks affinity prevents the scheduler to balance the load across > - * the system. > + * The tasks affinity previously prevented the scheduler from balancing > + * load across the system. > */ > group_imbalanced, Thanks - I did a few more fixes and updates to the comments, this is how it ended up looking like (full patch below): /* * 'group_type' describes the group of CPUs at the moment of load balancing. * * The enum is ordered by pulling priority, with the group with lowest priority * first so the group_type can simply be compared when selecting the busiest * group. See update_sd_pick_busiest(). */ enum group_type { /* The group has spare capacity that can be used to run more tasks. */ group_has_spare = 0, /* * The group is fully used and the tasks don't compete for more CPU * cycles. Nevertheless, some tasks might wait before running. */ group_fully_busy, /* * SD_ASYM_CPUCAPACITY only: One task doesn't fit with CPU's capacity * and must be migrated to a more powerful CPU. */ group_misfit_task, /* * SD_ASYM_PACKING only: One local CPU with higher capacity is available, * and the task should be migrated to it instead of running on the * current CPU. */ group_asym_packing, /* * The tasks' affinity constraints previously prevented the scheduler * from balancing the load across the system. */ group_imbalanced, /* * The CPU is overloaded and can't provide expected CPU cycles to all * tasks. */ group_overloaded }; I also added your Acked-by, which I think was implicit? :) Thanks, Ingo =====> From: Vincent Guittot <vincent.guittot@linaro.org> Date: Tue, 12 Nov 2019 15:50:43 +0100 Subject: [PATCH] sched/fair: Add comments for group_type and balancing at SD_NUMA level Add comments to describe each state of goup_type and to add some details about the load balance at NUMA level. [ Valentin Schneider: Updates to the comments. ] [ mingo: Other updates to the comments. ] Reported-by: Mel Gorman <mgorman@suse.de> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> Acked-by: Valentin Schneider <valentin.schneider@arm.com> Cc: Ben Segall <bsegall@google.com> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> Cc: Juri Lelli <juri.lelli@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Mike Galbraith <efault@gmx.de> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: https://lkml.kernel.org/r/1573570243-1903-1-git-send-email-vincent.guittot@linaro.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- kernel/sched/fair.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 2fc08e7d9cd6..1f93d96dd06b 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6980,17 +6980,40 @@ static unsigned long __read_mostly max_load_balance_interval = HZ/10; enum fbq_type { regular, remote, all }; /* - * group_type describes the group of CPUs at the moment of the load balance. + * 'group_type' describes the group of CPUs at the moment of load balancing. + * * The enum is ordered by pulling priority, with the group with lowest priority - * first so the groupe_type can be simply compared when selecting the busiest - * group. see update_sd_pick_busiest(). + * first so the group_type can simply be compared when selecting the busiest + * group. See update_sd_pick_busiest(). */ enum group_type { + /* The group has spare capacity that can be used to run more tasks. */ group_has_spare = 0, + /* + * The group is fully used and the tasks don't compete for more CPU + * cycles. Nevertheless, some tasks might wait before running. + */ group_fully_busy, + /* + * SD_ASYM_CPUCAPACITY only: One task doesn't fit with CPU's capacity + * and must be migrated to a more powerful CPU. + */ group_misfit_task, + /* + * SD_ASYM_PACKING only: One local CPU with higher capacity is available, + * and the task should be migrated to it instead of running on the + * current CPU. + */ group_asym_packing, + /* + * The tasks' affinity constraints previously prevented the scheduler + * from balancing the load across the system. + */ group_imbalanced, + /* + * The CPU is overloaded and can't provide expected CPU cycles to all + * tasks. + */ group_overloaded }; @@ -8589,7 +8612,11 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s /* * Try to use spare capacity of local group without overloading it or - * emptying busiest + * emptying busiest. + * XXX Spreading tasks across NUMA nodes is not always the best policy + * and special care should be taken for SD_NUMA domain level before + * spreading the tasks. For now, load_balance() fully relies on + * NUMA_BALANCING and fbq_classify_group/rq to override the decision. */ if (local->group_type == group_has_spare) { if (busiest->group_type > group_fully_busy) { ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] sched/fair: add comments for group_type and balancing at SD_NUMA level 2019-11-18 13:34 ` [PATCH v2] " Ingo Molnar @ 2019-11-18 14:06 ` Valentin Schneider 0 siblings, 0 replies; 5+ messages in thread From: Valentin Schneider @ 2019-11-18 14:06 UTC (permalink / raw) To: Ingo Molnar Cc: Vincent Guittot, linux-kernel, mingo, peterz, dietmar.eggemann, juri.lelli, rostedt, mgorman, bsegall On 18/11/2019 13:34, Ingo Molnar wrote: > Thanks - I did a few more fixes and updates to the comments, this is how > it ended up looking like (full patch below): > [...] LGTM, thanks! > I also added your Acked-by, which I think was implicit? :) > Hah, I'm not used to handing those out, but sure! ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: sched/core] sched/fair: Add comments for group_type and balancing at SD_NUMA level 2019-11-12 14:50 [PATCH] sched/fair: add comments for group_type and balancing at SD_NUMA level Vincent Guittot 2019-11-12 17:42 ` Valentin Schneider @ 2019-11-18 17:42 ` tip-bot2 for Vincent Guittot 1 sibling, 0 replies; 5+ messages in thread From: tip-bot2 for Vincent Guittot @ 2019-11-18 17:42 UTC (permalink / raw) To: linux-tip-commits Cc: Mel Gorman, Vincent Guittot, Valentin Schneider, Ben Segall, Dietmar Eggemann, Juri Lelli, Linus Torvalds, Mike Galbraith, Peter Zijlstra, Steven Rostedt, Thomas Gleixner, Ingo Molnar, Borislav Petkov, linux-kernel The following commit has been merged into the sched/core branch of tip: Commit-ID: a9723389cc759c891d481de271ac73eeaa123bcb Gitweb: https://git.kernel.org/tip/a9723389cc759c891d481de271ac73eeaa123bcb Author: Vincent Guittot <vincent.guittot@linaro.org> AuthorDate: Tue, 12 Nov 2019 15:50:43 +01:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Mon, 18 Nov 2019 14:33:12 +01:00 sched/fair: Add comments for group_type and balancing at SD_NUMA level Add comments to describe each state of goup_type and to add some details about the load balance at NUMA level. [ Valentin Schneider: Updates to the comments. ] [ mingo: Other updates to the comments. ] Reported-by: Mel Gorman <mgorman@suse.de> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> Acked-by: Valentin Schneider <valentin.schneider@arm.com> Cc: Ben Segall <bsegall@google.com> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> Cc: Juri Lelli <juri.lelli@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Mike Galbraith <efault@gmx.de> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: https://lkml.kernel.org/r/1573570243-1903-1-git-send-email-vincent.guittot@linaro.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- kernel/sched/fair.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 2fc08e7..1f93d96 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6980,17 +6980,40 @@ static unsigned long __read_mostly max_load_balance_interval = HZ/10; enum fbq_type { regular, remote, all }; /* - * group_type describes the group of CPUs at the moment of the load balance. + * 'group_type' describes the group of CPUs at the moment of load balancing. + * * The enum is ordered by pulling priority, with the group with lowest priority - * first so the groupe_type can be simply compared when selecting the busiest - * group. see update_sd_pick_busiest(). + * first so the group_type can simply be compared when selecting the busiest + * group. See update_sd_pick_busiest(). */ enum group_type { + /* The group has spare capacity that can be used to run more tasks. */ group_has_spare = 0, + /* + * The group is fully used and the tasks don't compete for more CPU + * cycles. Nevertheless, some tasks might wait before running. + */ group_fully_busy, + /* + * SD_ASYM_CPUCAPACITY only: One task doesn't fit with CPU's capacity + * and must be migrated to a more powerful CPU. + */ group_misfit_task, + /* + * SD_ASYM_PACKING only: One local CPU with higher capacity is available, + * and the task should be migrated to it instead of running on the + * current CPU. + */ group_asym_packing, + /* + * The tasks' affinity constraints previously prevented the scheduler + * from balancing the load across the system. + */ group_imbalanced, + /* + * The CPU is overloaded and can't provide expected CPU cycles to all + * tasks. + */ group_overloaded }; @@ -8589,7 +8612,11 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s /* * Try to use spare capacity of local group without overloading it or - * emptying busiest + * emptying busiest. + * XXX Spreading tasks across NUMA nodes is not always the best policy + * and special care should be taken for SD_NUMA domain level before + * spreading the tasks. For now, load_balance() fully relies on + * NUMA_BALANCING and fbq_classify_group/rq to override the decision. */ if (local->group_type == group_has_spare) { if (busiest->group_type > group_fully_busy) { ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-11-18 17:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-12 14:50 [PATCH] sched/fair: add comments for group_type and balancing at SD_NUMA level Vincent Guittot 2019-11-12 17:42 ` Valentin Schneider 2019-11-18 13:34 ` [PATCH v2] " Ingo Molnar 2019-11-18 14:06 ` Valentin Schneider 2019-11-18 17:42 ` [tip: sched/core] sched/fair: Add " tip-bot2 for Vincent Guittot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).