All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josua Mayer <josua@solid-run.com>
To: Ioana Ciornei <ioana.ciornei@nxp.com>
Cc: "Christophe Leroy (CS GROUP)" <chleroy@kernel.org>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Horia GeantÄ" <horia.geanta@nxp.com>,
	"Pankaj Gupta" <pankaj.gupta@nxp.com>,
	"Gaurav Jain" <gaurav.jain@nxp.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"Yazan Shhady" <yazan.shhady@solid-run.com>,
	"Jon Nettleton" <jon@solid-run.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>
Subject: Re: [PATCH RFC net-next 2/3] crypto: caam/qi2 - skip isolated CPUs when setting up queue pairs
Date: Thu, 8 Oct 2026 10:49:57 +0000	[thread overview]
Message-ID: <6d6f2e02-afd3-4e24-8b54-68cb93ce8174@solid-run.com> (raw)
In-Reply-To: <cozavqeupdtr7cyn33nvtfqoqkw3sop5tffou6753vwg63hgwf@jr6gx236yblx>

Hi Ioana,

Thank you for taking a look at my patch!

On 10/6/26 11:27, Ioana Ciornei wrote:
> On Wed, Sep 30, 2026 at 06:54:59PM +0200, Josua Mayer wrote:
>> The driver looks up the affine DPIO and sets up one Rx/Tx queue pair for
>> each online CPU, up to the number of queues. Response notifications and
>> NAPI polling are handled on the DPIO object's assigned CPU. All online
>> may enqueue requests.
>>
>> When isolcpus=managed_irq is used to reserve CPUs for latency-sensitive
>> applications, those CPUs are expected to be free of routine device
>> interrupts.
>>
>> The DPIO driver assigns affine CPUs only from the HK_TYPE_MANAGED_IRQ
>> housekeeping mask, leaving isolated CPUs without an affine DPIO. This
>> defers caam qi2 probe indefinitely once DPIO setup reaches an isolated
>> CPU.
>>
>> Restrict queue pairs, notifications and NAPI polling to the online CPUs
>> in the HK_TYPE_MANAGED_IRQ housekeeping mask, so that isolated CPUs are
>> skipped and don't defer probe. Isolated CPUs may still enqueue requests,
>> with responses handled on housekeeping CPUs.
>
> Don't you need to also change the for_each_online_cpu() from
> dpaa2_dpseci_setup() to take into account only the housekeeping
> managed_irq cores?
I initially thought no, because of the "Allow all cores to enqueue"
comment in code.
>
>
> Something like:
>
> --- a/drivers/crypto/caam/caamalg_qi2.c
> +++ b/drivers/crypto/caam/caamalg_qi2.c
> @@ -5101,7 +5101,7 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
>          }
>
>          i = 0;
> -       for_each_online_cpu(cpu) {
> +       for_each_cpu_and(cpu, cpu_online_mask, hk_mask) {
>                  u8 j;
>
>                  j = i % priv->num_pairs;

Correct.

The loop is doing two things in one: assign tx queue to each cpu,
and assign rx queue to each cpu.

Only rx side is an issue for isolcpus. Do you have any idea why each cpu
got a tx queue assigned, and if this may be changed?


Here is a copy of the complete loop, with inline comments on the
control flow:

> 	i = 0;
> 	for_each_online_cpu(cpu) {
> 		u8 j;
>
> 		j = i % priv->num_pairs;
>
> 		ppriv = per_cpu_ptr(priv->ppriv, cpu);
> 		ppriv->req_fqid = priv->tx_queue_attr[j].fqid;
Assigns a tx queue to the cpu, one tx queue may be assigned to
multiple cores due to modulo above (if i >= num_pairs).
> 		/*
> 		 * Allow all cores to enqueue, while only some of them
> 		 * will take part in dequeuing.
> 		 */
> 		if (++i > priv->num_pairs)
> 			continue;
After this i <= num_pairs.
> 		ppriv->rsp_fqid = priv->rx_queue_attr[j].fqid;
> 		ppriv->prio = j;
Assigns a rx queue to the cpu.
This is the case which causes issues with isolcpus, and it should
be avoided for non-housekeeping cpus.
> 		dev_dbg(dev, "pair %d: rx queue %d, tx queue %d\n", j,
> 			priv->rx_queue_attr[j].fqid,
> 			priv->tx_queue_attr[j].fqid);
>
> 		ppriv->net_dev = alloc_netdev_dummy(0);
> 		if (!ppriv->net_dev) {
> 			err = -ENOMEM;
> 			goto err_alloc_netdev;
> 		}
> 		cpumask_set_cpu(cpu, priv->clean_mask);
> 		ppriv->net_dev->dev = *dev;
>
> 		netif_napi_add_tx_weight(ppriv->net_dev, &ppriv->napi,
> 					 dpaa2_dpseci_poll,
> 					 DPAA2_CAAM_NAPI_WEIGHT);
> 	}
dpaa2_caam_enqueue unconditionally accesses ppriv->dpio per-cpu
and ppriv->req_fqid, not gated by any checks, percpu (any cpu).

This causes two issues:
1. each cpu is still expected to have a dpio object
2. each cpu is still expected to have an assigned tx queue

I don't quite understand how we get into dpaa2_caam_enqueue,
I suppose it can legitimately happen on any cpu core whatsoever?

sincerely
Josua Mayer

  reply	other threads:[~2026-10-08 10:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30 16:54 [PATCH RFC net-next 0/3] dpaa2: avoid scheduling dpio interrupts on isolated CPUs Josua Mayer
2026-09-30 16:54 ` [PATCH RFC net-next 1/3] dpaa2-eth: skip isolated CPUs when allocating channels Josua Mayer
2026-09-30 16:54 ` [PATCH RFC net-next 2/3] crypto: caam/qi2 - skip isolated CPUs when setting up queue pairs Josua Mayer
2026-10-06  8:27   ` Ioana Ciornei
2026-10-08 10:49     ` Josua Mayer [this message]
2026-10-08 14:01       ` Ioana Ciornei
2026-09-30 16:55 ` [PATCH RFC net-next 3/3] soc: fsl: dpio: skip isolated CPUs when assigning portals Josua Mayer

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=6d6f2e02-afd3-4e24-8b54-68cb93ce8174@solid-run.com \
    --to=josua@solid-run.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=chleroy@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gaurav.jain@nxp.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=horia.geanta@nxp.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=jon@solid-run.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pankaj.gupta@nxp.com \
    --cc=yazan.shhady@solid-run.com \
    /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.