Messages in this thread | | | Date | Fri, 10 Mar 2023 09:30:54 -0800 | From | Tony Luck <> | Subject | Re: [PATCH 7/7] x86/resctrl: Determine if Sub-NUMA Cluster is enabled and initialize. |
| |
On Mon, Feb 27, 2023 at 02:30:38PM +0100, Peter Newman wrote: > Hi Tony, > > On Thu, Jan 26, 2023 at 7:42 PM Tony Luck <tony.luck@intel.com> wrote: > > +static __init int find_snc_ways(void) > > +{ > > + unsigned long *node_caches; > > + int cpu, node, ret; > > + > > + node_caches = kcalloc(BITS_TO_LONGS(nr_node_ids), sizeof(*node_caches), GFP_KERNEL); > > + if (!node_caches) > > + return 1; > > + > > + cpus_read_lock(); > > + for_each_node(node) { > > Someone tried this patch on a machine with a CPU-less node... > > We need to check for this: > > + if (cpumask_empty(cpumask_of_node(node))) > + continue; > > > + cpu = cpumask_first(cpumask_of_node(node)); > > + set_bit(get_cpu_cacheinfo_id(cpu, 3), node_caches); > > + } > > + cpus_read_unlock();
Peter,
Tell me more about your CPU-less nodes. Your fix avoids a bad pointer reference (because cpumask_first() returns cpu >= nr_cpu_ids for an empty bitmask).
But now I'm worried about whether I have the right values in the formula:
nr_node_ids / bitmap_weight(node_caches, nr_node_ids);
This fix avoids counting the L3 from a non-existent CPU, but still counts the node in the numerator.
Is your CPU-less node a full (non-SNC) node? Like this:
Socket 0 Socket 1 +--------------------+ +--------------------+ | . | | . | | SNC 0.0 . SNC 0.1 | | zero . zero | | . | | CPUs . CPUs | | . | | . | | . | | . | +--------------------+ +--------------------+ | L3 Cache | | L3 Cache | +--------------------+ +--------------------+
I could fix this case by counting how many CPU-less nodes I find, and reducing the numerator (the denominator didn't count the L3 cache from socket 1 because there are no CPUs there)
(nr_node_ids - n_empty_nodes) / bitmap_weight(node_caches, nr_node_ids);
=> 2 / 1
But that won't work if your CPU-less node is an SNC node and the other SNC node in the same socket does have some CPUs:
Socket 0 Socket 1 +--------------------+ +--------------------+ | . | | . | | SNC 0.0 . SNC 0.1 | | zero . SNC 1.1 | | . | | CPUs . | | . | | . | | . | | . | +--------------------+ +--------------------+ | L3 Cache | | L3 Cache | +--------------------+ +--------------------+
This would get 3 / 2 ... i.e. I should still count the empty node because its cache was counted by its SNC buddy.
-Tony
| |