From: Peter Zijlstra <peterz@infradead.org>
To: Lauro Venancio <lvenanci@redhat.com>
Cc: lwang@redhat.com, riel@redhat.com, Mike Galbraith <efault@gmx.de>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] sched/topology: the group balance cpu must be a cpu where the group is installed
Date: Tue, 25 Apr 2017 18:26:36 +0200 [thread overview]
Message-ID: <20170425162636.3jvmiys6ej5gtsxx@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <91317113-f1a7-a1c6-812e-cbda5284d404@redhat.com>
On Tue, Apr 25, 2017 at 12:56:23PM -0300, Lauro Venancio wrote:
> > Another thing I've been thinking about; I think we can do away with the
> > kzalloc() in build_group_from_child_sched_domain() and use the sdd->sg
> > storage.
> I considered this too. I decided to do not change this because I was not
> sure if the kzalloc() was there for performance reasons. Currently, all
> groups are allocated in the NUMA node they are used.
> If we use sdd->sg storage, we may have groups allocated in one NUMA node
> being used in another node.
Right.. I cannot remember :/
/me once again kicks himself for not writing more comments
It does save a few lines.. and I suspect that if we do this, we could
actually completely get rid of sched_group_capacity, since its now
always the same as the group (again), which should removes more lines
still.
But I'll shelf this patch for now.. we've got enough changes as is.
I still need to write a changelog for the new #2, which has become ugly
again, because its needs a second sched_domains_tmpmask.
(compile tested only)
---
kernel/sched/topology.c | 76 ++++++++++++++++++------------------------------
1 file changed, 29 insertions(+), 47 deletions(-)
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -501,10 +501,8 @@ enum s_alloc {
* balancing.
*/
static void
-build_group_mask(struct sched_domain *sd, struct sched_group *sg, struct cpumask *mask)
+build_group_mask(struct sd_data *sdd, struct cpumask *sg_span, struct cpumask *mask)
{
- const struct cpumask *sg_span = sched_group_cpus(sg);
- struct sd_data *sdd = sd->private;
struct sched_domain *sibling;
int i;
@@ -542,49 +540,34 @@ int group_balance_cpu(struct sched_group
}
static struct sched_group *
-build_group_from_child_sched_domain(struct sched_domain *sd, int cpu)
+get_overlap_group(struct sd_data *sdd, int cpu)
{
- struct sched_group *sg;
- struct cpumask *sg_span;
+ struct sched_domain *sd = *per_cpu_ptr(sdd->sd, cpu);
+ struct sched_domain *child = sd->child;
+ struct sched_group *group;
+ struct cpumask *mask = sched_domains_tmpmask2;
- sg = kzalloc_node(sizeof(struct sched_group) + cpumask_size(),
- GFP_KERNEL, cpu_to_node(cpu));
+ /*
+ * Overlap must have !overlap children.
+ * This is before degenerate throws them out.
+ */
+ BUG_ON(!sd->child);
- if (!sg)
- return NULL;
+ build_group_mask(sdd, sched_domain_span(child), mask);
+ cpu = cpumask_first_and(sched_domain_span(child), mask);
- sg_span = sched_group_cpus(sg);
- if (sd->child)
- cpumask_copy(sg_span, sched_domain_span(sd->child));
- else
- cpumask_copy(sg_span, sched_domain_span(sd));
+ BUG_ON(cpu >= nr_cpu_ids);
- return sg;
-}
+ group = *per_cpu_ptr(sdd->sg, cpu);
+ group->sgc = *per_cpu_ptr(sdd->sgc, cpu);
-static void init_overlap_sched_group(struct sched_domain *sd,
- struct sched_group *sg)
-{
- struct cpumask *mask = sched_domains_tmpmask2;
- struct sd_data *sdd = sd->private;
- struct cpumask *sg_span;
- int cpu;
+ atomic_inc(&group->ref);
+ atomic_inc(&group->sgc->ref);
- build_group_mask(sd, sg, mask);
- cpu = cpumask_first_and(sched_group_cpus(sg), mask);
+ cpumask_copy(sched_group_cpus(group), sched_domain_span(child));
+ cpumask_copy(sched_group_mask(group), mask);
- sg->sgc = *per_cpu_ptr(sdd->sgc, cpu);
- if (atomic_inc_return(&sg->sgc->ref) == 1)
- cpumask_copy(sched_group_mask(sg), mask);
-
- /*
- * Initialize sgc->capacity such that even if we mess up the
- * domains and no possible iteration will get us here, we won't
- * die on a /0 trap.
- */
- sg_span = sched_group_cpus(sg);
- sg->sgc->capacity = SCHED_CAPACITY_SCALE * cpumask_weight(sg_span);
- sg->sgc->min_capacity = SCHED_CAPACITY_SCALE;
+ return group;
}
static int
@@ -620,14 +603,18 @@ build_overlap_sched_groups(struct sched_
if (!cpumask_test_cpu(i, sched_domain_span(sibling)))
continue;
- sg = build_group_from_child_sched_domain(sibling, cpu);
- if (!sg)
- goto fail;
+ sg = get_overlap_group(sdd, i);
sg_span = sched_group_cpus(sg);
cpumask_or(covered, covered, sg_span);
- init_overlap_sched_group(sd, sg);
+ /*
+ * Initialize sgc->capacity such that even if we mess up the
+ * domains and no possible iteration will get us here, we won't
+ * die on a /0 trap.
+ */
+ sg->sgc->capacity = SCHED_CAPACITY_SCALE * cpumask_weight(sg_span);
+ sg->sgc->min_capacity = SCHED_CAPACITY_SCALE;
if (!first)
first = sg;
@@ -639,11 +626,6 @@ build_overlap_sched_groups(struct sched_
sd->groups = first;
return 0;
-
-fail:
- free_sched_groups(first, 0);
-
- return -ENOMEM;
}
static int get_group(int cpu, struct sd_data *sdd, struct sched_group **sg)
next prev parent reply other threads:[~2017-04-25 16:26 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-20 19:51 [PATCH 0/4] sched/topology: fix overlap group capacity and balance cpu Lauro Ramos Venancio
2017-04-20 19:51 ` [PATCH 1/4] sched/topology: optimize build_group_mask() Lauro Ramos Venancio
2017-05-15 9:06 ` [tip:sched/core] sched/topology: Optimize build_group_mask() tip-bot for Lauro Ramos Venancio
2017-04-20 19:51 ` [PATCH 2/4] sched/topology: all instances of a sched group must use the same sched_group_capacity Lauro Ramos Venancio
2017-04-20 19:51 ` [PATCH 3/4] sched/topology: move comment about asymmetric node setups Lauro Ramos Venancio
2017-04-21 16:31 ` Peter Zijlstra
2017-05-15 9:06 ` [tip:sched/core] sched/topology: Move " tip-bot for Lauro Ramos Venancio
2017-04-20 19:51 ` [PATCH 4/4] sched/topology: the group balance cpu must be a cpu where the group is installed Lauro Ramos Venancio
2017-04-24 13:03 ` Peter Zijlstra
2017-04-24 14:19 ` Peter Zijlstra
2017-04-24 14:27 ` Peter Zijlstra
2017-04-24 15:19 ` Lauro Venancio
2017-04-24 22:19 ` Peter Zijlstra
2017-04-24 15:11 ` Lauro Venancio
2017-04-24 22:15 ` Peter Zijlstra
2017-04-25 12:17 ` Peter Zijlstra
2017-04-25 14:33 ` Lauro Venancio
2017-04-25 15:12 ` Peter Zijlstra
2017-04-25 15:22 ` Peter Zijlstra
2017-04-25 15:27 ` Peter Zijlstra
2017-04-25 15:39 ` Peter Zijlstra
2017-04-25 15:52 ` Peter Zijlstra
2017-04-25 15:56 ` Lauro Venancio
2017-04-25 16:26 ` Peter Zijlstra [this message]
2017-04-26 16:31 ` [PATCH 0/4] sched/topology: fix overlap group capacity and balance cpu Peter Zijlstra
2017-04-26 17:59 ` Lauro Venancio
2017-04-26 22:43 ` Peter Zijlstra
2017-04-28 10:33 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170425162636.3jvmiys6ej5gtsxx@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=efault@gmx.de \
--cc=linux-kernel@vger.kernel.org \
--cc=lvenanci@redhat.com \
--cc=lwang@redhat.com \
--cc=mingo@kernel.org \
--cc=riel@redhat.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).