Fix CPU topology parsing, add UTs

src/fibers/cpu.cpp

@@ -45,38 +45,70 @@ static int readSysfsUint32(const char * path, uint32_t * out) noexcept
     return 0;
 }
 
-static bool cpuInCpulist(uint32_t cpu, const char * list) noexcept
+// Parse a non-negative decimal integer at *p, advancing p past the digits.
+// Returns the parsed value, or 0 if no digits are present.
+static uint32_t parseUint32(const char *& p) noexcept
 {
-    const char * p = list;
-    while (*p && *p != '\n')
+    uint32_t value = 0;
+    while (*p >= '0' && *p <= '9')
     {
-        uint32_t start = 0;
-        while (*p >= '0' && *p <= '9')
-        {
-            start = start * 10 + static_cast<uint32_t>(*p++ - '0');
-        }
-        uint32_t end = start;
-        if (*p == '-')
+        value = value * 10 + static_cast<uint32_t>(*p++ - '0');
+    }
+    return value;
+}
+
+// Parse a single Linux cpulist entry "[a[-b[:c[/d]]]]" and return whether @p cpu
+// is included. The kernel's full grammar (see bitmap_parselist) selects bit
+// positions in [a, b] for which (pos - a) % d < c -- the stride form is rare
+// outside of cgroup-restricted layouts but is valid sysfs output.
+static bool cpuInCpulistEntry(uint32_t cpu, const char *& p) noexcept
+{
+    uint32_t start = parseUint32(p);
+    uint32_t end = start;
+    if (*p == '-')
+    {
+        ++p;
+        end = parseUint32(p);
+    }
+
+    // Optional stride: ":used[/group]". used defaults to 1 (every position),
+    // group defaults to used (a:c is shorthand for a:c/c, which selects all
+    // positions in the range).
+    uint32_t used = 1;
+    uint32_t group = 1;
+    if (*p == ':')
+    {
+        ++p;
+        used = parseUint32(p);
+        group = used;
+        if (*p == '/')
         {
             ++p;
-            end = 0;
-            while (*p >= '0' && *p <= '9')
-            {
-                end = end * 10 + static_cast<uint32_t>(*p++ - '0');
-            }
+            group = parseUint32(p);
         }
-        if (cpu >= start && cpu <= end)
+    }
+
+    if (cpu < start || cpu > end || group == 0)
+    {
+        return false;
+    }
+    return (cpu - start) % group < used;
+}
+
+bool cpuInCpulist(uint32_t cpu, const char * list) noexcept
+{
+    const char * p = list;
+    while (*p && *p != '\n')
+    {
+        if (cpuInCpulistEntry(cpu, p))
         {
             return true;
         }
-        if (*p == ',')
-        {
-            ++p;
-        }
-        else
+        if (*p != ',')
         {
             break;
         }
+        ++p;
     }
     return false;
 }
@@ -93,7 +125,10 @@ void readCpuTopologies(CpuTopology * topologies, uint32_t processorCount) noexce
         ::snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%u/topology/core_id", cpu);
         readSysfsUint32(path, &topologies[cpu].coreId);
 
-        topologies[cpu].numaNodeId = 0;
+        // numaNodeId stays at UINT32_MAX until the NUMA pass below sets it.
+        // In environments without NUMA sysfs (containers, minimal kernels) it
+        // remains UINT32_MAX so topologyCostCycles falls through to the safe
+        // cross-NUMA cost rather than misclassifying every CPU as same-node.
     }
 
     // Open each NUMA node file once and fill all CPUs that belong to it.
