| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <jhj35y5v5s9.mognet@arm.com>
Date: Tue, 09 Feb 2021 11:28:54 +0000
From: Valentin Schneider <valentin.schneider@....com>
To: Barry Song <song.bao.hua@...ilicon.com>,
vincent.guittot@...aro.org, mgorman@...e.de, mingo@...nel.org,
peterz@...radead.org, dietmar.eggemann@....com,
morten.rasmussen@....com, linux-kernel@...r.kernel.org
Cc: linuxarm@...neuler.org, xuwei5@...wei.com, liguozhu@...ilicon.com,
tiantao6@...ilicon.com, wanghuiqiang@...wei.com,
prime.zeng@...ilicon.com, jonathan.cameron@...wei.com,
guodong.xu@...aro.org, Barry Song <song.bao.hua@...ilicon.com>,
Meelis Roos <mroos@...ux.ee>
Subject: Re: [PATCH v3] sched/topology: fix the issue groups don't span domain->span for NUMA diameter > 2
On 09/02/21 21:21, Barry Song wrote:
> Reported-by: Valentin Schneider <valentin.schneider@....com>
> Tested-by: Meelis Roos <mroos@...ux.ee>
> Signed-off-by: Barry Song <song.bao.hua@...ilicon.com>
Tested on a bunch of NUMA topologies via QEMU (AMD Epyc, D06, Sunfire) and
the results look sane.
Small comment nits below, but regardless:
Reviewed-by: Valentin Schneider <valentin.schneider@....com>
> ---
> -v3:
> Mainly updated according to Valentin's comments. While the approach was
> started by me, Valentin contributed the most useful edit and comments.
> Thanks, Valentin!
>
Happy to help, thanks for fixing this!
> * fixed a potential issue that re-used sgc might be located in
> a sched_domain which will be degenrated;
> * code cleanup to make it more readable
>
> While Valentin started another approach which completely removed overlapped
> sched_group, we both agree that it is better to have a solution which won't
> touch machines without 3-hops issue first:
> https://lore.kernel.org/lkml/jhjpn1a232z.mognet@arm.com/
>
I think this doesn't scale properly so I doubt it's ever going to see the
light of day, but I had to write it to convince myself of it :/
> kernel/sched/topology.c | 91 +++++++++++++++++++++++++++--------------
> 1 file changed, 61 insertions(+), 30 deletions(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 5d3675c7a76b..ab5ebf17f30a 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -982,6 +953,31 @@ static void init_overlap_sched_group(struct sched_domain *sd,
> sg->sgc->max_capacity = SCHED_CAPACITY_SCALE;
> }
>
> +static struct sched_domain *
> +find_descended_sibling(struct sched_domain *sd, struct sched_domain *sibling)
> +{
> + /*
> + * The proper descendant would be the one whose child won't span out
> + * of sd
> + */
> + while (sibling->child &&
> + !cpumask_subset(sched_domain_span(sibling->child),
> + sched_domain_span(sd)))
> + sibling = sibling->child;
> +
> + /*
> + * As we are referencing sgc across different topology level, we need
> + * to go down to skip those sched_domains which don't contribute to
> + * scheduling because they will be degenerated in cpu_attach_domain
+ * This is important because we must make sure we point to an sgc that
+ * will be updated via update_group_capacity().
> + */
> + while (sibling->child &&
> + cpumask_equal(sched_domain_span(sibling->child),
> + sched_domain_span(sibling)))
> + sibling = sibling->child;
> +
> + return sibling;
> +}
> +
> static int
> build_overlap_sched_groups(struct sched_domain *sd, int cpu)
> {
> @@ -1015,6 +1011,41 @@ build_overlap_sched_groups(struct sched_domain *sd, int cpu)
> if (!cpumask_test_cpu(i, sched_domain_span(sibling)))
> continue;
>
> + /*
> + * Usually we build sched_group by sibling's child sched_domain
> + * But for machines whose NUMA diameter are 3 or above, we move
> + * to build sched_group by sibling's proper descendant's child
> + * domain because sibling's child sched_domain will span out of
> + * the sched_domain being built as below.
> + *
> + * Smallest diameter=3 topology is:
> + *
> + * node 0 1 2 3
> + * 0: 10 20 30 40
> + * 1: 20 10 20 30
> + * 2: 30 20 10 20
> + * 3: 40 30 20 10
> + *
> + * 0 --- 1 --- 2 --- 3
> + *
> + * NUMA-3 0-3 N/A N/A 0-3
> + * groups: {0-2},{1-3} {1-3},{0-2}
> + *
> + * NUMA-2 0-2 0-3 0-3 1-3
> + * groups: {0-1},{1-3} {0-2},{2-3} {1-3},{0-1} {2-3},{0-2}
> + *
> + * NUMA-1 0-1 0-2 1-3 2-3
> + * groups: {0},{1} {1},{2},{0} {2},{3},{1} {3},{2}
> + *
> + * NUMA-0 0 1 2 3
> + *
> + * The NUMA-2 groups for nodes 0 and 3 are obviously buggered, as the
^^^
s/are/would be/
> + * group span isn't a subset of the domain span.
^^^^^
s/isn't/wouldn't be/
> + */
> + if (sibling->child &&
> + !cpumask_subset(sched_domain_span(sibling->child), span))
> + sibling = find_descended_sibling(sd, sibling);
> +
> sg = build_group_from_child_sched_domain(sibling, cpu);
> if (!sg)
> goto fail;
> @@ -1022,7 +1053,7 @@ build_overlap_sched_groups(struct sched_domain *sd, int cpu)
> sg_span = sched_group_span(sg);
> cpumask_or(covered, covered, sg_span);
>
> - init_overlap_sched_group(sd, sg);
> + init_overlap_sched_group(sibling, sg);
>
> if (!first)
> first = sg;
> --
> 2.25.1
Powered by blists - more mailing lists