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

[Waiman Long <longman@redhat.com>]

On 8/26/26 1:37 PM, Waiman Long wrote:
> 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 ignoring the HK_TYPE_MANAGED_IRQ
> housekeeping cpumask if the allocated cpumask has already been
> cleared before. Since the HK_TYPE_MANAGED_IRQ housekeeping cpumask
> is supposed to be used on a best effort basis, it is OK to ignore it
> in this particular case. Also add a check_hkcpu boolean flag in struct
> vmbus_channel to control the HK_TYPE_MANAGED_IRQ housekeeping CPU check
> in target_cpu_store() and init_vp_index().
>
> 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>

Please ignore this patch. I have sent out a v2 after reviewing feedback 
from sashiko.

Cheers,
Longman

> ---
>   drivers/hv/channel_mgmt.c | 14 +++++++++++---
>   drivers/hv/vmbus_drv.c    |  3 ++-
>   include/linux/hyperv.h    |  7 +++++++
>   3 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 89d214dda360..30d91668e1c5 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -774,6 +774,7 @@ static void init_vp_index(struct vmbus_channel *channel)
>   	}
>   
>   	for (i = 1; i <= ncpu + 1; i++) {
> +		channel->check_hkcpu = true;
>   		while (true) {
>   			numa_node = next_numa_node_id++;
>   			if (numa_node == nr_node_ids) {
> @@ -788,14 +789,21 @@ static void init_vp_index(struct vmbus_channel *channel)
>   
>   retry:
>   		cpumask_xor(available_mask, allocated_mask, cpumask_of_node(numa_node));
> -		cpumask_and(available_mask, available_mask, hk_mask);
> +		if (channel->check_hkcpu)
> +			cpumask_and(available_mask, available_mask, hk_mask);
>   
>   		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 have to ignore the
> +			 * HK_TYPE_MANAGED_IRQ housekeeping cpumask as its use
> +			 * is on a best effort basis, not a must.
>   			 */
> -			cpumask_clear(allocated_mask);
> +			if (!cpumask_empty(allocated_mask))
> +				cpumask_clear(allocated_mask);
> +			else
> +				channel->check_hkcpu = false;
>   			goto retry;
>   		}
>   
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 6824bd7cb3c4..bea578cd0aa7 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1751,7 +1751,8 @@ int vmbus_channel_set_cpu(struct vmbus_channel *channel, u32 target_cpu)
>   	if (target_cpu >= nr_cpumask_bits)
>   		return -EINVAL;
>   
> -	if (!cpumask_test_cpu(target_cpu, housekeeping_cpumask(HK_TYPE_MANAGED_IRQ)))
> +	if (channel->check_hkcpu &&
> +	    !cpumask_test_cpu(target_cpu, housekeeping_cpumask(HK_TYPE_MANAGED_IRQ)))
>   		return -EINVAL;
>   
>   	if (!cpu_online(target_cpu))
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index a2b484679eb4..0d8df79c26ca 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -831,6 +831,13 @@ struct vmbus_channel {
>   	 */
>   	bool out_full_flag;
>   
> +	/*
> +	 * Check target_cpu in target_cpu_store() to make sure that it is in the
> +	 * HK_TYPE_MANAGED_IRQ housekeeping cpumask and reject it if not when
> +	 * the flag is set.
> +	 */
> +	bool check_hkcpu;
> +
>   	/* Channel callback's invoked in softirq context */
>   	struct tasklet_struct callback_event;
>   	void (*onchannel_callback)(void *context);