@@ -137,7 +172,10 @@ uint64_t topologyCostCycles(const CpuTopology & first, const CpuTopology & secon
         // HT sibling ~1 us
         return Tsc::nanosecondsToCycles(1'000);
     }
-    if (first.numaNodeId == second.numaNodeId)
+    // Treat unknown NUMA (UINT32_MAX, e.g. containers without /sys/devices/system/node)
+    // as cross-NUMA: the equality check below would otherwise classify two unknown
+    // CPUs as same-node.
+    if (first.numaNodeId != UINT32_MAX && first.numaNodeId == second.numaNodeId)
     {
         // same NUMA ~50 us
         return Tsc::nanosecondsToCycles(50'000);

src/fibers/cpu.h

@@ -24,4 +24,11 @@ void readCpuTopologies(CpuTopology * topologies, uint32_t processorCount) noexce
  */
 uint64_t topologyCostCycles(const CpuTopology & first, const CpuTopology & second) noexcept;
 
+/**
+ * Test whether @p cpu is in a Linux cpulist string. Accepts the kernel grammar
+ * "[a[-b[:c[/d]]]](,...)" with optional stride: positions in [a, b] for which
+ * (pos - a) % d < c. Defaults are b=a, c=1, d=c. Exposed for unit tests.
+ */
+bool cpuInCpulist(uint32_t cpu, const char * list) noexcept;
+
 } // namespace silk

src/fibers/tests/cpu-test.cpp

@@ -65,7 +65,90 @@ TEST(CpuTopology, crossNumaCost)
     EXPECT_EQ(topologyCostCycles(a, b), Tsc::nanosecondsToCycles(500'000));
 }
 
-// readCpuTopologies: smoke test — must not crash and should populate at least
+// Container-style: package and core known but no NUMA sysfs, so numaNodeId
+// stays at the default UINT32_MAX. Both ends are "unknown NUMA"; the cost
+// must fall through to cross-NUMA, not classify them as same-node by virtue
+// of UINT32_MAX == UINT32_MAX.
+TEST(CpuTopology, unknownNumaReturnsCrossNumaCost)
+{
+    CpuTopology a{0, 0, UINT32_MAX};
+    CpuTopology b{1, 0, UINT32_MAX};
+    EXPECT_EQ(topologyCostCycles(a, b), Tsc::nanosecondsToCycles(500'000));
+}
+
+// cpuInCpulist: single CPU.
+TEST(CpuTopology, cpuInCpulistSingle)
+{
+    EXPECT_TRUE(cpuInCpulist(5, "5"));
+    EXPECT_FALSE(cpuInCpulist(4, "5"));
+    EXPECT_FALSE(cpuInCpulist(6, "5"));
+}
+
+// cpuInCpulist: contiguous range.
+TEST(CpuTopology, cpuInCpulistRange)
+{
+    EXPECT_TRUE(cpuInCpulist(0, "0-7"));
+    EXPECT_TRUE(cpuInCpulist(7, "0-7"));
+    EXPECT_TRUE(cpuInCpulist(3, "0-7"));
+    EXPECT_FALSE(cpuInCpulist(8, "0-7"));
+}
+
+// cpuInCpulist: comma-separated entries.
+TEST(CpuTopology, cpuInCpulistMultiple)
+{
+    EXPECT_TRUE(cpuInCpulist(1, "0-3,5,7-9"));
+    EXPECT_TRUE(cpuInCpulist(5, "0-3,5,7-9"));
+    EXPECT_TRUE(cpuInCpulist(9, "0-3,5,7-9"));
+    EXPECT_FALSE(cpuInCpulist(4, "0-3,5,7-9"));
+    EXPECT_FALSE(cpuInCpulist(6, "0-3,5,7-9"));
+    EXPECT_FALSE(cpuInCpulist(10, "0-3,5,7-9"));
+}
+
+// cpuInCpulist: stride syntax. "0-7:2/4" means positions in [0,7] for which
+// (pos - 0) % 4 < 2 -- i.e. {0,1,4,5}.
+TEST(CpuTopology, cpuInCpulistStride)
+{
+    EXPECT_TRUE(cpuInCpulist(0, "0-7:2/4"));
+    EXPECT_TRUE(cpuInCpulist(1, "0-7:2/4"));
+    EXPECT_FALSE(cpuInCpulist(2, "0-7:2/4"));
+    EXPECT_FALSE(cpuInCpulist(3, "0-7:2/4"));
+    EXPECT_TRUE(cpuInCpulist(4, "0-7:2/4"));
+    EXPECT_TRUE(cpuInCpulist(5, "0-7:2/4"));
+    EXPECT_FALSE(cpuInCpulist(6, "0-7:2/4"));
+    EXPECT_FALSE(cpuInCpulist(7, "0-7:2/4"));
+    EXPECT_FALSE(cpuInCpulist(8, "0-7:2/4"));
+}
+
+// cpuInCpulist: ":c" without "/d" defaults d=c, so all positions in the range
+// are selected (kernel shorthand for the no-stride case).
+TEST(CpuTopology, cpuInCpulistStrideDefaultsGroupToUsed)
+{
+    EXPECT_TRUE(cpuInCpulist(0, "0-7:2"));
+    EXPECT_TRUE(cpuInCpulist(3, "0-7:2"));
+    EXPECT_TRUE(cpuInCpulist(7, "0-7:2"));
+    EXPECT_FALSE(cpuInCpulist(8, "0-7:2"));
+}
+
+// cpuInCpulist: stride combined with comma list.
+TEST(CpuTopology, cpuInCpulistStrideMixed)
+{
+    // "0-3,8-15:1/2": {0,1,2,3} ∪ {8,10,12,14}
+    EXPECT_TRUE(cpuInCpulist(2, "0-3,8-15:1/2"));
+    EXPECT_FALSE(cpuInCpulist(4, "0-3,8-15:1/2"));
+    EXPECT_TRUE(cpuInCpulist(8, "0-3,8-15:1/2"));
+    EXPECT_FALSE(cpuInCpulist(9, "0-3,8-15:1/2"));
+    EXPECT_TRUE(cpuInCpulist(14, "0-3,8-15:1/2"));
+    EXPECT_FALSE(cpuInCpulist(15, "0-3,8-15:1/2"));
+}
+
+// cpuInCpulist: trailing newline must be tolerated (sysfs reads include it).
+TEST(CpuTopology, cpuInCpulistTrailingNewline)
+{
+    EXPECT_TRUE(cpuInCpulist(3, "0-7\n"));
+    EXPECT_FALSE(cpuInCpulist(8, "0-7\n"));
+}
+
+// readCpuTopologies: smoke test -- must not crash and should populate at least
 // one CPU with a valid package ID on a real Linux system.
 TEST(CpuTopology, readDoesNotCrash)
 {