Date: Mon, 7 Jan 2019 12:58:48 -0200 From: Leandro <leandro.lupori@gmail.com> To: rgrimes@freebsd.org Cc: Conrad Meyer <cem@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r342771 - in head: share/man/man4 sys/kern sys/powerpc/powernv sys/powerpc/powerpc sys/powerpc/ps3 sys/powerpc/pseries sys/sys sys/x86/x86 Message-ID: <CAC7XEcKwfh5wOqr7Vg51YLm_w3_5ad4Q9_F%2BZta%2BUOtfT6C-Pw@mail.gmail.com> In-Reply-To: <201901041907.x04J7Rse084674@pdx.rh.CN85.dnsmgr.net> References: <201901041831.x04IVHa5070991@repo.freebsd.org> <201901041907.x04J7Rse084674@pdx.rh.CN85.dnsmgr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello, This change makes the kernel unbootable on POWER8: panic: Duplicate children in 0xc0000000021d0490. mask (ffffffffffffffff,ffff,0,0) child (ff,0,0,0) cpuid = 0 time = 1 KDB: stack backtrace: 0xe000000000008270: at .kdb_backtrace+0x5c 0xe0000000000083a0: at .vpanic+0x1b4 0xe000000000008460: at .panic+0x38 0xe0000000000084f0: at .topo_init_root+0x1d0 0xe000000000008640: at .smp_topo_1level+0x8c 0xe0000000000086f0: at .opal_call+0x408 0xe000000000008790: at .cpu_topo+0x6c 0xe000000000008810: at .smp_topo+0x11c 0xe000000000008940: at .schedinit+0x80 0xe0000000000089e0: at .mi_startup+0x1f8 0xe000000000008a80: at .__start+0xc4 The panic is due to the new call to cpu_topo(), at sys/powerpc/powerpc/mp_machdep.c. If the call is commented, everything works fine. Also, the related values displayed by sysctl seem correct: kern.smp.cpus: 80 kern.smp.threads_per_core: 8 kern.smp.cores: 10 So, my hunch is that cpu_topo() or the functions called by it are already being called elsewhere, and the new call is creating duplicated nodes, causing the panic. Is the call to cpu_topo() really needed? Thanks, Leandro On Fri, Jan 4, 2019 at 5:07 PM Rodney W. Grimes <freebsd@pdx.rh.cn85.dnsmgr.net> wrote: > > > Author: cem > > Date: Fri Jan 4 18:31:17 2019 > > New Revision: 342771 > > URL: https://svnweb.freebsd.org/changeset/base/342771 > > > > Log: > > Expose threads-per-core and physical core count information > > > > With new sysctls (to the best of our ability do detect them). Restructured > > smp.4 slightly for clarity (keep relevant stuff closer to the top) while > > documenting. > > > > Reviewed by: markj, jhibbits (ppc parts) > > MFC after: 3 days > > Sponsored by: Dell EMC Isilon > > Differential Revision: https://reviews.freebsd.org/D18322 > > > > Modified: > > head/share/man/man4/smp.4 > > head/sys/kern/subr_smp.c > > head/sys/powerpc/powernv/platform_powernv.c > > head/sys/powerpc/powerpc/mp_machdep.c > > head/sys/powerpc/ps3/platform_ps3.c > > head/sys/powerpc/pseries/platform_chrp.c > > head/sys/sys/smp.h > > head/sys/x86/x86/mp_x86.c > > Can you please alter this to match how topology is expressed > by most tools, > Sockets, > Cores per socket, > Threads per core. > > ncpu = S * C * T > > This matches how bhyve, qemu, kvm and others express > this information. > > Thanks, > Rod > > > Modified: head/share/man/man4/smp.4 > > ============================================================================== > > --- head/share/man/man4/smp.4 Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/share/man/man4/smp.4 Fri Jan 4 18:31:17 2019 (r342771) > > @@ -23,7 +23,7 @@ > > .\" > > .\" $FreeBSD$ > > .\" > > -.Dd January 6, 2018 > > +.Dd January 4, 2019 > > .Dt SMP 4 > > .Os > > .Sh NAME > > @@ -35,27 +35,6 @@ > > The > > .Nm > > kernel implements symmetric multi-processor support. > > -.Sh COMPATIBILITY > > -Support for multi-processor systems is present for all Tier-1 > > -architectures on > > -.Fx . > > -Currently, this includes amd64, i386 and sparc64. > > -Support is enabled using > > -.Cd options SMP . > > -It is permissible to use the SMP kernel configuration on non-SMP equipped > > -motherboards. > > -.Sh I386 NOTES > > -For i386 systems, the > > -.Nm > > -kernel supports motherboards that follow the Intel MP specification, > > -version 1.4. > > -In addition to > > -.Cd options SMP , > > -i386 also requires > > -.Cd device apic . > > -The > > -.Xr mptable 1 > > -command may be used to view the status of multi-processor support. > > .Pp > > .Nm > > support can be disabled by setting the loader tunable > > @@ -66,6 +45,13 @@ The number of CPUs detected by the system is available > > the read-only sysctl variable > > .Va hw.ncpu . > > .Pp > > +The number of online threads per CPU core is available in the read-only sysctl > > +variable > > +.Va kern.smp.threads_per_core . > > +The number of physical CPU cores detected by the system is available in the > > +read-only sysctl variable > > +.Va kern.smp.cores . > > +.Pp > > .Fx > > allows specific CPUs on a multi-processor system to be disabled. > > This can be done using the > > @@ -74,6 +60,12 @@ tunable, where X is the APIC ID of a CPU. > > Setting this tunable to 1 will result in the corresponding CPU being > > disabled. > > .Pp > > +.Fx > > +supports simultaneous multithreading on x86 and powerpc platforms. > > +On x86, the logical CPUs can be disabled by setting the > > +.Va machdep.hyperthreading_allowed > > +tunable to zero. > > +.Pp > > The > > .Xr sched_ule 4 > > scheduler implements CPU topology detection and adjusts the scheduling > > @@ -122,13 +114,26 @@ two quad-core processors is: > > .Pp > > This information is used internally by the kernel to schedule related > > tasks on CPUs that are closely grouped together. > > -.Pp > > -.Fx > > -supports hyperthreading on Intel CPU's on the i386 and AMD64 platforms. > > -Because using logical CPUs can cause performance penalties under certain loads, > > -the logical CPUs can be disabled by setting the > > -.Va machdep.hyperthreading_allowed > > -tunable to zero. > > +.Sh COMPATIBILITY > > +Support for multi-processor systems is present for all Tier-1 and Tier-2 > > +architectures on > > +.Fx . > > +Currently, this includes x86, powerpc, arm, and sparc64. > > +Support is enabled using > > +.Cd options SMP . > > +It is permissible to use the SMP kernel configuration on non-SMP hardware. > > +.Sh I386 NOTES > > +For i386 systems, the > > +.Nm > > +kernel supports motherboards that follow the Intel MP specification, > > +version 1.4. > > +In addition to > > +.Cd options SMP , > > +i386 also requires > > +.Cd device apic . > > +The > > +.Xr mptable 1 > > +command may be used to view the status of multi-processor support. > > .Sh SEE ALSO > > .Xr cpuset 1 , > > .Xr mptable 1 , > > @@ -166,3 +171,20 @@ in > > also introduced support for SMP on the sparc64 architecture. > > .Sh AUTHORS > > .An Steve Passe Aq Mt fsmp@FreeBSD.org > > +.Sh CAVEATS > > +The > > +.Va kern.smp.threads_per_core > > +and > > +.Va kern.smp.cores > > +sysctl variables are provided as a best-effort guess. > > +If an architecture or platform adds SMT and > > +.Fx > > +has not yet implemented detection, the reported values may be inaccurate. > > +In this case, > > +.Va kern.smp.threads_per_core > > +will report > > +.Dv 1 > > +and > > +.Va kern.smp.cores > > +will report the same value as > > +.Va hw.ncpu . > > > > Modified: head/sys/kern/subr_smp.c > > ============================================================================== > > --- head/sys/kern/subr_smp.c Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/kern/subr_smp.c Fri Jan 4 18:31:17 2019 (r342771) > > @@ -98,6 +98,14 @@ int smp_cpus = 1; /* how many cpu's running */ > > SYSCTL_INT(_kern_smp, OID_AUTO, cpus, CTLFLAG_RD|CTLFLAG_CAPRD, &smp_cpus, 0, > > "Number of CPUs online"); > > > > +int smp_threads_per_core = 1; /* how many SMT threads are running per core */ > > +SYSCTL_INT(_kern_smp, OID_AUTO, threads_per_core, CTLFLAG_RD|CTLFLAG_CAPRD, > > + &smp_threads_per_core, 0, "Number of SMT threads online per core"); > > + > > +int mp_ncores = -1; /* how many physical cores running */ > > +SYSCTL_INT(_kern_smp, OID_AUTO, cores, CTLFLAG_RD|CTLFLAG_CAPRD, &mp_ncores, 0, > > + "Number of CPUs online"); > > + > > int smp_topology = 0; /* Which topology we're using. */ > > SYSCTL_INT(_kern_smp, OID_AUTO, topology, CTLFLAG_RDTUN, &smp_topology, 0, > > "Topology override setting; 0 is default provided by hardware."); > > @@ -154,6 +162,7 @@ mp_start(void *dummy) > > > > /* Probe for MP hardware. */ > > if (smp_disabled != 0 || cpu_mp_probe() == 0) { > > + mp_ncores = 1; > > mp_ncpus = 1; > > CPU_SETOF(PCPU_GET(cpuid), &all_cpus); > > return; > > @@ -162,6 +171,11 @@ mp_start(void *dummy) > > cpu_mp_start(); > > printf("FreeBSD/SMP: Multiprocessor System Detected: %d CPUs\n", > > mp_ncpus); > > + > > + /* Provide a default for most architectures that don't have SMT/HTT. */ > > + if (mp_ncores < 0) > > + mp_ncores = mp_ncpus; > > + > > cpu_mp_announce(); > > } > > SYSINIT(cpu_mp, SI_SUB_CPU, SI_ORDER_THIRD, mp_start, NULL); > > @@ -823,6 +837,7 @@ static void > > mp_setvariables_for_up(void *dummy) > > { > > mp_ncpus = 1; > > + mp_ncores = 1; > > mp_maxid = PCPU_GET(cpuid); > > CPU_SETOF(mp_maxid, &all_cpus); > > KASSERT(PCPU_GET(cpuid) == 0, ("UP must have a CPU ID of zero")); > > > > Modified: head/sys/powerpc/powernv/platform_powernv.c > > ============================================================================== > > --- head/sys/powerpc/powernv/platform_powernv.c Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/powerpc/powernv/platform_powernv.c Fri Jan 4 18:31:17 2019 (r342771) > > @@ -435,12 +435,16 @@ powernv_smp_topo(platform_t plat) > > break; > > } > > > > + smp_threads_per_core = nthreads; > > + > > if (mp_ncpus % nthreads != 0) { > > printf("WARNING: Irregular SMP topology. Performance may be " > > "suboptimal (%d threads, %d on first core)\n", > > mp_ncpus, nthreads); > > return (smp_topo_none()); > > } > > + > > + mp_ncores = mp_ncpus / nthreads; > > > > /* Don't do anything fancier for non-threaded SMP */ > > if (nthreads == 1) > > > > Modified: head/sys/powerpc/powerpc/mp_machdep.c > > ============================================================================== > > --- head/sys/powerpc/powerpc/mp_machdep.c Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/powerpc/powerpc/mp_machdep.c Fri Jan 4 18:31:17 2019 (r342771) > > @@ -186,6 +186,11 @@ cpu_mp_start(void) > > next: > > error = platform_smp_next_cpu(&cpu); > > } > > + > > +#ifdef SMP > > + /* Probe mp_ncores and smp_threads_per_core as a side effect. */ > > + (void)cpu_topo(); > > +#endif > > } > > > > void > > > > Modified: head/sys/powerpc/ps3/platform_ps3.c > > ============================================================================== > > --- head/sys/powerpc/ps3/platform_ps3.c Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/powerpc/ps3/platform_ps3.c Fri Jan 4 18:31:17 2019 (r342771) > > @@ -246,6 +246,8 @@ ps3_smp_start_cpu(platform_t plat, struct pcpu *pc) > > static struct cpu_group * > > ps3_smp_topo(platform_t plat) > > { > > + mp_ncores = 1; > > + smp_threads_per_core = 2; > > return (smp_topo_1level(CG_SHARE_L1, 2, CG_FLAG_SMT)); > > } > > #endif > > > > Modified: head/sys/powerpc/pseries/platform_chrp.c > > ============================================================================== > > --- head/sys/powerpc/pseries/platform_chrp.c Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/powerpc/pseries/platform_chrp.c Fri Jan 4 18:31:17 2019 (r342771) > > @@ -517,6 +517,8 @@ chrp_smp_topo(platform_t plat) > > ncpus++; > > } > > > > + mp_ncores = ncores; > > + > > if (ncpus % ncores != 0) { > > printf("WARNING: Irregular SMP topology. Performance may be " > > "suboptimal (%d CPUS, %d cores)\n", ncpus, ncores); > > @@ -527,6 +529,7 @@ chrp_smp_topo(platform_t plat) > > if (ncpus == ncores) > > return (smp_topo_none()); > > > > + smp_threads_per_core = ncpus / ncores; > > return (smp_topo_1level(CG_SHARE_L1, ncpus / ncores, CG_FLAG_SMT)); > > } > > #endif > > > > Modified: head/sys/sys/smp.h > > ============================================================================== > > --- head/sys/sys/smp.h Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/sys/smp.h Fri Jan 4 18:31:17 2019 (r342771) > > @@ -167,8 +167,10 @@ extern cpuset_t logical_cpus_mask; > > > > extern u_int mp_maxid; > > extern int mp_maxcpus; > > +extern int mp_ncores; > > extern int mp_ncpus; > > extern volatile int smp_started; > > +extern int smp_threads_per_core; > > > > extern cpuset_t all_cpus; > > extern cpuset_t cpuset_domain[MAXMEMDOM]; /* CPUs in each NUMA domain. */ > > > > Modified: head/sys/x86/x86/mp_x86.c > > ============================================================================== > > --- head/sys/x86/x86/mp_x86.c Fri Jan 4 18:21:49 2019 (r342770) > > +++ head/sys/x86/x86/mp_x86.c Fri Jan 4 18:31:17 2019 (r342771) > > @@ -607,6 +607,7 @@ assign_cpu_ids(void) > > { > > struct topo_node *node; > > u_int smt_mask; > > + int nhyper; > > > > smt_mask = (1u << core_id_shift) - 1; > > > > @@ -615,6 +616,7 @@ assign_cpu_ids(void) > > * beyond MAXCPU. CPU 0 is always assigned to the BSP. > > */ > > mp_ncpus = 0; > > + nhyper = 0; > > TOPO_FOREACH(node, &topo_root) { > > if (node->type != TOPO_TYPE_PU) > > continue; > > @@ -642,6 +644,9 @@ assign_cpu_ids(void) > > continue; > > } > > > > + if (cpu_info[node->hwid].cpu_hyperthread) > > + nhyper++; > > + > > cpu_apic_ids[mp_ncpus] = node->hwid; > > apic_cpuids[node->hwid] = mp_ncpus; > > topo_set_pu_id(node, mp_ncpus); > > @@ -651,6 +656,9 @@ assign_cpu_ids(void) > > KASSERT(mp_maxid >= mp_ncpus - 1, > > ("%s: counters out of sync: max %d, count %d", __func__, mp_maxid, > > mp_ncpus)); > > + > > + mp_ncores = mp_ncpus - nhyper; > > + smp_threads_per_core = mp_ncpus / mp_ncores; > > } > > > > /* > > > > > > -- > Rod Grimes rgrimes@freebsd.org >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAC7XEcKwfh5wOqr7Vg51YLm_w3_5ad4Q9_F%2BZta%2BUOtfT6C-Pw>