Re: [PATCH v22 net-next 06/12] net/nebula-matrix: add intr resource implementation
Jakub Kicinski <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- --- Patch [6]: [PATCH v22 net-next 06/12] net/nebula-matrix: add intr resource implementation --- commit 32d01aebd4d29040305943012638ac9e59c45fb8 Author: illusion wang <[email protected]> net/nebula-matrix: add intr resource implementation This commit adds the nbl_interrupt module to manage driver global MSI-X vector indices. It extends hw_ops with callbacks to program vendor-specific internal MSI-X mapping registers, configure MSI-X metadata, and bind interrupts to PF mailbox channels. > diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c > new file mode 100644 > index 0000000000000..cb6874d13a88f > --- /dev/null > +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c [ ... ] > @@ -0,0 +1,335 @@ [ ... ] > +release_net_bits: > + while (i--) { > + intr_index = tmp_interrupts[i]; > + if (intr_index >= NBL_NET_INTR_BASE) > + clear_bit(intr_index - NBL_NET_INTR_BASE, > + intr_mgt->interrupt_net_bitmap); > + else > + clear_bit(intr_index, > + intr_mgt->interrupt_others_bitmap); > + } > + i = num_net_msix; > +release_all_bits: > + while (i--) { [Severity: High] Does this fall-through inadvertently corrupt the global interrupt bitmaps? If resource allocation fails in the first loop for net interrupts, the code jumps to release_net_bits, loops i down to 0, sets i = num_net_msix, and then falls through into release_all_bits. Because tmp_interrupts was allocated with devm_kcalloc, any uninitialized entries are 0. When the second loop evaluates these uninitialized zero values, 0 < NBL_NET_INTR_BASE, meaning the code will clear bit 0 in interrupt_others_bitmap and also doubly clear the already-released net bits. Should there be a jump to free_prealloc right before release_all_bits to avoid evaluating the array twice? > + intr_index = tmp_interrupts[i]; > + if (intr_index >= NBL_NET_INTR_BASE) > + clear_bit(intr_index - NBL_NET_INTR_BASE, > + intr_mgt->interrupt_net_bitmap); > + else > + clear_bit(intr_index, > + intr_mgt->interrupt_others_bitmap); > + } > + goto free_prealloc; > +}