回复：[PATCH v18 net-next 07/11] net/nebula-matrix: add intr resource implementation

[Illusion Wang]

>commit 7a32ca8335c9041ec03b80d30716c474e81723ad
>Author: illusion.wang <illusion.wang@xxxxxxxxxxxxxxxxx>
>net/nebula-matrix: add intr resource implementation
>This patch adds the nbl_interrupt module for management of driver-private
>MSI-X indices, plus hw_ops callbacks to program the function MSI-X mapping
>registers, configure MSI-X info, and enable mailbox interrupts.
>> 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..6d6924e7d9a82
>> --- /dev/null
>> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c
>[ ... ]
>> +int nbl_res_intr_destroy_msix_map(struct nbl_resource_mgt *res_mgt,
>> +				  u16 func_id)
>> +{
>[ ... ]
>> +	/* use ctrl dev bdf */
>> +	hw_ops->configure_msix_map(res_mgt->hw_ops_tbl->priv, func_id, false, 0,
>> +				   0, 0, 0);
>> +
>> +	intr_mgt->func_intr_res[func_id].interrupts = NULL;
>> +	intr_mgt->func_intr_res[func_id].num_interrupts = 0;
>> +	intr_mgt->func_intr_res[func_id].num_net_interrupts = 0;
>> +	hw_ops->flush_write(res_mgt->hw_ops_tbl->priv);
>> +
>> +	return 0;
>> +}
>Does this code leak the device-managed memory? nbl_res_intr_configure_msix_map()
>allocates interrupts via devm_kcalloc() and msix_map_table->base_addr via
>dmam_alloc_coherent(). Setting the pointer to NULL here without calling
>devm_kfree() and dmam_free_coherent() means the memory won't be freed until
>the device is detached. If the MSI-X map is repeatedly reconfigured, could
>this exhaust memory?
>[ ... ]
>> +int nbl_res_intr_configure_msix_map(struct nbl_resource_mgt *res_mgt,
>> +				    u16 func_id, u16 num_net_msix,
>> +				    u16 num_others_msix,
>> +				    bool net_msix_mask_en)
>> +{
[ ... ]
>> +	msix_map_table = &intr_mgt->func_intr_res[func_id].msix_map_table;
>> +	WARN_ON(msix_map_table->base_addr);
>> +	msix_map_table->size =
>> +		sizeof(struct nbl_msix_map) * NBL_MSIX_MAP_TABLE_MAX_ENTRIES;
>> +	/* HW indexes the MSI-X map table with a fixed stride of 1024 entries,
>> +	 * so we must allocate the full table even if fewer vectors are used.
>> +	 */
>> +	msix_map_table->base_addr = dmam_alloc_coherent(dev,
>> +							msix_map_table->size,
>> +							&msix_map_table->dma,
>> +							GFP_KERNEL);
>> +	if (!msix_map_table->base_addr) {
>> +		dev_err(dev, "Allocate DMA memory for function msix map table failed\n");
>> +		msix_map_table->size = 0;
>> +		return -ENOMEM;
>> +	}
>> +
>> +	interrupts = devm_kcalloc(dev, requested, sizeof(interrupts[0]),
>> +				  GFP_KERNEL);
>> +	if (!interrupts) {
>> +		ret = -ENOMEM;
>> +		goto alloc_interrupts_err;
>> +	}
[ ... ]
>> +get_interrupt_err:
>> +	while (i--) {
>> +		intr_index = 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);
>> +	}
>> +
>> +	intr_mgt->func_intr_res[func_id].num_interrupts = 0;
>> +	intr_mgt->func_intr_res[func_id].interrupts = NULL;
>> +	intr_mgt->func_intr_res[func_id].num_net_interrupts = 0;
>> +alloc_interrupts_err:
>> +
>> +	return ret;
>> +}
>If an error occurs after allocating the memory (e.g., jumping to
>get_interrupt_err or alloc_interrupts_err), does this error path also leak
>the devm_kcalloc() and dmam_alloc_coherent() memory by simply returning
>without freeing it?

Both use devm-managed resources, so I think no manual free is needed.

destroy_msix_map: The interrupts array is allocated with devm_kcalloc(), and
the MSI-X map table is allocated with dmam_alloc_coherent() on the device's
DMA pool. Both are tied to the device lifecycle — they are automatically
reclaimed on driver detach. Setting the pointer to NULL is defensive;
the devm framework handles the actual free.
configure_msix_map error path: Same reasoning. interrupts is devm_kcalloc(),
and the DMA table is device-managed. On any error, the function returns to
the caller, which will eventually trigger driver remove and devm cleanup.
There is no repeated-reconfig loop in the current design — configure_msix_map
is only called during probe/reinit, not in a hot path — so exhaustion is not
a practical concern. Adding manual free calls would duplicate what devm already
does and risk double-free.