Re: [PATCH v3 4/4] net: mana: Allocate MSI-X vectors dynamically

On Wed, May 14, 2025 at 05:04:03AM +0000, Michael Kelley wrote:

From: Shradha Gupta <redacted> Sent: Friday, May 9, 2025 3:14 AM

quoted

Currently, the MANA driver allocates MSI-X vectors statically based on
MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
up allocating more vectors than it needs. This is because, by this time
we do not have a HW channel and do not know how many IRQs should be
allocated.

To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
after getting the value supported by hardware, dynamically add the
remaining MSI-X vectors.

After this patch is applied, there are two functions for setting up IRQs:
1. mana_gd_setup_dyn_irqs()
2. mana_gd_setup_irqs()

#1 is about 78 lines of code and comments, while #2 is about 103 lines of
code and comments. But the two functions have a lot of commonality,
and that amount of commonality raises a red flag for me.

Have you looked at parameterizing things so a single function can serve
both purposes? I haven't worked through all the details, but at first
glance it looks very feasible, and without introducing unreasonable
messiness. Saving 70 to 80 lines of fairly duplicative code is worth a bit
of effort.

I have some other comments on the code. But if those two functions can
be combined, I'd rather re-review the result before adding comments that
may become irrelevant due to the restructuring.

Michael

On previous iteration I already mentioned that this patch is too big,
doesn't do exactly one thing and should be split to become a reviewable 
change. Shradha split-out the irq_setup() piece, and your review proves
that splitting helps to reviewability.

The rest of the change is still a mess. I agree that the functions you
mention likely duplicate each other. But overall, this patch bomb is
above my ability to review. Fortunately, I don't have to.

Thanks,
Yury