Re: [PATCH v2] sched/topology: Check average distances to remote packages

[Tim Chen <tim.c.chen@linux.intel.com>]

On Wed, 2026-02-25 at 17:32 +0100, Peter Zijlstra wrote:
> On Wed, Feb 25, 2026 at 04:44:09PM +0100, Peter Zijlstra wrote:
> 
> > Yes, so this assumes that all u sized clusters on the trace are similar
> > and 'sane' without verification.
> 
> That gave me an idea; how's this then?

Sorry I was sick for a few days.  Just catching up on this
thread here. I think your patch takes care of both GNR SNC-3 
with 3 compute dies (with non-symmetric remote
distances) and generic SNC-2 with 2 dies (symmetric
distances) very well.

Minor suggestion below for the patch.

Will ask the original GNR teams with the problem to try
it out.

> 
> ---
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 5cd6950ab672..b1e464fd98c0 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -513,33 +513,99 @@ static void __init build_sched_topology(void)
>  }
>  
>  #ifdef CONFIG_NUMA
> -static int sched_avg_remote_distance;
> -static int avg_remote_numa_distance(void)
> +
> +static bool slit_cluster_symmetric(int i, int j, int n)
>  {
> -	int i, j;
> -	int distance, nr_remote, total_distance;
> +	WARN_ON_ONCE((i % n) || (j % n));
>  
> -	if (sched_avg_remote_distance > 0)
> -		return sched_avg_remote_distance;
> -
> -	nr_remote = 0;
> -	total_distance = 0;
> -	for_each_node_state(i, N_CPU) {
> -		for_each_node_state(j, N_CPU) {
> -			distance = node_distance(i, j);
> -
> -			if (distance >= REMOTE_DISTANCE) {
> -				nr_remote++;
> -				total_distance += distance;
> -			}
> +	for (int k = i; k < i + n; k++) {
> +		for (int l = k; l < j + n; l++) {
> +			if (node_distance(k, l) != node_distance(k, l))
> +				return false;
>  		}
>  	}
> -	if (nr_remote)
> -		sched_avg_remote_distance = total_distance / nr_remote;
> -	else
> -		sched_avg_remote_distance = REMOTE_DISTANCE;
>  
> -	return sched_avg_remote_distance;
> +	return true;
> +}
> +
> +static bool slit_cluster_match(int i, int j, int x, int y, int n)

Seems like we only call this function with i==j and x==y (i.e. cluster
at i and cluster at x). Can we simplify?

Thanks.

Tim

> +{
> +	WARN_ON_ONCE((i % n) || (j % n) || (x % n) || (y % n));
> +
> +	for (int k = 0; k < n; k++) {
> +		for (int l = k; l < n; l++) {
> +			if (node_distance(i + k, j + l) != node_distance(x + k, y + l))
> +				return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
> +/*
> + * Find the largest symmetric,repeating cluster in an attempt to identify the
> + * unit size.
> + */
> +static int slit_cluster_size(void)
> +{
> +	int nodes = num_possible_nodes();
> +
> +	/*
> +	 * There are at least 2 packages; so half-nodes is the largest
> +	 * possible unit, go down from that.
> +	 */
> +	for (int u = nodes / 2; u; u--) {
> +		/*
> +		 * If u doesn't divide nodes, it can't be a unit.
> +		 */
> +		if (nodes % u)
> +			continue;
> +
> +		/*
> +		 * Unit must be symmetric,
> +		 */
> +		if (!slit_cluster_symmetric(0, 0, u))
> +			continue;
> +
> +		/*
> +		 * and repeating.
> +		 */
> +		if (slit_cluster_match(0, 0, u, u, u))
> +			return u;
> +	}
> +
> +	return nodes;
> +}
> +
> +static int slit_cluster_distance(int i, int j)
> +{
> +	static int u = 0;
> +	long d = 0;
> +	int x, y;
> +
> +	if (!u)
> +		u = slit_cluster_size();
> +
> +	/*
> +	 * Is this a unit cluster on the trace?
> +	 */
> +	if ((i / u) == (j / u))
> +		return node_distance(i, j);
> +
> +	/*
> +	 * Off-trace cluster, return average of the cluster to force symmetry.
> +	 */
> +	x = i - (i % u);
> +	y = j - (j % u);
> +
> +	for (i = x; i < x + u; i++) {
> +		for (j = y; j < y + u; j++) {
> +			d += node_distance(i, j);
> +			d += node_distance(j, i);
> +		}
> +	}
> +
> +	return d / (2*u*u);
>  }
>  
>  int arch_sched_node_distance(int from, int to)
> @@ -550,8 +616,7 @@ int arch_sched_node_distance(int from, int to)
>  	case INTEL_GRANITERAPIDS_X:
>  	case INTEL_ATOM_DARKMONT_X:
>  
> -		if (!x86_has_numa_in_package || topology_max_packages() == 1 ||
> -		    d < REMOTE_DISTANCE)
> +		if (!x86_has_numa_in_package || topology_max_packages() == 1)
>  			return d;
>  
>  		/*
> @@ -564,19 +629,8 @@ int arch_sched_node_distance(int from, int to)
>  		 * in the remote package in the same sched group.
>  		 * Simplify NUMA domains and avoid extra NUMA levels including
>  		 * different remote NUMA nodes and local nodes.
> -		 *
> -		 * GNR and CWF don't expect systems with more than 2 packages
> -		 * and more than 2 hops between packages. Single average remote
> -		 * distance won't be appropriate if there are more than 2
> -		 * packages as average distance to different remote packages
> -		 * could be different.
>  		 */
> -		WARN_ONCE(topology_max_packages() > 2,
> -			  "sched: Expect only up to 2 packages for GNR or CWF, "
> -			  "but saw %d packages when building sched domains.",
> -			  topology_max_packages());
> -
> -		d = avg_remote_numa_distance();
> +		return slit_cluster_distance(from, to);
>  	}
>  	return d;
>  }