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 <long...@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);