[PATCH 0/4] crypto: iaa - rework the WQ selection tables

[PATCH 0/4] crypto: iaa - rework the WQ selection tables

[Vinicius Costa Gomes]

iaa_crypto maps each CPU to a single IAA device through a flat CPU
index, which ignores packages (where the actual IAA devices live) and
leaves other IAA devices in the package idle.

Patches 1-2 make the request carry the IAA workqueue
representation ('iaa_wq') it holds a reference on, instead of looking
it up again. Patch 3 replaces the per-CPU WQ table with per-package
tables published with RCU, and patch 4 keeps each CPU on one WQ for
decompression.

Patches 1-3 are meant more of a simplification, to make the code
easier to reason about. Patch 4 is "low hanging" optimization (because
decompressions are very fast, they benefit when the same CPU uses the
same engine). Future work will introduce more complex optimizations.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
Vinicius Costa Gomes (4):
      crypto: iaa - carry the iaa_wq in the request
      crypto: iaa - take the WQ from the request in the submit helpers
      crypto: iaa - use RCU per-package WQ tables
      crypto: iaa - keep each CPU on one WQ for decompression

 drivers/crypto/intel/iaa/iaa_crypto.h      |   7 -
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 393 ++++++++++++-----------------
 2 files changed, 157 insertions(+), 243 deletions(-)
---
base-commit: 178aff4cf425b3fc51ff09c571fa1bfbc1eb8ea4
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
change-id: 20261008-iaa-crypto-wq-tables-4e5911143017

Best regards,
--  
Vinicius

[PATCH 1/4] crypto: iaa - carry the iaa_wq in the request

[Vinicius Costa Gomes]

The submit helpers and the completion look the iaa_wq up again through
the WQ's private pointer, after the entry point has already found it to
take its reference. The submit helpers are also passed the WQ
separately, so a request has no single record of which WQ it holds its
reference on.

Store the iaa_wq in the request context when the reference is taken, and
use it everywhere after that. This makes the association of a
IAA workqueue and the request clear.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index c9ab4b83ae02..0f5bd244b00a 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -159,6 +159,7 @@ static bool async_mode;
 static bool use_irq;
 
 struct iaa_req_ctx {
+	struct iaa_wq *iaa_wq;
 	u32 compression_crc;
 	struct page *bounce_src;
 	dma_addr_t bounce_src_dma;
@@ -1070,7 +1071,7 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 
 	compression_ctx = crypto_tfm_ctx(ctx->tfm);
 
-	iaa_wq = idxd_wq_get_private(idxd_desc->wq);
+	iaa_wq = req_ctx->iaa_wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1177,7 +1178,7 @@ static int iaa_compress(struct crypto_tfm *tfm,	struct acomp_req *req,
 	struct device *dev;
 	int ret = 0;
 
-	iaa_wq = idxd_wq_get_private(wq);
+	iaa_wq = req_ctx->iaa_wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1328,7 +1329,7 @@ static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
 	struct device *dev;
 	int ret = 0;
 
-	iaa_wq = idxd_wq_get_private(wq);
+	iaa_wq = req_ctx->iaa_wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1405,6 +1406,7 @@ static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
 {
 	struct iaa_device_compression_mode *active_compression_mode;
 	struct iaa_compression_ctx *ctx = crypto_tfm_ctx(tfm);
+	struct iaa_req_ctx *req_ctx = acomp_request_ctx(req);
 	struct iaa_device *iaa_device;
 	struct idxd_desc *idxd_desc;
 	struct iax_hw_desc *desc;
@@ -1414,7 +1416,7 @@ static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
 	struct device *dev;
 	int ret = 0;
 
-	iaa_wq = idxd_wq_get_private(wq);
+	iaa_wq = req_ctx->iaa_wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1555,6 +1557,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	}
 
 	iaa_wq = idxd_wq_get_private(wq);
+	req_ctx->iaa_wq = iaa_wq;
 
 	dev = &wq->idxd->pdev->dev;
 
@@ -1664,6 +1667,7 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 	}
 
 	iaa_wq = idxd_wq_get_private(wq);
+	req_ctx->iaa_wq = iaa_wq;
 
 	dev = &wq->idxd->pdev->dev;
 

-- 
2.56.0

[PATCH 2/4] crypto: iaa - take the WQ from the request in the submit helpers

[Vinicius Costa Gomes]

iaa_compress(), iaa_compress_verify() and iaa_decompress() take the WQ
to submit to as a parameter, and also find the iaa_wq in the request
context. Nothing ties the two together.

Drop the parameter and use the request's iaa_wq->wq.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index 0f5bd244b00a..7b02d3836623 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -1049,7 +1049,6 @@ static int iaa_remap_for_verify(struct device *dev, struct iaa_wq *iaa_wq,
 				dma_addr_t *src_addr, dma_addr_t *dst_addr);
 
 static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
-			       struct idxd_wq *wq,
 			       dma_addr_t src_addr, unsigned int slen,
 			       dma_addr_t dst_addr, unsigned int *dlen);
 
