crypto: iaa - rework the WQ selection tables
Subsystems (2)
Vinicius Costa Gomes
maintainercover crypto: iaa - rework the WQ selection tables
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.
---
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,
Signature · 1 line
Vinicius
Vinicius Costa Gomes
maintainerPATCH 1/4 crypto: iaa - carry the iaa_wq in the request
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.
--- drivers/crypto/intel/iaa/iaa_crypto_main.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/crypto/intel/iaa/iaa_crypto_main.c +8 -4
@@ -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;
Signature · 1 line
2.56.0
Vinicius Costa Gomes
maintainerPATCH 2/4 crypto: iaa - take the WQ from the request in the submit helpers
In reply to Vinicius Costa Gomesiaa_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.
--- drivers/crypto/intel/iaa/iaa_crypto_main.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/crypto/intel/iaa/iaa_crypto_main.c +11 -10
@@ -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;
Signature · 1 line
2.56.0
Vinicius Costa Gomes
maintainerPATCH 3/4 crypto: iaa - use RCU per-package WQ tables
In reply to Vinicius Costa Gomesrebalance_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.
--- drivers/crypto/intel/iaa/iaa_crypto.h | 7 - drivers/crypto/intel/iaa/iaa_crypto_main.c | 346 ++++++++++------------------- 2 files changed, 117 insertions(+), 236 deletions(-)
drivers/crypto/intel/iaa/iaa_crypto.h +0 -7
@@ -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/*
drivers/crypto/intel/iaa/iaa_crypto_main.c +117 -229
507 lines not on this page · raw message
Signature · 1 line
2.56.0
Vinicius Costa Gomes
maintainerPATCH 4/4 crypto: iaa - keep each CPU on one WQ for decompression
In reply to Vinicius Costa GomesDecompressions 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.
--- drivers/crypto/intel/iaa/iaa_crypto_main.c | 40 +++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 6 deletions(-)
drivers/crypto/intel/iaa/iaa_crypto_main.c +34 -6
@@ -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) {
Signature · 1 line
2.56.0
Herbert Xu
maintainerRe: [PATCH 3/4] crypto: iaa - use RCU per-package WQ tables
In reply to Vinicius Costa GomesQuoted 22 lines · 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 theidxd_wqcan be freed after that.
Quoted 5 lines
Signed-off-by Vinicius Costa Gomes <[email protected]>--- 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,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt