Avoid using __sync builtins and instead prefer the kernel's own facilities: See comments below for suggestions, preserving the assumed memory ordering requirements (but please double-check). By using atomic_t instead of __sync, you'd also avoid any data races due to plain concurrent accesses. Reported in: https://lore.kernel.org/linux-crypto/CANpmjNM2b26Oo6k-4EqfrJf1sBj3WoFf-NQnwsLr3EW9B=G8kw@xxxxxxxxxxxxxx/ On Wed, 13 Nov 2019, Zaibo Xu wrote: > SEC driver provides PCIe hardware device initiation with > AES, SM4, and 3DES skcipher algorithms registered to Crypto. > It uses Hisilicon QM as interface to CPU. > > Signed-off-by: Zaibo Xu <xuzaibo@xxxxxxxxxx> > Signed-off-by: Longfang Liu <liulongfang@xxxxxxxxxx> > --- > drivers/crypto/hisilicon/Kconfig | 16 + > drivers/crypto/hisilicon/Makefile | 1 + > drivers/crypto/hisilicon/sec2/Makefile | 2 + > drivers/crypto/hisilicon/sec2/sec.h | 132 +++++ > drivers/crypto/hisilicon/sec2/sec_crypto.c | 886 +++++++++++++++++++++++++++++ > drivers/crypto/hisilicon/sec2/sec_crypto.h | 198 +++++++ > drivers/crypto/hisilicon/sec2/sec_main.c | 640 +++++++++++++++++++++ > 7 files changed, 1875 insertions(+) > create mode 100644 drivers/crypto/hisilicon/sec2/Makefile > create mode 100644 drivers/crypto/hisilicon/sec2/sec.h > create mode 100644 drivers/crypto/hisilicon/sec2/sec_crypto.c > create mode 100644 drivers/crypto/hisilicon/sec2/sec_crypto.h > create mode 100644 drivers/crypto/hisilicon/sec2/sec_main.c [...] > diff --git a/drivers/crypto/hisilicon/sec2/sec.h b/drivers/crypto/hisilicon/sec2/sec.h > new file mode 100644 > index 0000000..443b6c5 > --- /dev/null > +++ b/drivers/crypto/hisilicon/sec2/sec.h > @@ -0,0 +1,132 @@ [...] > + > +/* SEC request of Crypto */ > +struct sec_req { > + struct sec_sqe sec_sqe; > + struct sec_ctx *ctx; > + struct sec_qp_ctx *qp_ctx; > + > + /* Cipher supported only at present */ > + struct sec_cipher_req c_req; > + int err_type; > + int req_id; > + > + /* Status of the SEC request */ > + int fake_busy; This could be atomic_t fake_busy; > +}; > + [...] > diff --git a/drivers/crypto/hisilicon/sec2/sec_crypto.c b/drivers/crypto/hisilicon/sec2/sec_crypto.c > new file mode 100644 > index 0000000..23092a9 > --- /dev/null > +++ b/drivers/crypto/hisilicon/sec2/sec_crypto.c > @@ -0,0 +1,886 @@ Add #include <linux/atomic.h> [...] > +static int sec_bd_send(struct sec_ctx *ctx, struct sec_req *req) > +{ > + struct sec_qp_ctx *qp_ctx = req->qp_ctx; > + int ret; > + > + mutex_lock(&qp_ctx->req_lock); > + ret = hisi_qp_send(qp_ctx->qp, &req->sec_sqe); > + mutex_unlock(&qp_ctx->req_lock); > + > + if (ret == -EBUSY) > + return -ENOBUFS; > + > + if (!ret) { > + if (req->fake_busy) This could be: atomic_read(&req->fake_busy) > + ret = -EBUSY; > + else > + ret = -EINPROGRESS; > + } > + > + return ret; > +} [...] > +static void sec_skcipher_callback(struct sec_ctx *ctx, struct sec_req *req) > +{ > + struct skcipher_request *sk_req = req->c_req.sk_req; > + struct sec_qp_ctx *qp_ctx = req->qp_ctx; > + > + atomic_dec(&qp_ctx->pending_reqs); > + sec_free_req_id(req); > + > + /* IV output at encrypto of CBC mode */ > + if (ctx->c_ctx.c_mode == SEC_CMODE_CBC && req->c_req.encrypt) > + sec_update_iv(req); > + > + if (__sync_bool_compare_and_swap(&req->fake_busy, 1, 0)) This could be: int expect_val = 1; ... if (atomic_try_cmpxchg_relaxed(&req->fake_busy, &expect_val, 0)) > + sk_req->base.complete(&sk_req->base, -EINPROGRESS); > + > + sk_req->base.complete(&sk_req->base, req->err_type); > +} > + > +static void sec_request_uninit(struct sec_ctx *ctx, struct sec_req *req) > +{ > + struct sec_qp_ctx *qp_ctx = req->qp_ctx; > + > + atomic_dec(&qp_ctx->pending_reqs); > + sec_free_req_id(req); > + sec_put_queue_id(ctx, req); > +} > + > +static int sec_request_init(struct sec_ctx *ctx, struct sec_req *req) > +{ > + struct sec_qp_ctx *qp_ctx; > + int issue_id, ret; > + > + /* To load balance */ > + issue_id = sec_get_queue_id(ctx, req); > + qp_ctx = &ctx->qp_ctx[issue_id]; > + > + req->req_id = sec_alloc_req_id(req, qp_ctx); > + if (req->req_id < 0) { > + sec_put_queue_id(ctx, req); > + return req->req_id; > + } > + > + if (ctx->fake_req_limit <= atomic_inc_return(&qp_ctx->pending_reqs)) > + req->fake_busy = 1; > + else > + req->fake_busy = 0; These could be: atomic_set(&req->fake_busy, ...) > + > + ret = ctx->req_op->get_res(ctx, req); > + if (ret) { > + atomic_dec(&qp_ctx->pending_reqs); > + sec_request_uninit(ctx, req); > + dev_err(SEC_CTX_DEV(ctx), "get resources failed!\n"); > + } > + > + return ret; > +} Thanks, -- Marco