Re: [PATCH net-next v05 5/8] hinic3: TX & RX Queue coalesce interfaces

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 25/06/2025 04:41, Fan Gong wrote:
> Add TX RX queue coalesce interfaces initialization.
> It configures the parameters of tx & tx msix coalesce.
> 
> Co-developed-by: Xin Guo <guoxin09@huawei.com>
> Signed-off-by: Xin Guo <guoxin09@huawei.com>
> Co-developed-by: Zhu Yikai <zhuyikai1@h-partners.com>
> Signed-off-by: Zhu Yikai <zhuyikai1@h-partners.com>
> Signed-off-by: Fan Gong <gongfan1@huawei.com>
> ---
>   .../net/ethernet/huawei/hinic3/hinic3_main.c  | 65 +++++++++++++++++--
>   .../ethernet/huawei/hinic3/hinic3_nic_dev.h   | 10 +++
>   2 files changed, 70 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_main.c b/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
> index 497f2a36f35d..8d1c7a388762 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
> @@ -17,12 +17,57 @@
>   
>   #define HINIC3_NIC_DRV_DESC  "Intelligent Network Interface Card Driver"
>   
> -#define HINIC3_RX_BUF_LEN            2048
> -#define HINIC3_LRO_REPLENISH_THLD    256
> -#define HINIC3_NIC_DEV_WQ_NAME       "hinic3_nic_dev_wq"
> +#define HINIC3_RX_BUF_LEN          2048
> +#define HINIC3_LRO_REPLENISH_THLD  256
> +#define HINIC3_NIC_DEV_WQ_NAME     "hinic3_nic_dev_wq"
>   
> -#define HINIC3_SQ_DEPTH              1024
> -#define HINIC3_RQ_DEPTH              1024
> +#define HINIC3_SQ_DEPTH            1024
> +#define HINIC3_RQ_DEPTH            1024
> +
> +#define HINIC3_DEFAULT_TXRX_MSIX_PENDING_LIMIT      2
> +#define HINIC3_DEFAULT_TXRX_MSIX_COALESC_TIMER_CFG  25
> +#define HINIC3_DEFAULT_TXRX_MSIX_RESEND_TIMER_CFG   7
> +
> +static void init_intr_coal_param(struct net_device *netdev)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct hinic3_intr_coal_info *info;
> +	u16 i;
> +
> +	for (i = 0; i < nic_dev->max_qps; i++) {
> +		info = &nic_dev->intr_coalesce[i];
> +		info->pending_limit = HINIC3_DEFAULT_TXRX_MSIX_PENDING_LIMIT;
> +		info->coalesce_timer_cfg = HINIC3_DEFAULT_TXRX_MSIX_COALESC_TIMER_CFG;
> +		info->resend_timer_cfg = HINIC3_DEFAULT_TXRX_MSIX_RESEND_TIMER_CFG;
> +	}
> +}
> +
> +static int hinic3_init_intr_coalesce(struct net_device *netdev)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct hinic3_hwdev *hwdev = nic_dev->hwdev;
> +	u64 size;
> +
> +	size = sizeof(*nic_dev->intr_coalesce) * nic_dev->max_qps;
> +	if (!size) {
> +		dev_err(hwdev->dev, "Cannot allocate zero size intr coalesce\n");
> +		return -EINVAL;
> +	}

I don't see how that can happen. max_qps is checked to be positive value
in nic_dev_init and is never changed to less than 1. It's way too
defensive coding here.

> +	nic_dev->intr_coalesce = kzalloc(size, GFP_KERNEL);

kcalloc(nic_dev->max_qps, sizeof(*nic_dev->intr_coalesce), GFP_KERNEL)
will make it, no need for local variable

> +	if (!nic_dev->intr_coalesce)
> +		return -ENOMEM;
> +
> +	init_intr_coal_param(netdev);
> +
> +	return 0;
> +}
> +
> +static void hinic3_free_intr_coalesce(struct net_device *netdev)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +
> +	kfree(nic_dev->intr_coalesce);
> +}
>   
>   static int hinic3_alloc_txrxqs(struct net_device *netdev)
>   {
> @@ -42,8 +87,17 @@ static int hinic3_alloc_txrxqs(struct net_device *netdev)
>   		goto err_free_txqs;
>   	}
>   
> +	err = hinic3_init_intr_coalesce(netdev);
> +	if (err) {
> +		dev_err(hwdev->dev, "Failed to init_intr_coalesce\n");
> +		goto err_free_rxqs;
> +	}
> +
>   	return 0;
>   
> +err_free_rxqs:
> +	hinic3_free_rxqs(netdev);
> +
>   err_free_txqs:
>   	hinic3_free_txqs(netdev);
>   
> @@ -52,6 +106,7 @@ static int hinic3_alloc_txrxqs(struct net_device *netdev)
>   
>   static void hinic3_free_txrxqs(struct net_device *netdev)
>   {
> +	hinic3_free_intr_coalesce(netdev);
>   	hinic3_free_rxqs(netdev);
>   	hinic3_free_txqs(netdev);
>   }
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h
> index c994fc9b6ee0..9577cc673257 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_dev.h
> @@ -51,6 +51,12 @@ struct hinic3_dyna_txrxq_params {
>   	struct hinic3_irq_cfg      *irq_cfg;
>   };
>   
> +struct hinic3_intr_coal_info {
> +	u8 pending_limit;
> +	u8 coalesce_timer_cfg;
> +	u8 resend_timer_cfg;
> +};
> +
>   struct hinic3_nic_dev {
>   	struct pci_dev                  *pdev;
>   	struct net_device               *netdev;
> @@ -70,10 +76,14 @@ struct hinic3_nic_dev {
>   	u16                             num_qp_irq;
>   	struct msix_entry               *qps_msix_entries;
>   
> +	struct hinic3_intr_coal_info    *intr_coalesce;
> +
>   	bool                            link_status_up;
>   };
>   
>   void hinic3_set_netdev_ops(struct net_device *netdev);
> +int hinic3_qps_irq_init(struct net_device *netdev);
> +void hinic3_qps_irq_uninit(struct net_device *netdev);
>   
>   /* Temporary prototypes. Functions become static in later submission. */
>   void qp_add_napi(struct hinic3_irq_cfg *irq_cfg);