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

[PATCH] 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 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>
---
 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);
-- 
2.55.0

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

[Waiman Long]

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);