@@ -1134,7 +1133,7 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 			goto out;
 		}
 
-		ret = iaa_compress_verify(ctx->tfm, ctx->req, iaa_wq->wq, src_addr,
+		ret = iaa_compress_verify(ctx->tfm, ctx->req, src_addr,
 					  ctx->req->slen, dst_addr, &ctx->req->dlen);
 		if (ret) {
 			dev_dbg(dev, "%s: compress verify failed ret=%d\n", __func__, ret);
@@ -1162,7 +1161,6 @@ static void iaa_desc_complete(struct idxd_desc *idxd_desc,
 }
 
 static int iaa_compress(struct crypto_tfm *tfm,	struct acomp_req *req,
-			struct idxd_wq *wq,
 			dma_addr_t src_addr, unsigned int slen,
 			dma_addr_t dst_addr, unsigned int *dlen)
 {
@@ -1175,10 +1173,12 @@ static int iaa_compress(struct crypto_tfm *tfm,	struct acomp_req *req,
 	struct idxd_device *idxd;
 	struct iaa_wq *iaa_wq;
 	struct pci_dev *pdev;
+	struct idxd_wq *wq;
 	struct device *dev;
 	int ret = 0;
 
 	iaa_wq = req_ctx->iaa_wq;
+	wq = iaa_wq->wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1313,7 +1313,6 @@ static int iaa_remap_for_verify(struct device *dev, struct iaa_wq *iaa_wq,
 }
 
 static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
-			       struct idxd_wq *wq,
 			       dma_addr_t src_addr, unsigned int slen,
 			       dma_addr_t dst_addr, unsigned int *dlen)
 {
@@ -1326,10 +1325,12 @@ static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
 	struct idxd_device *idxd;
 	struct iaa_wq *iaa_wq;
 	struct pci_dev *pdev;
+	struct idxd_wq *wq;
 	struct device *dev;
 	int ret = 0;
 
 	iaa_wq = req_ctx->iaa_wq;
+	wq = iaa_wq->wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1400,7 +1401,6 @@ static int iaa_compress_verify(struct crypto_tfm *tfm, struct acomp_req *req,
 }
 
 static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
-			  struct idxd_wq *wq,
 			  dma_addr_t src_addr, unsigned int slen,
 			  dma_addr_t dst_addr, unsigned int *dlen)
 {
@@ -1413,10 +1413,12 @@ static int iaa_decompress(struct crypto_tfm *tfm, struct acomp_req *req,
 	struct idxd_device *idxd;
 	struct iaa_wq *iaa_wq;
 	struct pci_dev *pdev;
+	struct idxd_wq *wq;
 	struct device *dev;
 	int ret = 0;
 
 	iaa_wq = req_ctx->iaa_wq;
+	wq = iaa_wq->wq;
 	iaa_device = iaa_wq->iaa_device;
 	idxd = iaa_device->idxd;
 	pdev = idxd->pdev;
@@ -1582,8 +1584,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	dev_dbg(dev, "map dst %llx req->dst %p dlen %d sg_len %d\n", dst_addr,
 		req->dst, req->dlen, sg_dma_len(req->dst));
 
-	ret = iaa_compress(tfm, req, wq, src_addr, req->slen, dst_addr,
-			   &req->dlen);
+	ret = iaa_compress(tfm, req, src_addr, req->slen, dst_addr, &req->dlen);
 	if (ret == -EINPROGRESS)
 		return ret;
 
@@ -1594,7 +1595,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 			goto out;
 		}
 
-		ret = iaa_compress_verify(tfm, req, wq, src_addr, req->slen,
+		ret = iaa_compress_verify(tfm, req, src_addr, req->slen,
 					  dst_addr, &req->dlen);
 		if (ret)
 			dev_dbg(dev, "asynchronous compress verification failed ret=%d\n", ret);
@@ -1719,8 +1720,8 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 	dev_dbg(dev, "map dst %llx req->dst %p dlen %d sg_len %d\n", dst_addr,
 		req->dst, req->dlen, sg_dma_len(req->dst));
 
-	ret = iaa_decompress(tfm, req, wq, src_addr, req->slen,
-			     dst_addr, &req->dlen);
+	ret = iaa_decompress(tfm, req, src_addr, req->slen, dst_addr,
+			     &req->dlen);
 	if (ret == -EINPROGRESS)
 		return ret;
 

-- 
2.56.0

[PATCH 3/4] crypto: iaa - use RCU per-package WQ tables

[Vinicius Costa Gomes]

rebalance_wq_table() gives CPU n all the WQs of IAA device
n / cpus_per_iaa. The flat index ignores package boundaries, so a CPU
can be paired with a device on another package, and each CPU sends all
its requests to one device while the other devices of its package stay
idle. The table is also rebuilt in place, so a request issued during a
probe or remove can read a NULL WQ and fail with -ENODEV.

Replace it with one table per package, holding the WQs of every IAA
device on that package, and have each CPU round-robin over its package's
table. Rebalance builds the whole set of tables and publishes it with
rcu_replace_pointer(). The submit paths look the WQ up, and take their
reference on it, inside an RCU read-side section.

- A package without IAA devices gets a copy of another package's table,
  so the lookup needs no fallback.
- Every table has room for all the WQs, a few pointers more than it
  needs, so they can be sized without counting per package.
- If a rebalance fails, the current tables are kept. On removal they
  are cleared instead, because they still hold the removed WQ. Removal
  then waits for a grace period before returning, because the idxd_wq
  can be freed after that.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto.h      |   7 -
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 346 ++++++++++-------------------
 2 files changed, 117 insertions(+), 236 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto.h b/drivers/crypto/intel/iaa/iaa_crypto.h
index 56985e395263..aece8af011c2 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto.h
+++ b/drivers/crypto/intel/iaa/iaa_crypto.h
@@ -79,13 +79,6 @@ struct iaa_device {
 	atomic64_t			decomp_bytes;
 };
 
-struct wq_table_entry {
-	struct idxd_wq **wqs;
-	int	max_wqs;
-	int	n_wqs;
-	int	cur_wq;
-};
-
 #define IAA_AECS_ALIGN			32
 
 /*
diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index 7b02d3836623..2ce545e8df42 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -12,7 +12,9 @@
 #include <linux/mempool.h>
 #include <uapi/linux/idxd.h>
 #include <linux/highmem.h>
+#include <linux/numa.h>
 #include <linux/sched/smt.h>
+#include <linux/topology.h>
 #include <crypto/internal/acompress.h>
 
 #include "idxd.h"
@@ -29,62 +31,34 @@
 
 /* number of iaa instances probed */
 static unsigned int nr_iaa;
-static unsigned int nr_cpus;
-static unsigned int nr_nodes;
-static unsigned int nr_cpus_per_node;
 
-/* Number of physical cpus sharing each iaa instance */
-static unsigned int cpus_per_iaa;
-
-/* Per-cpu lookup table for balanced wqs */
-static struct wq_table_entry __percpu *wq_table;
-
-static struct idxd_wq *wq_table_next_wq(int cpu)
-{
-	struct wq_table_entry *entry = per_cpu_ptr(wq_table, cpu);
-
-	if (++entry->cur_wq >= entry->n_wqs)
-		entry->cur_wq = 0;
-
-	if (!entry->wqs[entry->cur_wq])
-		return NULL;
-
-	pr_debug("%s: returning wq at idx %d (iaa wq %d.%d) from cpu %d\n", __func__,
-		 entry->cur_wq, entry->wqs[entry->cur_wq]->idxd->id,
-		 entry->wqs[entry->cur_wq]->id, cpu);
+struct iaa_pkg_table {
+	struct rcu_head	rcu;
+	int		n_wqs;
+	struct idxd_wq	*wqs[];
+};
 
-	return entry->wqs[entry->cur_wq];
-}
+/* Indexed by logical package id, the count is fixed at boot */
+static struct iaa_pkg_table __rcu **iaa_pkg_tables;
+static DEFINE_PER_CPU(unsigned int, iaa_wq_idx);
 
-static void wq_table_add(int cpu, struct idxd_wq *wq)
+static int iaa_cpu_pkg(int cpu)
 {
-	struct wq_table_entry *entry = per_cpu_ptr(wq_table, cpu);
-
-	if (WARN_ON(entry->n_wqs == entry->max_wqs))
-		return;
-
-	entry->wqs[entry->n_wqs++] = wq;
+	int pkg = (cpu < nr_cpu_ids) ? topology_logical_package_id(cpu) : 0;
 
-	pr_debug("%s: added iaa wq %d.%d to idx %d of cpu %d\n", __func__,
-		 entry->wqs[entry->n_wqs - 1]->idxd->id,
-		 entry->wqs[entry->n_wqs - 1]->id, entry->n_wqs - 1, cpu);
+	return (pkg >= 0 && pkg < topology_max_packages()) ? pkg : 0;
 }
 
-static void wq_table_free_entry(int cpu)
+/* Round-robins over the WQs of the caller's package */
+static struct idxd_wq *iaa_next_wq(void)
 {
-	struct wq_table_entry *entry = per_cpu_ptr(wq_table, cpu);
+	struct iaa_pkg_table *table;
 
-	kfree(entry->wqs);
-	memset(entry, 0, sizeof(*entry));
-}
-
-static void wq_table_clear_entry(int cpu)
-{
-	struct wq_table_entry *entry = per_cpu_ptr(wq_table, cpu);
+	table = rcu_dereference(iaa_pkg_tables[iaa_cpu_pkg(raw_smp_processor_id())]);
+	if (!table)
+		return NULL;
 
-	entry->n_wqs = 0;
-	entry->cur_wq = 0;
-	memset(entry->wqs, 0, entry->max_wqs * sizeof(struct idxd_wq *));
+	return table->wqs[this_cpu_inc_return(iaa_wq_idx) % table->n_wqs];
 }
 
 LIST_HEAD(iaa_devices);
@@ -618,16 +592,6 @@ static void del_iaa_wq(struct iaa_device *iaa_device, struct idxd_wq *wq)
 	}
 }
 
-static void clear_wq_table(void)
-{
-	int cpu;
-
-	for (cpu = 0; cpu < nr_cpus; cpu++)
-		wq_table_clear_entry(cpu);
-
-	pr_debug("cleared wq table\n");
-}
-
 static void free_iaa_device(struct iaa_device *iaa_device)
 {
 	if (!iaa_device)
@@ -708,41 +672,19 @@ static int iaa_wq_put(struct idxd_wq *wq)
 	return ret;
 }
 
-static void free_wq_table(void)
-{
-	int cpu;
-
-	for (cpu = 0; cpu < nr_cpus; cpu++)
-		wq_table_free_entry(cpu);
-
-	free_percpu(wq_table);
-
-	pr_debug("freed wq table\n");
-}
-
-static int alloc_wq_table(int max_wqs)
+/* Publishes @tables, or empties every slot if NULL, and frees the old ones */
+static void publish_pkg_tables(struct iaa_pkg_table **tables)
 {
-	struct wq_table_entry *entry;
-	int cpu;
-
-	wq_table = alloc_percpu(struct wq_table_entry);
-	if (!wq_table)
-		return -ENOMEM;
+	struct iaa_pkg_table *old;
+	int pkg;
 
-	for (cpu = 0; cpu < nr_cpus; cpu++) {
-		entry = per_cpu_ptr(wq_table, cpu);
-		entry->wqs = kzalloc_objs(*entry->wqs, max_wqs);
-		if (!entry->wqs) {
-			free_wq_table();
-			return -ENOMEM;
-		}
-
-		entry->max_wqs = max_wqs;
+	for (pkg = 0; pkg < topology_max_packages(); pkg++) {
+		old = rcu_replace_pointer(iaa_pkg_tables[pkg],
+					  tables ? tables[pkg] : NULL,
+					  lockdep_is_held(&iaa_devices_lock));
+		if (old)
+			kfree_rcu(old, rcu);
 	}
-
-	pr_debug("initialized wq table\n");
-
-	return 0;
 }
 
 static int save_iaa_wq(struct idxd_wq *wq)
@@ -806,10 +748,6 @@ static int save_iaa_wq(struct idxd_wq *wq)
 
 	if (WARN_ON(nr_iaa == 0))
 		return -EINVAL;
-
-	cpus_per_iaa = (nr_nodes * nr_cpus_per_node) / nr_iaa;
-	if (!cpus_per_iaa)
-		cpus_per_iaa = 1;
 out:
 	return ret;
 }
@@ -824,116 +762,71 @@ static void remove_iaa_wq(struct idxd_wq *wq)
 			break;
 		}
 	}
-
-	if (nr_iaa) {
-		cpus_per_iaa = (nr_nodes * nr_cpus_per_node) / nr_iaa;
-		if (!cpus_per_iaa)
-			cpus_per_iaa = 1;
-	} else
-		cpus_per_iaa = 1;
 }
 
