Re: [PATCH net-next v10 2/7] r8169: refactor RX path to prepare for multi-queue

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8169: refactor RX path to prepare for multi-queue

This patch refactors the RX path by introducing a per-queue array
struct rtl8169_rx_ring, allocating the netdev with devm_alloc_etherdev_mqs(),
and updating related initialization and cleanup routines.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 8f3a5c50299ff..924df06398287 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
[ ... ]
> @@ -5873,9 +5995,15 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (rc)
>  		return rc;
>  
> +	tp->rx_ring = kcalloc(tp->num_rx_rings, sizeof(struct rtl8169_rx_ring), GFP_KERNEL);
> +	if (!tp->rx_ring)
> +		return -ENOMEM;
> +

[Severity: Medium]
Does this code leak the rx_ring array on driver removal?

tp->rx_ring is explicitly allocated with kcalloc() here, but it appears there
is no corresponding kfree(tp->rx_ring) in rtl_remove_one(). Since
devm_kcalloc() isn't used, shouldn't tp->rx_ring be freed when the device is
unbound?

>  	tp->rtl8169_napi = kcalloc(tp->irq_nvecs, sizeof(struct napi_struct), GFP_KERNEL);
> -	if (!tp->rtl8169_napi)
> +	if (!tp->rtl8169_napi) {
> +		kfree(tp->rx_ring);
>  		return -ENOMEM;
> +	}
-- 
pw-bot: cr