Re: [PATCH v2 0/2] cpupower: fix topology array handling

[Ali Ahmet Memis]

On Tue, 4 Aug 2026 14:45:48 -0600 Shuah Khan wrote:
> Did you think about a scenario when the following check will be tru - i.e
> core == -1 is trur?

I went looking for one and could not find it, so that branch may well be
dead. What I checked:

On the architectures using drivers/base/arch_topology.c, reset_cpu_topology()
does start every possible CPU at core_id = -1, but store_cpu_topology()
overwrites it for any CPU that comes up without firmware topology:

	if (cpuid_topo->package_id != -1)
		goto topology_populated;
	cpuid_topo->thread_id = -1;
	cpuid_topo->core_id = cpuid;
	cpuid_topo->package_id = cpu_to_node(cpuid);

and it is called from the bring-up paths, arch/arm64/kernel/smp.c and
arch/riscv/kernel/smpboot.c. On x86 core_id is either derived from the apic
id in arch/x86/kernel/cpu/topology_common.c or set to 0 in smpboot.c, so it
is never negative either.

A CPU with no topology at all does not show up as -1 either. The topology
attribute group is created from a CPU hotplug prepare callback in
drivers/base/topology.c, so a CPU that never comes up has no topology
directory and the read fails outright rather than returning -1.

That last case is the one that matters here, and it takes one of the two
earlier continue branches rather than the one you quoted.

> Can you elaborate on a real scenario where this could happen after
> replacing malloc() with calloc() and making sure core_cpu_list is
> initialized to "-1" like in the above conditional?

Those two branches set pkg and core to -1 and leave core_cpu_list untouched,
so under calloc it stays empty, and the count is still wrong. I ran this
against a fake sysfs tree with the CPU count pinned, cpu0 with real topology
and cpu1 with no topology files at all:

  unpatched            cores=2
  patch 1 only         cores=2
  patch 1 and 2        cores=1

and with three CPUs, cpu0 and cpu1 real and cpu2 unreadable:

  patch 1 only         cores=3
  patch 1 and 2        cores=2

The reason is the seed, not the buffer contents:

	last_cpu_list = cpu_top->core_info[0].core_cpu_list;
	cpu_top->cores = 1;

An empty string and "-1" both sort ahead of a real cpu list, so after the
qsort entry 0 is an incomplete one, and cores is seeded to 1 from it without
ever looking at pkg. The pkg != -1 check inside the loop only guards the
entries that follow, never the one the count started from. That is why
initializing the buffer to "-1" does not help: it changes what entry 0
contains, not the fact that it is counted.

One consequence worth stating rather than leaving for you to find. If no CPU
has usable topology at all, the count changes:

  patch 1 only         cores=1
  patch 1 and 2        cores=0

That direction looks like the consistent one rather than a regression, since
pkgs already reports 0 in that case today, so the current code prints
"Packages: 0 - Cores: 1" and after patch 2 it prints "Packages: 0 -
Cores: 0". The only in-tree consumer of cores is the dprint() in
cpupower-monitor.c, so nothing there divides by it or sizes an allocation
with it, but cores is in the installed cpupower.h so I cannot speak for
out-of-tree users of the library.