-static int wq_table_add_wqs(int iaa, int cpu)
+static int iaa_device_pkg(struct iaa_device *iaa_device)
 {
-	struct iaa_device *iaa_device, *found_device = NULL;
-	int ret = 0, cur_iaa = 0, n_wqs_added = 0;
-	struct idxd_device *idxd;
-	struct iaa_wq *iaa_wq;
-	struct pci_dev *pdev;
-	struct device *dev;
-
-	list_for_each_entry(iaa_device, &iaa_devices, list) {
-		idxd = iaa_device->idxd;
-		pdev = idxd->pdev;
-		dev = &pdev->dev;
-
-		if (cur_iaa != iaa) {
-			cur_iaa++;
-			continue;
-		}
+	int node = dev_to_node(&iaa_device->idxd->pdev->dev);
 
-		found_device = iaa_device;
-		dev_dbg(dev, "getting wq from iaa_device %d, cur_iaa %d\n",
-			found_device->idxd->id, cur_iaa);
-		break;
-	}
+	if (node == NUMA_NO_NODE)
+		node = first_online_node;
 
-	if (!found_device) {
-		found_device = list_first_entry_or_null(&iaa_devices,
-							struct iaa_device, list);
-		if (!found_device) {
-			pr_debug("couldn't find any iaa devices with wqs!\n");
-			ret = -EINVAL;
-			goto out;
-		}
-		cur_iaa = 0;
-
-		idxd = found_device->idxd;
-		pdev = idxd->pdev;
-		dev = &pdev->dev;
-		dev_dbg(dev, "getting wq from only iaa_device %d, cur_iaa %d\n",
-			found_device->idxd->id, cur_iaa);
-	}
-
-	list_for_each_entry(iaa_wq, &found_device->wqs, list) {
-		wq_table_add(cpu, iaa_wq->wq);
-		pr_debug("rebalance: added wq for cpu=%d: iaa wq %d.%d\n",
-			 cpu, iaa_wq->wq->idxd->id, iaa_wq->wq->id);
-		n_wqs_added++;
-	}
-
-	if (!n_wqs_added) {
-		pr_debug("couldn't find any iaa wqs!\n");
-		ret = -EINVAL;
-		goto out;
-	}
-out:
-	return ret;
+	return iaa_cpu_pkg(cpumask_first(cpumask_of_node(node)));
 }
 
