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

[Waiman Long <longman@redhat.com>]

On 8/27/26 5:10 PM, Michael Kelley wrote:
> From: Waiman Long <longman@redhat.com> Sent: Thursday, August 27, 2026 12:38 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 also skipping NUMA node that has no
>> housekeeping CPU in the inner while loop of init_vp_index(). As the outer
>> for loop will only be reached if the housekeeping cpumask isn't empty,
>> a NUMA node with housekeeping CPUs will eventually be found.
>>
>> 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 | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
>> index 89d214dda360..ed121d74d73f 100644
>> --- a/drivers/hv/channel_mgmt.c
>> +++ b/drivers/hv/channel_mgmt.c
>> @@ -752,6 +752,7 @@ static void init_vp_index(struct vmbus_channel *channel)
>>   	u32 i, ncpu = num_online_cpus();
>>   	cpumask_var_t available_mask;
>>   	struct cpumask *allocated_mask;
>> +	const struct cpumask *node_mask;
>>   	const struct cpumask *hk_mask = housekeeping_cpumask(HK_TYPE_MANAGED_IRQ);
>>   	u32 target_cpu;
>>   	int numa_node;
>> @@ -780,14 +781,16 @@ static void init_vp_index(struct vmbus_channel *channel)
>>   				next_numa_node_id = 0;
>>   				continue;
>>   			}
>> -			if (cpumask_empty(cpumask_of_node(numa_node)))
>> +			node_mask = cpumask_of_node(numa_node);
>> +			if (cpumask_empty(node_mask) ||
>> +			    !cpumask_intersects(node_mask, hk_mask))
> The cpumask_empty() test looks to be redundant. The
> cpumask_intersects() test will catch the case where
> node_mask is empty.
>
> Otherwise, I think this looks good as a solution to the core
> problem.
>
> Michael

Yes, I am aware that cpumask_empty() test is redundant and can be 
skipped. I keep it just to make it easier to read. I can certainly drop 
the cpumask_empty() statement.

Cheers,
Longman

>
>>   				continue;
>>   			break;
>>   		}
>>   		allocated_mask = &hv_context.hv_numa_map[numa_node];
>>
>>   retry:
>> -		cpumask_xor(available_mask, allocated_mask, cpumask_of_node(numa_node));
>> +		cpumask_xor(available_mask, allocated_mask, node_mask);
>>   		cpumask_and(available_mask, available_mask, hk_mask);
>>
>>   		if (cpumask_empty(available_mask)) {
>> --
>> 2.55.0
>>