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

[Michael Kelley]

From: Waiman Long <longman@xxxxxxxxxx> Sent: Friday, August 28, 2026 6:22 PM
> 
> On 8/27/26 5:10 PM, Michael Kelley wrote:
> > From: Waiman Long <longman@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >>   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.
> 

The flip side is people like me try to figure out "why is this
here?", thinking there must be a reason. :-)  I'd say drop the
code and add a comment like "Also catches an empty node_mask".

Michael