-/*
- * Rebalance the wq table so that given a cpu, it's easy to find the
- * closest IAA instance.  The idea is to try to choose the most
- * appropriate IAA instance for a caller and spread available
- * workqueues around to clients.
- */
-static void rebalance_wq_table(void)
+static int rebalance_pkg_tables(void)
 {
-	const struct cpumask *node_cpus;
-	int node_cpu, node, cpu, iaa = 0;
+	struct iaa_pkg_table **tables, *table, *first = NULL;
+	struct iaa_device *iaa_device;
+	struct iaa_wq *iaa_wq;
+	int pkg, n_wqs = 0;
 
-	if (nr_iaa == 0)
-		return;
+	if (nr_iaa == 0) {
+		publish_pkg_tables(NULL);
+		return 0;
+	}
 
-	pr_debug("rebalance: nr_nodes=%d, nr_cpus %d, nr_iaa %d, cpus_per_iaa %d\n",
-		 nr_nodes, nr_cpus, nr_iaa, cpus_per_iaa);
+	tables = kzalloc_objs(*tables, topology_max_packages());
+	if (!tables)
+		return -ENOMEM;
 
-	clear_wq_table();
+	list_for_each_entry(iaa_device, &iaa_devices, list)
+		n_wqs += iaa_device->n_wq;
 
-	if (nr_iaa == 1) {
-		for_each_possible_cpu(cpu) {
-			if (WARN_ON(wq_table_add_wqs(0, cpu)))
+	/* Every table has room for all the WQs, a few more than it needs */
+	list_for_each_entry(iaa_device, &iaa_devices, list) {
+		pkg = iaa_device_pkg(iaa_device);
+		if (!tables[pkg]) {
+			tables[pkg] = kzalloc_flex(*tables[pkg], wqs, n_wqs);
+			if (!tables[pkg])
 				goto err;
 		}
-
-		return;
+		table = tables[pkg];
+		first = first ?: table;
+		list_for_each_entry(iaa_wq, &iaa_device->wqs, list)
+			table->wqs[table->n_wqs++] = iaa_wq->wq;
 	}
 
-	cpu = 0;
-	for_each_node_with_cpus(node) {
-		node_cpus = cpumask_of_node(node);
-
-		for_each_cpu(node_cpu, node_cpus) {
-			iaa = cpu / cpus_per_iaa;
-			if (WARN_ON(wq_table_add_wqs(iaa, node_cpu)))
-				goto err;
-			cpu++;
-		}
+	/* A package without IAA devices gets a copy of another one's table */
+	for (pkg = 0; pkg < topology_max_packages(); pkg++) {
+		if (tables[pkg])
+			continue;
+		tables[pkg] = kmemdup(first, struct_size(first, wqs, n_wqs),
+				      GFP_KERNEL);
+		if (!tables[pkg])
+			goto err;
 	}
 
-	return;
+	publish_pkg_tables(tables);
+	kfree(tables);
+
+	return 0;
 err:
-	pr_debug("could not add any wqs for iaa %d to cpu %d!\n", iaa, cpu);
+	for (pkg = 0; pkg < topology_max_packages(); pkg++)
+		kfree(tables[pkg]);
+	kfree(tables);
+
+	return -ENOMEM;
 }
 
 static inline int check_completion(struct device *dev,
@@ -1519,7 +1412,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	struct iaa_compression_ctx *compression_ctx;
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
-	int cpu, ret = 0;
+	int ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct idxd_wq *wq;
 	struct device *dev;
@@ -1544,17 +1437,13 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	if (sg_nents(req->src) > 1 || sg_nents(req->dst) > 1)
 		return deflate_generic_compress(req);
 
-	cpu = get_cpu();
-	wq = wq_table_next_wq(cpu);
-	put_cpu();
-	if (!wq) {
-		pr_debug("no wq configured for cpu=%d\n", cpu);
-		return -ENODEV;
-	}
-
-	ret = iaa_wq_get(wq);
+	/* The table, and its WQs until iaa_wq_get() returns, need RCU */
+	rcu_read_lock();
+	wq = iaa_next_wq();
+	ret = wq ? iaa_wq_get(wq) : -ENODEV;
+	rcu_read_unlock();
 	if (ret) {
-		pr_debug("no wq available for cpu=%d\n", cpu);
+		pr_debug("no wq available\n");
 		return -ENODEV;
 	}
 
@@ -1623,7 +1512,7 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
 	bool use_bounce_src = false;
-	int cpu, ret = 0;
+	int ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct device *dev;
 	struct idxd_wq *wq;
@@ -1653,17 +1542,13 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 		use_bounce_src = true;
 	}
 
-	cpu = get_cpu();
-	wq = wq_table_next_wq(cpu);
-	put_cpu();
-	if (!wq) {
-		pr_debug("no wq configured for cpu=%d\n", cpu);
-		return -ENODEV;
-	}
-
-	ret = iaa_wq_get(wq);
+	/* The table, and its WQs until iaa_wq_get() returns, need RCU */
+	rcu_read_lock();
+	wq = iaa_next_wq();
+	ret = wq ? iaa_wq_get(wq) : -ENODEV;
+	rcu_read_unlock();
 	if (ret) {
-		pr_debug("no wq available for cpu=%d\n", cpu);
+		pr_debug("no wq available\n");
 		return -ENODEV;
 	}
 
@@ -1799,7 +1684,7 @@ static int iaa_crypto_probe(struct idxd_dev *idxd_dev)
 	struct idxd_device *idxd = wq->idxd;
 	struct idxd_driver_data *data = idxd->data;
 	struct device *dev = &idxd_dev->conf_dev;
-	bool first_wq = false;
+	bool first_wq;
 	int ret = 0;
 
 	if (idxd->state != IDXD_DEV_ENABLED)
@@ -1835,18 +1720,15 @@ static int iaa_crypto_probe(struct idxd_dev *idxd_dev)
 
 	mutex_lock(&iaa_devices_lock);
 
-	if (list_empty(&iaa_devices)) {
-		ret = alloc_wq_table(wq->idxd->max_wqs);
-		if (ret)
-			goto err_alloc;
-		first_wq = true;
-	}
+	first_wq = list_empty(&iaa_devices);
 
 	ret = save_iaa_wq(wq);
 	if (ret)
 		goto err_save;
 
-	rebalance_wq_table();
+	ret = rebalance_pkg_tables();
+	if (ret)
+		goto err_remove;
 
 	if (first_wq) {
 		iaa_crypto_enabled = true;
@@ -1854,7 +1736,7 @@ static int iaa_crypto_probe(struct idxd_dev *idxd_dev)
 		if (ret != 0) {
 			iaa_crypto_enabled = false;
 			dev_dbg(dev, "IAA compression device registration failed\n");
-			goto err_register;
+			goto err_remove;
 		}
 		try_module_get(THIS_MODULE);
 
@@ -1867,13 +1749,11 @@ static int iaa_crypto_probe(struct idxd_dev *idxd_dev)
 
 	return ret;
 
-err_register:
+err_remove:
 	remove_iaa_wq(wq);
 	free_iaa_wq(idxd_wq_get_private(wq));
+	rebalance_pkg_tables();
 err_save:
-	if (first_wq)
-		free_wq_table();
-err_alloc:
 	mutex_unlock(&iaa_devices_lock);
 	idxd_drv_disable_wq(wq);
 err:
@@ -1918,11 +1798,15 @@ static void iaa_crypto_remove(struct idxd_dev *idxd_dev)
 	}
 
 	idxd_drv_disable_wq(wq);
-	rebalance_wq_table();
+
+	if (rebalance_pkg_tables()) {
+		/* The old tables still hold the removed WQ */
+		publish_pkg_tables(NULL);
+		pr_warn("%s: failed to rebalance the WQ tables\n", __func__);
+	}
 
 	if (nr_iaa == 0) {
 		iaa_crypto_enabled = false;
-		free_wq_table();
 		module_put(THIS_MODULE);
 
 		pr_info("iaa_crypto now DISABLED\n");
@@ -1930,6 +1814,9 @@ static void iaa_crypto_remove(struct idxd_dev *idxd_dev)
 out:
 	mutex_unlock(&iaa_devices_lock);
 	mutex_unlock(&wq->wq_lock);
+
+	/* The idxd_wq can be freed after this, so wait for old table readers */
+	synchronize_rcu();
 }
 
 static enum idxd_dev_type dev_types[] = {
@@ -1948,16 +1835,15 @@ static struct idxd_device_driver iaa_crypto_driver = {
 static int __init iaa_crypto_init_module(void)
 {
 	int ret = 0;
-	int node;
+	int cpu;
 
-	nr_cpus = num_possible_cpus();
-	for_each_node_with_cpus(node)
-		nr_nodes++;
-	if (!nr_nodes) {
-		pr_err("IAA couldn't find any nodes with cpus\n");
-		return -ENODEV;
-	}
-	nr_cpus_per_node = nr_cpus / nr_nodes;
+	iaa_pkg_tables = kzalloc_objs(*iaa_pkg_tables, topology_max_packages());
+	if (!iaa_pkg_tables)
+		return -ENOMEM;
+
+	/* Start each CPU on a different WQ */
+	for_each_possible_cpu(cpu)
+		per_cpu(iaa_wq_idx, cpu) = cpu;
 
 	ret = iaa_aecs_init_fixed();
 	if (ret < 0) {
@@ -2009,6 +1895,7 @@ static int __init iaa_crypto_init_module(void)
 err_bounce_pool:
 	iaa_aecs_cleanup_fixed();
 err_aecs_init:
+	kfree(iaa_pkg_tables);
 
 	goto out;
 }
@@ -2026,6 +1913,7 @@ static void __exit iaa_crypto_cleanup_module(void)
 	mempool_destroy(iaa_bounce_pool);
 	iaa_bounce_pool = NULL;
 	iaa_aecs_cleanup_fixed();
+	kfree(iaa_pkg_tables);
 
 	pr_debug("cleaned up\n");
 }

-- 
2.56.0

[PATCH 4/4] crypto: iaa - keep each CPU on one WQ for decompression

[Vinicius Costa Gomes]

Decompressions round-robin over the package's WQs like compressions
do. Decompressions are latency-sensitive, and do better staying on a
warm engine.

Have each CPU pick one WQ of its package's table the first time it
decompresses, spread over the table by a per-table counter, and stay on
it.

The CPU caches the index, not the WQ, so it cannot outlive the table it
came from; rebalance makes every CPU pick again.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/crypto/intel/iaa/iaa_crypto_main.c | 40 +++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
index 2ce545e8df42..4f2c99c099df 100644
--- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
+++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
@@ -35,12 +35,15 @@ static unsigned int nr_iaa;
 struct iaa_pkg_table {
 	struct rcu_head	rcu;
 	int		n_wqs;
+	atomic_t	decomp_next;
 	struct idxd_wq	*wqs[];
 };
 
 /* Indexed by logical package id, the count is fixed at boot */
 static struct iaa_pkg_table __rcu **iaa_pkg_tables;
 static DEFINE_PER_CPU(unsigned int, iaa_wq_idx);
+/* The WQ of its package's table a CPU decompresses on, -1 until it picks */
+static DEFINE_PER_CPU(int, iaa_decomp_wq_idx) = -1;
 
 static int iaa_cpu_pkg(int cpu)
 {
@@ -49,16 +52,35 @@ static int iaa_cpu_pkg(int cpu)
 	return (pkg >= 0 && pkg < topology_max_packages()) ? pkg : 0;
 }
 
-/* Round-robins over the WQs of the caller's package */
-static struct idxd_wq *iaa_next_wq(void)
+/* Pick the idx of the WQ on first usage on each CPU */
+static unsigned int iaa_decomp_wq_pick(struct iaa_pkg_table *table)
+{
+	int idx = this_cpu_read(iaa_decomp_wq_idx);
+
+	/* An index left over from a wider table just means picking again */
+	if (unlikely(idx < 0 || idx >= table->n_wqs)) {
+		idx = (unsigned int)atomic_inc_return(&table->decomp_next) %
+		      table->n_wqs;
+		this_cpu_write(iaa_decomp_wq_idx, idx);
+	}
+
+	return idx;
+}
+
+/* Compression round-robins over the package's WQs, decompression keeps one */
+static struct idxd_wq *iaa_next_wq(bool decomp)
 {
 	struct iaa_pkg_table *table;
+	unsigned int idx;
 
 	table = rcu_dereference(iaa_pkg_tables[iaa_cpu_pkg(raw_smp_processor_id())]);
 	if (!table)
 		return NULL;
 
-	return table->wqs[this_cpu_inc_return(iaa_wq_idx) % table->n_wqs];
+	idx = decomp ? iaa_decomp_wq_pick(table) :
+		       this_cpu_inc_return(iaa_wq_idx) % table->n_wqs;
+
+	return table->wqs[idx];
 }
 
 LIST_HEAD(iaa_devices);
@@ -779,7 +801,7 @@ static int rebalance_pkg_tables(void)
 	struct iaa_pkg_table **tables, *table, *first = NULL;
 	struct iaa_device *iaa_device;
 	struct iaa_wq *iaa_wq;
-	int pkg, n_wqs = 0;
+	int cpu, pkg, n_wqs = 0;
 
 	if (nr_iaa == 0) {
 		publish_pkg_tables(NULL);
@@ -820,6 +842,10 @@ static int rebalance_pkg_tables(void)
 	publish_pkg_tables(tables);
 	kfree(tables);
 
+	/* Have every CPU pick its decompression WQ again, from the new table */
+	for_each_possible_cpu(cpu)
+		WRITE_ONCE(per_cpu(iaa_decomp_wq_idx, cpu), -1);
+
 	return 0;
 err:
 	for (pkg = 0; pkg < topology_max_packages(); pkg++)
@@ -1412,6 +1438,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 	struct iaa_compression_ctx *compression_ctx;
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
+	bool decomp = false;
 	int ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct idxd_wq *wq;
@@ -1439,7 +1466,7 @@ static int iaa_comp_acompress(struct acomp_req *req)
 
 	/* The table, and its WQs until iaa_wq_get() returns, need RCU */
 	rcu_read_lock();
-	wq = iaa_next_wq();
+	wq = iaa_next_wq(decomp);
 	ret = wq ? iaa_wq_get(wq) : -ENODEV;
 	rcu_read_unlock();
 	if (ret) {
@@ -1512,6 +1539,7 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 	struct crypto_tfm *tfm = req->base.tfm;
 	dma_addr_t src_addr, dst_addr;
 	bool use_bounce_src = false;
+	bool decomp = true;
 	int ret = 0;
 	struct iaa_wq *iaa_wq;
 	struct device *dev;
@@ -1544,7 +1572,7 @@ static int iaa_comp_adecompress(struct acomp_req *req)
 
 	/* The table, and its WQs until iaa_wq_get() returns, need RCU */
 	rcu_read_lock();
-	wq = iaa_next_wq();
+	wq = iaa_next_wq(decomp);
 	ret = wq ? iaa_wq_get(wq) : -ENODEV;
 	rcu_read_unlock();
 	if (ret) {

-- 
2.56.0

Re: [PATCH 3/4] crypto: iaa - use RCU per-package WQ tables

[Herbert Xu]

On Mon, Oct 19, 2026 at 04:06:22PM -0700, Vinicius Costa Gomes wrote:
> rebalance_wq_table() gives CPU n all the WQs of IAA device
> n / cpus_per_iaa. The flat index ignores package boundaries, so a CPU
> can be paired with a device on another package, and each CPU sends all
> its requests to one device while the other devices of its package stay
> idle. The table is also rebuilt in place, so a request issued during a
> probe or remove can read a NULL WQ and fail with -ENODEV.
> 
> Replace it with one table per package, holding the WQs of every IAA
> device on that package, and have each CPU round-robin over its package's
> table. Rebalance builds the whole set of tables and publishes it with
> rcu_replace_pointer(). The submit paths look the WQ up, and take their
> reference on it, inside an RCU read-side section.
> 
> - A package without IAA devices gets a copy of another package's table,
>   so the lookup needs no fallback.
> - Every table has room for all the WQs, a few pointers more than it
>   needs, so they can be sized without counting per package.
> - If a rebalance fails, the current tables are kept. On removal they
>   are cleared instead, because they still hold the removed WQ. Removal
>   then waits for a grace period before returning, because the idxd_wq
>   can be freed after that.
> 
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/crypto/intel/iaa/iaa_crypto.h      |   7 -
>  drivers/crypto/intel/iaa/iaa_crypto_main.c | 346 ++++++++++-------------------
>  2 files changed, 117 insertions(+), 236 deletions(-)

Please respond to this comment:

https://sashiko.dev/#/patchset/20261019-iaa-crypto-wq-tables-v1-0-fab67b2a1538%40intel.com

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 3/4] crypto: iaa - use RCU per-package WQ tables

[Vinicius Costa Gomes]

Hi,

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Mon, Oct 19, 2026 at 04:06:22PM -0700, Vinicius Costa Gomes wrote:
>> rebalance_wq_table() gives CPU n all the WQs of IAA device
>> n / cpus_per_iaa. The flat index ignores package boundaries, so a CPU
>> can be paired with a device on another package, and each CPU sends all
>> its requests to one device while the other devices of its package stay
>> idle. The table is also rebuilt in place, so a request issued during a
>> probe or remove can read a NULL WQ and fail with -ENODEV.
>> 
>> Replace it with one table per package, holding the WQs of every IAA
>> device on that package, and have each CPU round-robin over its package's
>> table. Rebalance builds the whole set of tables and publishes it with
>> rcu_replace_pointer(). The submit paths look the WQ up, and take their
>> reference on it, inside an RCU read-side section.
>> 
>> - A package without IAA devices gets a copy of another package's table,
>>   so the lookup needs no fallback.
>> - Every table has room for all the WQs, a few pointers more than it
>>   needs, so they can be sized without counting per package.
>> - If a rebalance fails, the current tables are kept. On removal they
>>   are cleared instead, because they still hold the removed WQ. Removal
>>   then waits for a grace period before returning, because the idxd_wq
>>   can be freed after that.
>> 
>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>> ---
>>  drivers/crypto/intel/iaa/iaa_crypto.h      |   7 -
>>  drivers/crypto/intel/iaa/iaa_crypto_main.c | 346 ++++++++++-------------------
>>  2 files changed, 117 insertions(+), 236 deletions(-)
>
> Please respond to this comment:
>
> https://sashiko.dev/#/patchset/20261019-iaa-crypto-wq-tables-v1-0-fab67b2a1538%40intel.com
>

Weird that I haven't received any notification about these comments.

Anyway, as I moved some code around, sashiko considers it a "new" issue,
but from a quick look, it's a pre-existing one. Doesn't seem too hard to
fix, will include the fix on v2.


Cheers,
-- 
Vinicius