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

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

[Waiman Long]

There is a retry loop in init_vp_index() where the CPUs from a certain
node are stripped out if they have already been in the allocated cpumask
or not in HK_TYPE_MANAGED_IRQ housekeeping cpumask. If there is no
CPU left, the allocated cpumask is ignored and the process is retried
again. However, if the HK_TYPE_MANAGED_IRQ housekeeping cpumask turns
out not to contain any CPU in that particular node, that will become an
infinite retry loop.  This particular problem was reported by sashiko
[1]. This should rarely happen, but we still need to guard against this.

Fix this infinite loop problem by skipping to the next numa node if
the allocated cpumask has already been cleared before. Set target_cpu
to the default VMBUS_CONNECT_CPU instead if the for loop is ending.

Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org [1]
Fixes: 6640b5df1a38 ("Drivers: hv: vmbus: Don't assign VMbus channel interrupts to isolated CPUs")
Signed-off-by: Waiman Long <longman@redhat.com>
---
 drivers/hv/channel_mgmt.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 89d214dda360..e80ac9fb5ed3 100644
--- 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;
+			}
+			continue;	/* Try next numa node */
 		}
 
 		target_cpu = cpumask_first(available_mask);
-- 
2.55.0

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

[sashiko-bot]

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

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

[Michael Kelley]

From: Waiman Long <longman@redhat.com> Sent: Wednesday, August 26, 2026 12:25 PM
> 
> There is a retry loop in init_vp_index() where the CPUs from a certain
> node are stripped out if they have already been in the allocated cpumask
> or not in HK_TYPE_MANAGED_IRQ housekeeping cpumask. If there is no
> CPU left, the allocated cpumask is ignored and the process is retried
> again. However, if the HK_TYPE_MANAGED_IRQ housekeeping cpumask turns
> out not to contain any CPU in that particular node, that will become an
> infinite retry loop.  This particular problem was reported by sashiko
> [1]. This should rarely happen, but we still need to guard against this.
> 
> Fix this infinite loop problem by skipping to the next numa node if
> the allocated cpumask has already been cleared before. Set target_cpu
> to the default VMBUS_CONNECT_CPU instead if the for loop is ending.
> 
> Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org [1]
> Fixes: 6640b5df1a38 ("Drivers: hv: vmbus: Don't assign VMbus channel interrupts to isolated CPUs")
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  drivers/hv/channel_mgmt.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 89d214dda360..e80ac9fb5ed3 100644
> --- 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;
> +			}

As you describe in the commit message, the core problem is a NUMA
node that has no housekeeping CPUs. But rather than add a test
here for that condition, consider that there is a "while (true)" loop a
few lines above that cycles thru the NUMA nodes and already skips
NUMA nodes that have no CPUs. What if that loop was enhanced so
that NUMA nodes with no housekeeping CPUs are skipped? Then all
the existing machinery just works. I think this approach also
addresses the two Sashiko comments.

> +			if (i > ncpu) {
> +				target_cpu = VMBUS_CONNECT_CPU;
> +				break;

Can this condition really happen? Won't the "for" loop always find
a housekeeping CPU in some NUMA node? If you want some
additional robustness in case the "for" loop doesn't pick a
CPU, I think initializing target_cpu to VMBUS_CONNECT_CPU up
in the local variable definitions would be sufficient.

Michael

> +			}
> +			continue;	/* Try next numa node */
>  		}
> 
>  		target_cpu = cpumask_first(available_mask);
> --
> 2.55.0
>