lkml.org 
[lkml]   [2023]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2023-03-27 00:53    [W:3.795 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site