Re: [PATCH v4 1/2] sched: Create architecture specific sched domain distances

[Chen, Yu C]

On 10/1/2025 1:30 AM, Tim Chen wrote:
> On Tue, 2025-09-30 at 10:28 +0800, Chen, Yu C wrote:
> > On 9/30/2025 6:18 AM, Tim Chen wrote:
> > > On Sat, 2025-09-27 at 20:34 +0800, Chen, Yu C wrote:
> > > >
>
> [snip]
>
> > > > If our goal is to figure out whether the arch_sched_node_distance()
> > > > has been overridden, how about the following alias?
> > > >
> > > > int __weak arch_sched_node_distance(int from, int to)
> > > > {
> > > > 	return __node_distance(from, to);
> > > > }
> > > > int arch_sched_node_distance_original(int from, int to) __weak
> > > > __alias(arch_sched_node_distance);
> > > >
> > > > static bool arch_sched_node_distance_is_overridden(void)
> > > > {
> > > > 	return arch_sched_node_distance != arch_sched_node_distance_original;
> > > > }
> > > >
> > > > so arch_sched_node_distance_is_overridden() can replace
> > > > modified_sched_node_distance()
> > >
> > > I think that the alias version will still point to the replaced function and not
> > > the originally defined one.
> > >
> > > How about not using __weak and just explicitly define arch_sched_node_distance
> > > as a function pointer.  Change the code like below.
> >
> > The arch_sched_node_distance_original is defined as __weak, so it
> > should point to the old function even if the function has been
> > overridden. I did a test on a X86 VM and it seems to be so.
> > But using the arch_sched_node_distance as a function point
> > should also be OK.
>
> How about changing the code as follow. I think this change is cleaner.
> I tested it in my VM and works for detecting sched distance substitution.
> Thanks.

Yes, the following change looks good to me.

Thanks,
Chenyu> Tim
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index f25e4402c63e..3dc941258df3 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1897,31 +1897,17 @@ static void init_numa_topology_type(int offline_node)
>    * A NUMA level is created for each unique
>    * arch_sched_node_distance.
>    */
> -static bool __modified_sched_node_dist = true;
> -
> -int __weak arch_sched_node_distance(int from, int to)
> +static int numa_node_dist(int i, int j)
>   {
> -	if (__modified_sched_node_dist)
> -		__modified_sched_node_dist = false;
> -
> -	return node_distance(from, to);
> +	return node_distance(i, j);
>   }
>   
> -static bool modified_sched_node_distance(void)
> -{
> -	/*
> -	 * Call arch_sched_node_distance()
> -	 * to determine if arch_sched_node_distance
> -	 * has been modified from node_distance()
> -	 * to arch specific distance.
> -	 */
> -	arch_sched_node_distance(0, 0);
> -	return __modified_sched_node_dist;
> -}
> +int arch_sched_node_distance(int from, int to)
> +			     __weak __alias(numa_node_dist);
>   
> -static int numa_node_dist(int i, int j)
> +static bool modified_sched_node_distance(void)
>   {
> -	return node_distance(i, j);
> +	return numa_node_dist != arch_sched_node_distance;
>   }
>   
>   static int sched_record_numa_dist(int offline_node, int (*n_dist)(int, int),