SoCs such as the Ampere Altra define clusters but have no shared
processor-side cache. As of v5.16 with CONFIG_SCHED_CLUSTER and
CONFIG_SCHED_MC, build_sched_domain() will BUG() with:
BUG: arch topology borken
the CLS domain not a subset of the MC domain
for each CPU (160 times for a 2 socket 80 core Altra system). The MC
level cpu mask is then extended to that of the CLS child, and is later
removed entirely as redundant.
This change detects when all cpu_coregroup_mask weights=1 and uses an
alternative sched_domain_topology equivalent to the default if
CONFIG_SCHED_MC were disabled.
The final resulting sched domain topology is unchanged with or without
CONFIG_SCHED_CLUSTER, and the BUG is avoided:
For CPU0:
With CLS:
CLS [0-1]
DIE [0-79]
NUMA [0-159]
Without CLS:
DIE [0-79]
NUMA [0-159]
Cc: Catalin Marinas <catalin.marinas(a)arm.com>
Cc: Will Deacon <will(a)kernel.org>
Cc: Peter Zijlstra <peterz(a)infradead.org>
Cc: Vincent Guittot <vincent.guittot(a)linaro.org>
Cc: Barry Song <song.bao.hua(a)hisilicon.com>
Cc: Valentin Schneider <valentin.schneider(a)arm.com>
Cc: D. Scott Phillips <scott(a)os.amperecomputing.com>
Cc: Ilkka Koskinen <ilkka(a)os.amperecomputing.com>
Cc: <stable(a)vger.kernel.org> # 5.16.x
Signed-off-by: Darren Hart <darren(a)os.amperecomputing.com>
---
arch/arm64/kernel/smp.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 27df5c1e6baa..0a78ac5c8830 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -715,9 +715,22 @@ void __init smp_init_cpus(void)
}
}
+static struct sched_domain_topology_level arm64_no_mc_topology[] = {
+#ifdef CONFIG_SCHED_SMT
+ { cpu_smt_mask, cpu_smt_flags, SD_INIT_NAME(SMT) },
+#endif
+
+#ifdef CONFIG_SCHED_CLUSTER
+ { cpu_clustergroup_mask, cpu_cluster_flags, SD_INIT_NAME(CLS) },
+#endif
+ { cpu_cpu_mask, SD_INIT_NAME(DIE) },
+ { NULL, },
+};
+
void __init smp_prepare_cpus(unsigned int max_cpus)
{
const struct cpu_operations *ops;
+ bool use_no_mc_topology = true;
int err;
unsigned int cpu;
unsigned int this_cpu;
@@ -758,6 +771,25 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
set_cpu_present(cpu, true);
numa_store_cpu_info(cpu);
+
+ /*
+ * Only use no_mc topology if all cpu_coregroup_mask weights=1
+ */
+ if (cpumask_weight(cpu_coregroup_mask(cpu)) > 1)
+ use_no_mc_topology = false;
+ }
+
+ /*
+ * SoCs with no shared processor-side cache will have cpu_coregroup_mask
+ * weights=1. If they also define clusters with cpu_clustergroup_mask
+ * weights > 1, build_sched_domain() will trigger a BUG as the CLS
+ * cpu_mask will not be a subset of MC. It will extend the MC cpu_mask
+ * to match CLS, and later discard the MC level. Avoid the bug by using
+ * a topology without the MC if the cpu_coregroup_mask weights=1.
+ */
+ if (use_no_mc_topology) {
+ pr_info("cpu_coregroup_mask weights=1, skipping MC topology level");
+ set_sched_topology(arm64_no_mc_topology);
}
}
--
2.31.1
Hi,
we have found a bug involving CONFIG_READ_ONLY_THP_FOR_FS=y, introduced in
5.12 by cbd59c48ae2b ("mm/filemap: use head pages in
generic_file_buffered_read")
and apparently fixed in 5.17-rc1 by 6b24ca4a1a8d ("mm: Use multi-index
entries in the page cache")
The latter commit is part of folio rework so likely not stable material, so
it would be nice to have a small fix for e.g. 5.15 LTS. Preferably from
someone who understands xarray :)
The bug was found while building nodejs16, which involves running some
tests, first with a non-stripped node16 binary, and then stripping it. This
has a good chance of the stripped result becoming corrupted and not
executable anymore due to dynamic loader failures. It turns out that while
executed during tests, CONFIG_READ_ONLY_THP_FOR_FS=y allows khugepaged to
collapse the large executable mappings. Then /usr/bin/strip uses read() to
process the binary and triggers a bug introduced in cbd59c48ae2b where if a
read spans two or more collapsed THPs in the page cache, the first one will
be read multiple times instead.
Takashi Iwai has bisected using the nodejs build scenario to commit
cbd59c48ae2b.
I have distilled the scenario to the attached reproducer. There are some
assumptions for it to work:
- the passed path for file it creates/works with should be on a filesystem
such as xfs or ext4 that uses generic_file_buffered_read()
- the kernel should have e6be37b2e7bd ("mm/huge_memory.c: add missing
read-only THP checking in transparent_hugepage_enabled()") otherwise
khugepaged will not recognize the reproducer's mm as thp eligible (it had to
be some other mapping in nodejs that made it still possible to trigger this
during bisect)
- there's a pause to allow khugepaged to do its job, you can increase the
speed as instructed and verify with /proc/pid/smaps and meminfo that the
collapse in page cache has happened
- if the bug is reproduced, the reproducer will complain like this:
mismatch at offset 2097152: read value expected for offset 0 instead of 2097152
I've hacked some printk on top 5.16 (attached debug.patch)
which gives this output:
i=0 page=ffffea0004340000 page_offset=0 uoff=0 bytes=2097152
i=1 page=ffffea0004340000 page_offset=0 uoff=0 bytes=2097152
i=2 page=ffffea0004340000 page_offset=0 uoff=0 bytes=0
i=3 page=ffffea0004340000 page_offset=0 uoff=0 bytes=0
i=4 page=ffffea0004340000 page_offset=0 uoff=0 bytes=0
i=5 page=ffffea0004340000 page_offset=0 uoff=0 bytes=0
i=6 page=ffffea0004340000 page_offset=0 uoff=0 bytes=0
i=7 page=ffffea0004340000 page_offset=0 uoff=0 bytes=0
i=8 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
i=9 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
i=10 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
i=11 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
i=12 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
i=13 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
i=14 page=ffffea0004470000 page_offset=2097152 uoff=0 bytes=0
It seems filemap_get_read_batch() should be returning pages ffffea0004340000
and ffffea0004470000 consecutively in the pvec, but returns the first one 8
times, so it's read twice and then the rest is just skipped over as it's
beyond the requested read size.
I suspect these lines:
xas.xa_index = head->index + thp_nr_pages(head) - 1;
xas.xa_offset = (xas.xa_index >> xas.xa_shift) & XA_CHUNK_MASK;
commit 6b24ca4a1a8d changes those to xas_advance() (introduced one patch
earlier), so some self-contained fix should be possible for prior kernels?
But I don't understand xarray well enough.
My colleagues should be credited for initial analysis of the nodejs build
scenario:
Analyzed-by: Adam Majer <amajer(a)suse.com>
Analyzed-by: Dirk Mueller <dmueller(a)suse.com>
Bisected-by: Takashi Iwai <tiwai(a)suse.de>
Thanks,
Vlastimil