All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: han.junyang@zte.com.cn
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, ran.ming@zte.com.cn,
	han.chengfei@zte.com.cn, zhang.yanze@zte.com.cn
Subject: Re: [PATCH net-next v3 2/3] dinghai: add MSI-X interrupt pools
Date: Tue, 22 Sep 2026 11:09:13 +0100	[thread overview]
Message-ID: <20260922100913.GA13925@horms.kernel.org> (raw)
In-Reply-To: <202609211459460680vBvKdpuZZ9yR4QVn3ONR@zte.com.cn>

On Mon, Sep 21, 2026 at 02:59:46PM +0800, han.junyang@zte.com.cn wrote:
> From: Junyang Han <han.junyang@zte.com.cn>
> 
> Allocate the fixed MSI-X vector layout of the device and manage the
> vectors in per-purpose pools: the async event queues first, a range
> reserved for RDMA in between and the vq queue pairs last. This series
> wires up the async pool; the vq pool comes with the netdev series.
> 
> IRQs are reference counted so that several event queues can share one
> vector. The pool hands out the least loaded matching IRQ once it passes
> the pool minimum threshold, and binds newly created IRQs to the least
> loaded CPU of the requested affinity mask. Interrupt delivery fans out
> through an atomic notifier chain attached to each IRQ, which the async
> event queue setup posted later in this series hooks into.
> 
> Signed-off-by: Junyang Han <han.junyang@zte.com.cn>

...

> @@ -516,10 +604,24 @@ static int zxdh_pf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		goto err_modern_cfg;
>  	}
> 
> +	ret = zxdh_pf_irq_table_init(zxdh_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "zxdh_pf_irq_table_init failed: %d\n", ret);
> +		goto err_modern_cfg;
> +	}
> +
> +	ret = zxdh_pf_irq_table_create(zxdh_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "zxdh_pf_irq_table_create failed: %d\n", ret);
> +		goto err_irq_table;
> +	}

I am wondering if you considered calling zxdh_pf_irq_table_create()
from zxdh_pf_irq_table_init(). And likewise for
zxdh_pf_eq_table_init()/zxdh_pf_eq_table_create() in patch 3/3.

I mainly ask because it seems like it would simplify zxdh_pf_probe()
slightly. But I don't feel strongly about this.

> +
>  	devlink_register(devlink);
> 
>  	return 0;
> 
> +err_irq_table:
> +	kvfree(zxdh_dev->irq_table.priv);

As a counter to my previous comment: I see the line is changed to call
zxdh_pf_irq_table_destroy() in patch 3/3. But I'm wondering if it should be
(or would be nicer if it was) zxdh_pf_irq_table_destroy() in this patch.

>  err_modern_cfg:
>  	zxdh_pf_modern_cfg_uninit(zxdh_dev);
>  err_cfg_init:

...

> diff --git a/drivers/net/ethernet/zte/dinghai/zxdh_irq.c b/drivers/net/ethernet/zte/dinghai/zxdh_irq.c

...

> +static struct zxdh_irq *zxdh_irq_alloc(struct zxdh_irq_pool *pool, int vecidx,
> +				       const struct cpumask *affinity)
> +{
> +	struct zxdh_core_dev *zxdh_dev = pool->dev;
> +	struct zxdh_irq *irq;
> +	int err;
> +	int cpu;
> +
> +	irq = kzalloc_obj(*irq, GFP_KERNEL);
> +	if (!irq)
> +		return ERR_PTR(-ENOMEM);
> +
> +	irq->pool = pool;
> +	irq->irqn = pci_irq_vector(zxdh_dev->pdev, vecidx);
> +	if (irq->irqn < 0) {
> +		err = irq->irqn;
> +		goto err_irqn;
> +	}
> +
> +	ATOMIC_INIT_NOTIFIER_HEAD(&irq->nh);
> +	snprintf(irq->name, ZXDH_MAX_IRQ_NAME, "%s_%d@pci:%s", pool->name,
> +		 vecidx, pci_name(zxdh_dev->pdev));

W=1 builds complain about this. E.g. GCC 16.2.0 on x86_64 says:

  CC [M]  drivers/net/ethernet/zte/dinghai/zxdh_irq.o
drivers/net/ethernet/zte/dinghai/zxdh_irq.c: In function 'zxdh_irq_alloc':
drivers/net/ethernet/zte/dinghai/zxdh_irq.c:94:52: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size between 0 and 99 [-Wformat-truncation=]
   94 |         snprintf(irq->name, ZXDH_MAX_IRQ_NAME, "%s_%d@pci:%s", pool->name,
      |                                                    ^~
drivers/net/ethernet/zte/dinghai/zxdh_irq.c:94:9: note: 'snprintf' output 8 or more bytes (assuming 107) into a destination of size 100
   94 |         snprintf(irq->name, ZXDH_MAX_IRQ_NAME, "%s_%d@pci:%s", pool->name,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   95 |                  vecidx, pci_name(zxdh_dev->pdev));
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

...

  reply	other threads:[~2026-09-22 10:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21  6:51 [PATCH net-next v3 0/3] dinghai: firmware handshake, MSI-X pools and async event queues han.junyang
2026-09-21  6:56 ` [PATCH net-next v3 1/3] dinghai: add firmware version check and RISC-V readiness polling han.junyang
2026-09-22 10:11   ` [PATCH net-next v3 1/3] dinghai: add firmware version check and? " Simon Horman
2026-09-28 12:00     ` han.junyang
2026-09-21  6:59 ` [PATCH net-next v3 2/3] dinghai: add MSI-X interrupt pools han.junyang
2026-09-22 10:09   ` Simon Horman [this message]
2026-09-24 16:00     ` Jakub Kicinski
2026-09-25 13:16       ` Simon Horman
2026-09-28 12:10     ` han.junyang
2026-09-21  7:03 ` [PATCH net-next v3 3/3] dinghai: add async event queue for firmware notifications han.junyang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260922100913.GA13925@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=han.chengfei@zte.com.cn \
    --cc=han.junyang@zte.com.cn \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=ran.ming@zte.com.cn \
    --cc=zhang.yanze@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.