From: Corentin Labbe <clabbe.montjoie@gmail.com>
To: Iuliana Prodan <iuliana.prodan@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
Baolin Wang <baolin.wang@linaro.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Horia Geanta <horia.geanta@nxp.com>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@st.com>,
Maxime Ripard <mripard@kernel.org>,
Aymen Sghaier <aymen.sghaier@nxp.com>,
"David S. Miller" <davem@davemloft.net>,
Silvano Di Ninno <silvano.dininno@nxp.com>,
Franck Lenormand <franck.lenormand@nxp.com>,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH v2 1/2] crypto: engine - support for parallel requests
Date: Wed, 5 Feb 2020 20:11:28 +0100 [thread overview]
Message-ID: <20200205191128.GA32606@Red> (raw)
In-Reply-To: <1580819660-30211-2-git-send-email-iuliana.prodan@nxp.com>
On Tue, Feb 04, 2020 at 02:34:19PM +0200, Iuliana Prodan wrote:
> Added support for executing multiple requests, in parallel,
> for crypto engine.
> A new callback is added, can_enqueue_more, which asks the
> driver if the hardware has free space, to enqueue a new request.
> The new crypto_engine_alloc_init_and_set function, initialize
> crypto-engine, sets the maximum size for crypto-engine software
> queue (not hardcoded anymore) and the can_enqueue_more callback.
> On crypto_pump_requests, if can_enqueue_more callback returns true,
> a new request is send to hardware, until there is no space and the
> callback returns false.
>
> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> ---
> crypto/crypto_engine.c | 106 ++++++++++++++++++++++++++++++------------------
> include/crypto/engine.h | 10 +++--
> 2 files changed, 72 insertions(+), 44 deletions(-)
>
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> index eb029ff..aba934f 100644
> --- a/crypto/crypto_engine.c
> +++ b/crypto/crypto_engine.c
> @@ -22,32 +22,18 @@
> * @err: error number
> */
> static void crypto_finalize_request(struct crypto_engine *engine,
> - struct crypto_async_request *req, int err)
> + struct crypto_async_request *req, int err)
> {
> - unsigned long flags;
> - bool finalize_cur_req = false;
> int ret;
> struct crypto_engine_ctx *enginectx;
>
> - spin_lock_irqsave(&engine->queue_lock, flags);
> - if (engine->cur_req == req)
> - finalize_cur_req = true;
> - spin_unlock_irqrestore(&engine->queue_lock, flags);
> -
> - if (finalize_cur_req) {
> - enginectx = crypto_tfm_ctx(req->tfm);
> - if (engine->cur_req_prepared &&
> - enginectx->op.unprepare_request) {
> - ret = enginectx->op.unprepare_request(engine, req);
> - if (ret)
> - dev_err(engine->dev, "failed to unprepare request\n");
> - }
> - spin_lock_irqsave(&engine->queue_lock, flags);
> - engine->cur_req = NULL;
> - engine->cur_req_prepared = false;
> - spin_unlock_irqrestore(&engine->queue_lock, flags);
> + enginectx = crypto_tfm_ctx(req->tfm);
> + if (enginectx->op.prepare_request &&
> + enginectx->op.unprepare_request) {
> + ret = enginectx->op.unprepare_request(engine, req);
> + if (ret)
> + dev_err(engine->dev, "failed to unprepare request\n");
> }
> -
> req->complete(req, err);
>
> kthread_queue_work(engine->kworker, &engine->pump_requests);
> @@ -73,10 +59,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>
> spin_lock_irqsave(&engine->queue_lock, flags);
>
> - /* Make sure we are not already running a request */
> - if (engine->cur_req)
> - goto out;
> -
Hello
Your patch has the same problem than mine reported by Horia.
If a queue has more than one request, a first crypto_pump_requests() will send a request and for drivers which do not block on do_one_request() crypto_pump_requests() will end.
Then another crypto_pump_requests() will fire sending a second request while the driver does not support that.
So we need to replace engine->cur_req by another locking mechanism.
Perhaps the cleaner is to add a "request count" (increased when do_one_request, decreased in crypto_finalize_request)
I know that the early version have that and it was removed, but I do not see any better way.
Regards
next prev parent reply other threads:[~2020-02-05 19:11 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-04 12:34 [PATCH v2 0/2] crypto: engine - support for parallel and batch requests Iuliana Prodan
2020-02-04 12:34 ` [PATCH v2 1/2] crypto: engine - support for parallel requests Iuliana Prodan
2020-02-05 19:11 ` Corentin Labbe [this message]
2020-02-07 11:26 ` Iuliana Prodan
2020-02-07 12:17 ` Corentin Labbe
2020-02-04 12:34 ` [PATCH v2 2/2] crypto: engine - support for batch requests Iuliana Prodan
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=20200205191128.GA32606@Red \
--to=clabbe.montjoie@gmail.com \
--cc=alexandre.torgue@st.com \
--cc=ard.biesheuvel@linaro.org \
--cc=aymen.sghaier@nxp.com \
--cc=baolin.wang@linaro.org \
--cc=davem@davemloft.net \
--cc=franck.lenormand@nxp.com \
--cc=herbert@gondor.apana.org.au \
--cc=horia.geanta@nxp.com \
--cc=iuliana.prodan@nxp.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mcoquelin.stm32@gmail.com \
--cc=mripard@kernel.org \
--cc=silvano.dininno@nxp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).