Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Incrementing the bounded loop counter when skipping empty NUMA nodes causes premature search termination, falling back to CPU 0. - [Medium] When the loop falls back to VMBUS_CONNECT_CPU, it fails to register this allocation in the hv_numa_map, breaking the round-robin distribution tracker. --
commit 5b552e920b17d64534cf7a69fd8a78145809498a Author: Waiman Long <[email protected]> Drivers: hv: Avoid infinite retry loop in init_vp_index() This commit fixes an infinite retry loop in init_vp_index() that occurs when the housekeeping cpumask for managed IRQs contains no CPUs in a particular NUMA node. It resolves this by skipping to the next node if the allocated cpumask is already cleared, and falling back to VMBUS_CONNECT_CPU if the loop is about to end. Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c > --- a/drivers/hv/channel_mgmt.c > +++ b/drivers/hv/channel_mgmt.c > @@ -793,10 +793,20 @@ static void init_vp_index(struct vmbus_channel *channel) > 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 try the next numa > + * node. Set target_cpu to the default VMBUS_CONNECT_CPU > + * instead if the for loop is going to end. > */ > - cpumask_clear(allocated_mask); > - goto retry; > + if (!cpumask_empty(allocated_mask)) { > + cpumask_clear(allocated_mask); > + goto retry; > + } > + if (i > ncpu) { > + target_cpu = VMBUS_CONNECT_CPU; > + break; [Severity: Medium] When the loop falls back to VMBUS_CONNECT_CPU here, does this bypass updating the round-robin allocation tracker? The fallback block earlier in init_vp_index() correctly calls hv_set_allocated_cpu(VMBUS_CONNECT_CPU) to maintain the global allocation state: channel->target_cpu = VMBUS_CONNECT_CPU; if (perf_chn) hv_set_allocated_cpu(VMBUS_CONNECT_CPU); return; Could breaking out directly here without making the corresponding state update cause a load imbalance by stacking extra channels on CPU 0 without the balancer knowing? > + } > + continue; /* Try next numa node */ [Severity: High] Does this continue statement prematurely exhaust the loop iterations when encountering nodes with no housekeeping CPUs? Since the loop counter i limits the total search across ncpu + 1 iterations, incrementing it while skipping an empty NUMA node burns through the search budget without actually evaluating any CPUs. For example, in a topology with several isolated NUMA nodes, could skipping these empty nodes cause the loop budget to run out early, incorrectly forcing a fallback to VMBUS_CONNECT_CPU before checking all valid housekeeping CPUs? > } > > target_cpu = cpumask_first(available_mask); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
