Re: [PATCH v2] Drivers: hv: Avoid infinite retry loop in init_vp_index()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incrementing the bounded loop counter when skipping empty NUMA nodes causes premature search termination, falling back to CPU 0.
- [Medium] When the loop falls back to VMBUS_CONNECT_CPU, it fails to register this allocation in the hv_numa_map, breaking the round-robin distribution tracker.
--

commit 5b552e920b17d64534cf7a69fd8a78145809498a
Author: Waiman Long <longman@redhat.com>
Drivers: hv: Avoid infinite retry loop in init_vp_index()

This commit fixes an infinite retry loop in init_vp_index() that occurs when
the housekeeping cpumask for managed IRQs contains no CPUs in a particular
NUMA node. It resolves this by skipping to the next node if the allocated
cpumask is already cleared, and falling back to VMBUS_CONNECT_CPU if the loop
is about to end.

Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org

> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -793,10 +793,20 @@ static void init_vp_index(struct vmbus_channel *channel)
>  		if (cpumask_empty(available_mask)) {
>  			/*
>  			 * We have cycled through all the CPUs in the node;
> -			 * reset the allocated map.
> +			 * reset the allocated map. If the allocated map
> +			 * has already been cleared, we will try the next numa
> +			 * node. Set target_cpu to the default VMBUS_CONNECT_CPU
> +			 * instead if the for loop is going to end.
>  			 */
> -			cpumask_clear(allocated_mask);
> -			goto retry;
> +			if (!cpumask_empty(allocated_mask)) {
> +				cpumask_clear(allocated_mask);
> +				goto retry;
> +			}
> +			if (i > ncpu) {
> +				target_cpu = VMBUS_CONNECT_CPU;
> +				break;

[Severity: Medium]
When the loop falls back to VMBUS_CONNECT_CPU here, does this bypass updating
the round-robin allocation tracker? 

The fallback block earlier in init_vp_index() correctly calls
hv_set_allocated_cpu(VMBUS_CONNECT_CPU) to maintain the global allocation
state:

    channel->target_cpu = VMBUS_CONNECT_CPU;
    if (perf_chn)
        hv_set_allocated_cpu(VMBUS_CONNECT_CPU);
    return;

Could breaking out directly here without making the corresponding state update
cause a load imbalance by stacking extra channels on CPU 0 without the
balancer knowing?

> +			}
> +			continue;	/* Try next numa node */

[Severity: High]
Does this continue statement prematurely exhaust the loop iterations when
encountering nodes with no housekeeping CPUs?

Since the loop counter i limits the total search across ncpu + 1 iterations,
incrementing it while skipping an empty NUMA node burns through the search
budget without actually evaluating any CPUs.

For example, in a topology with several isolated NUMA nodes, could skipping
these empty nodes cause the loop budget to run out early, incorrectly forcing
a fallback to VMBUS_CONNECT_CPU before checking all valid housekeeping CPUs?

>  		}
>  
>  		target_cpu = cpumask_first(available_mask);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826192509.529838-1-longman@redhat.com?part=1