Re: [PATCH 15/23] Drivers: hv: Use RCU to protect access of HK_TYPE_MANAGED_IRQ cpumask

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Infinite loop causing RCU stall when a NUMA node lacks housekeeping CPUs.
- [Medium] Mixing `goto`-based cleanup with scope-based `guard()` violates subsystem guidelines.
- [Low] Unnecessary unconditional memory allocation for non-performance channels.
--

[Note: Due to recitation restrictions, this report uses a free-form summary
format rather than quoting the original patch lines directly.]

commit bc4398753271c194097795d20b6095f594c74b4c
Author: Waiman Long <longman@redhat.com>
Drivers: hv: Use RCU to protect access of HK_TYPE_MANAGED_IRQ cpumask

This commit uses RCU to protect the HK_TYPE_MANAGED_IRQ cpumask during access,
moving the cpumask allocation before taking the RCU read lock since the
allocation can sleep.

I noticed a few potential regressions in this patch:

First, looking at the loop in init_vp_index(), if a selected NUMA node has
no CPUs present in the housekeeping mask (for example, if the entire node is
isolated), the intersection of available_mask and hk_mask becomes empty.
The code then clears allocated_mask and jumps back to the retry label.

Since the variables do not change state across retries, does this create
an infinite loop? Because this patch places the loop inside an RCU
read-side critical section via guard(rcu)(), would this spin result in
an RCU stall?

Second, the patch introduces guard(rcu)() in init_vp_index() while retaining
the goto out_free cleanup path for free_cpumask_var(). According to the
kernel's cleanup guidelines, mixing goto-based error handling with
scope-based cleanups like guard() in the same function creates confusing
ownership semantics. 

Should this function be fully converted to scope-based cleanup, or 
alternatively, use explicit rcu_read_lock() and rcu_read_unlock() calls 
to avoid this mix?

Finally, before this patch, the code used short-circuit logic to skip
allocating the cpumask if the channel was not performance-critical. The
patch refactors this to unconditionally call alloc_cpumask_var() at the top
of the function before evaluating the channel type. 

Does this introduce unnecessary sleepable memory allocation and deallocation 
cycles for non-performance channels?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421030351.281436-1-longman@redhat.com?part=15