| Message ID | 20220424104140.44841-2-pizhenwei@bytedance.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | virtio-crypto: Improve performance | expand |
On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <pizhenwei@bytedance.com> wrote: > > Use temporary variable to make code easy to read and maintain. > /* Pad cipher's parameters */ > vcrypto->ctrl.u.sym_create_session.op_type = > cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); > vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo = > vcrypto->ctrl.header.algo; > vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen = > cpu_to_le32(keylen); > vcrypto->ctrl.u.sym_create_session.u.cipher.para.op = > cpu_to_le32(op); > --> > sym_create_session = &ctrl->u.sym_create_session; > sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); > sym_create_session->u.cipher.para.algo = ctrl->header.algo; > sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen); > sym_create_session->u.cipher.para.op = cpu_to_le32(op); > > The new style shows more obviously: > - the variable we want to operate. > - an assignment statement in a single line. Still hundreds of lines of changes, I'd leave this change to other mainters to dedice. Thanks > > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Gonglei <arei.gonglei@huawei.com> > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> > --- > .../virtio/virtio_crypto_akcipher_algs.c | 40 ++++++----- > .../virtio/virtio_crypto_skcipher_algs.c | 72 +++++++++---------- > 2 files changed, 59 insertions(+), 53 deletions(-) > > diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > index f3ec9420215e..20901a263fc8 100644 > --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c > @@ -106,23 +106,27 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher > unsigned int inlen; > int err; > unsigned int num_out = 0, num_in = 0; > + struct virtio_crypto_op_ctrl_req *ctrl; > + struct virtio_crypto_session_input *input; > > pkey = kmemdup(key, keylen, GFP_ATOMIC); > if (!pkey) > return -ENOMEM; > > spin_lock(&vcrypto->ctrl_lock); > - memcpy(&vcrypto->ctrl.header, header, sizeof(vcrypto->ctrl.header)); > - memcpy(&vcrypto->ctrl.u, para, sizeof(vcrypto->ctrl.u)); > - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR); > + ctrl = &vcrypto->ctrl; > + memcpy(&ctrl->header, header, sizeof(ctrl->header)); > + memcpy(&ctrl->u, para, sizeof(ctrl->u)); > + input = &vcrypto->input; > + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR); > > - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); > + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl)); > sgs[num_out++] = &outhdr_sg; > > sg_init_one(&key_sg, pkey, keylen); > sgs[num_out++] = &key_sg; > > - sg_init_one(&inhdr_sg, &vcrypto->input, sizeof(vcrypto->input)); > + sg_init_one(&inhdr_sg, input, sizeof(*input)); > sgs[num_out + num_in++] = &inhdr_sg; > > err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC); > @@ -134,12 +138,12 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher > !virtqueue_is_broken(vcrypto->ctrl_vq)) > cpu_relax(); > > - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) { > + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) { > err = -EINVAL; > goto out; > } > > - ctx->session_id = le64_to_cpu(vcrypto->input.session_id); > + ctx->session_id = le64_to_cpu(input->session_id); > ctx->session_valid = true; > err = 0; > > @@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher > > if (err < 0) > pr_err("virtio_crypto: Create session failed status: %u\n", > - le32_to_cpu(vcrypto->input.status)); > + le32_to_cpu(input->status)); > > return err; > } > @@ -161,23 +165,27 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe > struct virtio_crypto *vcrypto = ctx->vcrypto; > unsigned int num_out = 0, num_in = 0, inlen; > int err; > + struct virtio_crypto_op_ctrl_req *ctrl; > + struct virtio_crypto_inhdr *ctrl_status; > > spin_lock(&vcrypto->ctrl_lock); > if (!ctx->session_valid) { > err = 0; > goto out; > } > - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR; > - vcrypto->ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION); > - vcrypto->ctrl.header.queue_id = 0; > + ctrl_status = &vcrypto->ctrl_status; > + ctrl_status->status = VIRTIO_CRYPTO_ERR; > + ctrl = &vcrypto->ctrl; > + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION); > + ctrl->header.queue_id = 0; > > - destroy_session = &vcrypto->ctrl.u.destroy_session; > + destroy_session = &ctrl->u.destroy_session; > destroy_session->session_id = cpu_to_le64(ctx->session_id); > > - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); > + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl)); > sgs[num_out++] = &outhdr_sg; > > - sg_init_one(&inhdr_sg, &vcrypto->ctrl_status.status, sizeof(vcrypto->ctrl_status.status)); > + sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status)); > sgs[num_out + num_in++] = &inhdr_sg; > > err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC); > @@ -189,7 +197,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe > !virtqueue_is_broken(vcrypto->ctrl_vq)) > cpu_relax(); > > - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) { > + if (ctrl_status->status != VIRTIO_CRYPTO_OK) { > err = -EINVAL; > goto out; > } > @@ -201,7 +209,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe > spin_unlock(&vcrypto->ctrl_lock); > if (err < 0) { > pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n", > - vcrypto->ctrl_status.status, destroy_session->session_id); > + ctrl_status->status, destroy_session->session_id); > } > > return err; > diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c > index a618c46a52b8..e3c5bc8d6112 100644 > --- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c > +++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c > @@ -123,6 +123,9 @@ static int virtio_crypto_alg_skcipher_init_session( > int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT; > int err; > unsigned int num_out = 0, num_in = 0; > + struct virtio_crypto_op_ctrl_req *ctrl; > + struct virtio_crypto_session_input *input; > + struct virtio_crypto_sym_create_session_req *sym_create_session; > > /* > * Avoid to do DMA from the stack, switch to using > @@ -135,24 +138,22 @@ static int virtio_crypto_alg_skcipher_init_session( > > spin_lock(&vcrypto->ctrl_lock); > /* Pad ctrl header */ > - vcrypto->ctrl.header.opcode = > - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION); > - vcrypto->ctrl.header.algo = cpu_to_le32(alg); > + ctrl = &vcrypto->ctrl; > + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION); > + ctrl->header.algo = cpu_to_le32(alg); > /* Set the default dataqueue id to 0 */ > - vcrypto->ctrl.header.queue_id = 0; > + ctrl->header.queue_id = 0; > > - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR); > + input = &vcrypto->input; > + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR); > /* Pad cipher's parameters */ > - vcrypto->ctrl.u.sym_create_session.op_type = > - cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); > - vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo = > - vcrypto->ctrl.header.algo; > - vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen = > - cpu_to_le32(keylen); > - vcrypto->ctrl.u.sym_create_session.u.cipher.para.op = > - cpu_to_le32(op); > - > - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); > + sym_create_session = &ctrl->u.sym_create_session; > + sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); > + sym_create_session->u.cipher.para.algo = ctrl->header.algo; > + sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen); > + sym_create_session->u.cipher.para.op = cpu_to_le32(op); > + > + sg_init_one(&outhdr, ctrl, sizeof(*ctrl)); > sgs[num_out++] = &outhdr; > > /* Set key */ > @@ -160,7 +161,7 @@ static int virtio_crypto_alg_skcipher_init_session( > sgs[num_out++] = &key_sg; > > /* Return status and session id back */ > - sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input)); > + sg_init_one(&inhdr, input, sizeof(*input)); > sgs[num_out + num_in++] = &inhdr; > > err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, > @@ -180,20 +181,18 @@ static int virtio_crypto_alg_skcipher_init_session( > !virtqueue_is_broken(vcrypto->ctrl_vq)) > cpu_relax(); > > - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) { > + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) { > spin_unlock(&vcrypto->ctrl_lock); > pr_err("virtio_crypto: Create session failed status: %u\n", > - le32_to_cpu(vcrypto->input.status)); > + le32_to_cpu(input->status)); > kfree_sensitive(cipher_key); > return -EINVAL; > } > > if (encrypt) > - ctx->enc_sess_info.session_id = > - le64_to_cpu(vcrypto->input.session_id); > + ctx->enc_sess_info.session_id = le64_to_cpu(input->session_id); > else > - ctx->dec_sess_info.session_id = > - le64_to_cpu(vcrypto->input.session_id); > + ctx->dec_sess_info.session_id = le64_to_cpu(input->session_id); > > spin_unlock(&vcrypto->ctrl_lock); > > @@ -211,30 +210,30 @@ static int virtio_crypto_alg_skcipher_close_session( > struct virtio_crypto *vcrypto = ctx->vcrypto; > int err; > unsigned int num_out = 0, num_in = 0; > + struct virtio_crypto_op_ctrl_req *ctrl; > + struct virtio_crypto_inhdr *ctrl_status; > > spin_lock(&vcrypto->ctrl_lock); > - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR; > + ctrl_status = &vcrypto->ctrl_status; > + ctrl_status->status = VIRTIO_CRYPTO_ERR; > /* Pad ctrl header */ > - vcrypto->ctrl.header.opcode = > - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION); > + ctrl = &vcrypto->ctrl; > + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION); > /* Set the default virtqueue id to 0 */ > - vcrypto->ctrl.header.queue_id = 0; > + ctrl->header.queue_id = 0; > > - destroy_session = &vcrypto->ctrl.u.destroy_session; > + destroy_session = &ctrl->u.destroy_session; > > if (encrypt) > - destroy_session->session_id = > - cpu_to_le64(ctx->enc_sess_info.session_id); > + destroy_session->session_id = cpu_to_le64(ctx->enc_sess_info.session_id); > else > - destroy_session->session_id = > - cpu_to_le64(ctx->dec_sess_info.session_id); > + destroy_session->session_id = cpu_to_le64(ctx->dec_sess_info.session_id); > > - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); > + sg_init_one(&outhdr, ctrl, sizeof(*ctrl)); > sgs[num_out++] = &outhdr; > > /* Return status and session id back */ > - sg_init_one(&status_sg, &vcrypto->ctrl_status.status, > - sizeof(vcrypto->ctrl_status.status)); > + sg_init_one(&status_sg, &ctrl_status->status, sizeof(ctrl_status->status)); > sgs[num_out + num_in++] = &status_sg; > > err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, > @@ -249,11 +248,10 @@ static int virtio_crypto_alg_skcipher_close_session( > !virtqueue_is_broken(vcrypto->ctrl_vq)) > cpu_relax(); > > - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) { > + if (ctrl_status->status != VIRTIO_CRYPTO_OK) { > spin_unlock(&vcrypto->ctrl_lock); > pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n", > - vcrypto->ctrl_status.status, > - destroy_session->session_id); > + ctrl_status->status, destroy_session->session_id); > > return -EINVAL; > } > -- > 2.20.1 >
On 4/26/22 14:12, Jason Wang wrote: > On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <pizhenwei@bytedance.com> wrote: >> >> Use temporary variable to make code easy to read and maintain. >> /* Pad cipher's parameters */ >> vcrypto->ctrl.u.sym_create_session.op_type = >> cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); >> vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo = >> vcrypto->ctrl.header.algo; >> vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen = >> cpu_to_le32(keylen); >> vcrypto->ctrl.u.sym_create_session.u.cipher.para.op = >> cpu_to_le32(op); >> --> >> sym_create_session = &ctrl->u.sym_create_session; >> sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); >> sym_create_session->u.cipher.para.algo = ctrl->header.algo; >> sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen); >> sym_create_session->u.cipher.para.op = cpu_to_le32(op); >> >> The new style shows more obviously: >> - the variable we want to operate. >> - an assignment statement in a single line. > > Still hundreds of lines of changes, I'd leave this change to other > mainters to dedice. > > Thanks > Thanks to Jason! Hi, Lei What's your opinion? >> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Gonglei <arei.gonglei@huawei.com> >> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> >> --- >> .../virtio/virtio_crypto_akcipher_algs.c | 40 ++++++----- >> .../virtio/virtio_crypto_skcipher_algs.c | 72 +++++++++---------- >> 2 files changed, 59 insertions(+), 53 deletions(-) >> >> diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c >> index f3ec9420215e..20901a263fc8 100644 >> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c >> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c >> @@ -106,23 +106,27 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher >> unsigned int inlen; >> int err; >> unsigned int num_out = 0, num_in = 0; >> + struct virtio_crypto_op_ctrl_req *ctrl; >> + struct virtio_crypto_session_input *input; >> >> pkey = kmemdup(key, keylen, GFP_ATOMIC); >> if (!pkey) >> return -ENOMEM; >> >> spin_lock(&vcrypto->ctrl_lock); >> - memcpy(&vcrypto->ctrl.header, header, sizeof(vcrypto->ctrl.header)); >> - memcpy(&vcrypto->ctrl.u, para, sizeof(vcrypto->ctrl.u)); >> - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR); >> + ctrl = &vcrypto->ctrl; >> + memcpy(&ctrl->header, header, sizeof(ctrl->header)); >> + memcpy(&ctrl->u, para, sizeof(ctrl->u)); >> + input = &vcrypto->input; >> + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR); >> >> - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); >> + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl)); >> sgs[num_out++] = &outhdr_sg; >> >> sg_init_one(&key_sg, pkey, keylen); >> sgs[num_out++] = &key_sg; >> >> - sg_init_one(&inhdr_sg, &vcrypto->input, sizeof(vcrypto->input)); >> + sg_init_one(&inhdr_sg, input, sizeof(*input)); >> sgs[num_out + num_in++] = &inhdr_sg; >> >> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC); >> @@ -134,12 +138,12 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher >> !virtqueue_is_broken(vcrypto->ctrl_vq)) >> cpu_relax(); >> >> - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) { >> + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) { >> err = -EINVAL; >> goto out; >> } >> >> - ctx->session_id = le64_to_cpu(vcrypto->input.session_id); >> + ctx->session_id = le64_to_cpu(input->session_id); >> ctx->session_valid = true; >> err = 0; >> >> @@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher >> >> if (err < 0) >> pr_err("virtio_crypto: Create session failed status: %u\n", >> - le32_to_cpu(vcrypto->input.status)); >> + le32_to_cpu(input->status)); >> >> return err; >> } >> @@ -161,23 +165,27 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe >> struct virtio_crypto *vcrypto = ctx->vcrypto; >> unsigned int num_out = 0, num_in = 0, inlen; >> int err; >> + struct virtio_crypto_op_ctrl_req *ctrl; >> + struct virtio_crypto_inhdr *ctrl_status; >> >> spin_lock(&vcrypto->ctrl_lock); >> if (!ctx->session_valid) { >> err = 0; >> goto out; >> } >> - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR; >> - vcrypto->ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION); >> - vcrypto->ctrl.header.queue_id = 0; >> + ctrl_status = &vcrypto->ctrl_status; >> + ctrl_status->status = VIRTIO_CRYPTO_ERR; >> + ctrl = &vcrypto->ctrl; >> + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION); >> + ctrl->header.queue_id = 0; >> >> - destroy_session = &vcrypto->ctrl.u.destroy_session; >> + destroy_session = &ctrl->u.destroy_session; >> destroy_session->session_id = cpu_to_le64(ctx->session_id); >> >> - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); >> + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl)); >> sgs[num_out++] = &outhdr_sg; >> >> - sg_init_one(&inhdr_sg, &vcrypto->ctrl_status.status, sizeof(vcrypto->ctrl_status.status)); >> + sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status)); >> sgs[num_out + num_in++] = &inhdr_sg; >> >> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC); >> @@ -189,7 +197,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe >> !virtqueue_is_broken(vcrypto->ctrl_vq)) >> cpu_relax(); >> >> - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) { >> + if (ctrl_status->status != VIRTIO_CRYPTO_OK) { >> err = -EINVAL; >> goto out; >> } >> @@ -201,7 +209,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe >> spin_unlock(&vcrypto->ctrl_lock); >> if (err < 0) { >> pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n", >> - vcrypto->ctrl_status.status, destroy_session->session_id); >> + ctrl_status->status, destroy_session->session_id); >> } >> >> return err; >> diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c >> index a618c46a52b8..e3c5bc8d6112 100644 >> --- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c >> +++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c >> @@ -123,6 +123,9 @@ static int virtio_crypto_alg_skcipher_init_session( >> int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT; >> int err; >> unsigned int num_out = 0, num_in = 0; >> + struct virtio_crypto_op_ctrl_req *ctrl; >> + struct virtio_crypto_session_input *input; >> + struct virtio_crypto_sym_create_session_req *sym_create_session; >> >> /* >> * Avoid to do DMA from the stack, switch to using >> @@ -135,24 +138,22 @@ static int virtio_crypto_alg_skcipher_init_session( >> >> spin_lock(&vcrypto->ctrl_lock); >> /* Pad ctrl header */ >> - vcrypto->ctrl.header.opcode = >> - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION); >> - vcrypto->ctrl.header.algo = cpu_to_le32(alg); >> + ctrl = &vcrypto->ctrl; >> + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION); >> + ctrl->header.algo = cpu_to_le32(alg); >> /* Set the default dataqueue id to 0 */ >> - vcrypto->ctrl.header.queue_id = 0; >> + ctrl->header.queue_id = 0; >> >> - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR); >> + input = &vcrypto->input; >> + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR); >> /* Pad cipher's parameters */ >> - vcrypto->ctrl.u.sym_create_session.op_type = >> - cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); >> - vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo = >> - vcrypto->ctrl.header.algo; >> - vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen = >> - cpu_to_le32(keylen); >> - vcrypto->ctrl.u.sym_create_session.u.cipher.para.op = >> - cpu_to_le32(op); >> - >> - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); >> + sym_create_session = &ctrl->u.sym_create_session; >> + sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); >> + sym_create_session->u.cipher.para.algo = ctrl->header.algo; >> + sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen); >> + sym_create_session->u.cipher.para.op = cpu_to_le32(op); >> + >> + sg_init_one(&outhdr, ctrl, sizeof(*ctrl)); >> sgs[num_out++] = &outhdr; >> >> /* Set key */ >> @@ -160,7 +161,7 @@ static int virtio_crypto_alg_skcipher_init_session( >> sgs[num_out++] = &key_sg; >> >> /* Return status and session id back */ >> - sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input)); >> + sg_init_one(&inhdr, input, sizeof(*input)); >> sgs[num_out + num_in++] = &inhdr; >> >> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, >> @@ -180,20 +181,18 @@ static int virtio_crypto_alg_skcipher_init_session( >> !virtqueue_is_broken(vcrypto->ctrl_vq)) >> cpu_relax(); >> >> - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) { >> + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) { >> spin_unlock(&vcrypto->ctrl_lock); >> pr_err("virtio_crypto: Create session failed status: %u\n", >> - le32_to_cpu(vcrypto->input.status)); >> + le32_to_cpu(input->status)); >> kfree_sensitive(cipher_key); >> return -EINVAL; >> } >> >> if (encrypt) >> - ctx->enc_sess_info.session_id = >> - le64_to_cpu(vcrypto->input.session_id); >> + ctx->enc_sess_info.session_id = le64_to_cpu(input->session_id); >> else >> - ctx->dec_sess_info.session_id = >> - le64_to_cpu(vcrypto->input.session_id); >> + ctx->dec_sess_info.session_id = le64_to_cpu(input->session_id); >> >> spin_unlock(&vcrypto->ctrl_lock); >> >> @@ -211,30 +210,30 @@ static int virtio_crypto_alg_skcipher_close_session( >> struct virtio_crypto *vcrypto = ctx->vcrypto; >> int err; >> unsigned int num_out = 0, num_in = 0; >> + struct virtio_crypto_op_ctrl_req *ctrl; >> + struct virtio_crypto_inhdr *ctrl_status; >> >> spin_lock(&vcrypto->ctrl_lock); >> - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR; >> + ctrl_status = &vcrypto->ctrl_status; >> + ctrl_status->status = VIRTIO_CRYPTO_ERR; >> /* Pad ctrl header */ >> - vcrypto->ctrl.header.opcode = >> - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION); >> + ctrl = &vcrypto->ctrl; >> + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION); >> /* Set the default virtqueue id to 0 */ >> - vcrypto->ctrl.header.queue_id = 0; >> + ctrl->header.queue_id = 0; >> >> - destroy_session = &vcrypto->ctrl.u.destroy_session; >> + destroy_session = &ctrl->u.destroy_session; >> >> if (encrypt) >> - destroy_session->session_id = >> - cpu_to_le64(ctx->enc_sess_info.session_id); >> + destroy_session->session_id = cpu_to_le64(ctx->enc_sess_info.session_id); >> else >> - destroy_session->session_id = >> - cpu_to_le64(ctx->dec_sess_info.session_id); >> + destroy_session->session_id = cpu_to_le64(ctx->dec_sess_info.session_id); >> >> - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); >> + sg_init_one(&outhdr, ctrl, sizeof(*ctrl)); >> sgs[num_out++] = &outhdr; >> >> /* Return status and session id back */ >> - sg_init_one(&status_sg, &vcrypto->ctrl_status.status, >> - sizeof(vcrypto->ctrl_status.status)); >> + sg_init_one(&status_sg, &ctrl_status->status, sizeof(ctrl_status->status)); >> sgs[num_out + num_in++] = &status_sg; >> >> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, >> @@ -249,11 +248,10 @@ static int virtio_crypto_alg_skcipher_close_session( >> !virtqueue_is_broken(vcrypto->ctrl_vq)) >> cpu_relax(); >> >> - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) { >> + if (ctrl_status->status != VIRTIO_CRYPTO_OK) { >> spin_unlock(&vcrypto->ctrl_lock); >> pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n", >> - vcrypto->ctrl_status.status, >> - destroy_session->session_id); >> + ctrl_status->status, destroy_session->session_id); >> >> return -EINVAL; >> } >> -- >> 2.20.1 >> >
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c index f3ec9420215e..20901a263fc8 100644 --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c @@ -106,23 +106,27 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher unsigned int inlen; int err; unsigned int num_out = 0, num_in = 0; + struct virtio_crypto_op_ctrl_req *ctrl; + struct virtio_crypto_session_input *input; pkey = kmemdup(key, keylen, GFP_ATOMIC); if (!pkey) return -ENOMEM; spin_lock(&vcrypto->ctrl_lock); - memcpy(&vcrypto->ctrl.header, header, sizeof(vcrypto->ctrl.header)); - memcpy(&vcrypto->ctrl.u, para, sizeof(vcrypto->ctrl.u)); - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR); + ctrl = &vcrypto->ctrl; + memcpy(&ctrl->header, header, sizeof(ctrl->header)); + memcpy(&ctrl->u, para, sizeof(ctrl->u)); + input = &vcrypto->input; + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR); - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl)); sgs[num_out++] = &outhdr_sg; sg_init_one(&key_sg, pkey, keylen); sgs[num_out++] = &key_sg; - sg_init_one(&inhdr_sg, &vcrypto->input, sizeof(vcrypto->input)); + sg_init_one(&inhdr_sg, input, sizeof(*input)); sgs[num_out + num_in++] = &inhdr_sg; err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC); @@ -134,12 +138,12 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher !virtqueue_is_broken(vcrypto->ctrl_vq)) cpu_relax(); - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) { + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) { err = -EINVAL; goto out; } - ctx->session_id = le64_to_cpu(vcrypto->input.session_id); + ctx->session_id = le64_to_cpu(input->session_id); ctx->session_valid = true; err = 0; @@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher if (err < 0) pr_err("virtio_crypto: Create session failed status: %u\n", - le32_to_cpu(vcrypto->input.status)); + le32_to_cpu(input->status)); return err; } @@ -161,23 +165,27 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe struct virtio_crypto *vcrypto = ctx->vcrypto; unsigned int num_out = 0, num_in = 0, inlen; int err; + struct virtio_crypto_op_ctrl_req *ctrl; + struct virtio_crypto_inhdr *ctrl_status; spin_lock(&vcrypto->ctrl_lock); if (!ctx->session_valid) { err = 0; goto out; } - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR; - vcrypto->ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION); - vcrypto->ctrl.header.queue_id = 0; + ctrl_status = &vcrypto->ctrl_status; + ctrl_status->status = VIRTIO_CRYPTO_ERR; + ctrl = &vcrypto->ctrl; + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION); + ctrl->header.queue_id = 0; - destroy_session = &vcrypto->ctrl.u.destroy_session; + destroy_session = &ctrl->u.destroy_session; destroy_session->session_id = cpu_to_le64(ctx->session_id); - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl)); sgs[num_out++] = &outhdr_sg; - sg_init_one(&inhdr_sg, &vcrypto->ctrl_status.status, sizeof(vcrypto->ctrl_status.status)); + sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status)); sgs[num_out + num_in++] = &inhdr_sg; err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC); @@ -189,7 +197,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe !virtqueue_is_broken(vcrypto->ctrl_vq)) cpu_relax(); - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) { + if (ctrl_status->status != VIRTIO_CRYPTO_OK) { err = -EINVAL; goto out; } @@ -201,7 +209,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe spin_unlock(&vcrypto->ctrl_lock); if (err < 0) { pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n", - vcrypto->ctrl_status.status, destroy_session->session_id); + ctrl_status->status, destroy_session->session_id); } return err; diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c index a618c46a52b8..e3c5bc8d6112 100644 --- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c @@ -123,6 +123,9 @@ static int virtio_crypto_alg_skcipher_init_session( int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT; int err; unsigned int num_out = 0, num_in = 0; + struct virtio_crypto_op_ctrl_req *ctrl; + struct virtio_crypto_session_input *input; + struct virtio_crypto_sym_create_session_req *sym_create_session; /* * Avoid to do DMA from the stack, switch to using @@ -135,24 +138,22 @@ static int virtio_crypto_alg_skcipher_init_session( spin_lock(&vcrypto->ctrl_lock); /* Pad ctrl header */ - vcrypto->ctrl.header.opcode = - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION); - vcrypto->ctrl.header.algo = cpu_to_le32(alg); + ctrl = &vcrypto->ctrl; + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION); + ctrl->header.algo = cpu_to_le32(alg); /* Set the default dataqueue id to 0 */ - vcrypto->ctrl.header.queue_id = 0; + ctrl->header.queue_id = 0; - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR); + input = &vcrypto->input; + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR); /* Pad cipher's parameters */ - vcrypto->ctrl.u.sym_create_session.op_type = - cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); - vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo = - vcrypto->ctrl.header.algo; - vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen = - cpu_to_le32(keylen); - vcrypto->ctrl.u.sym_create_session.u.cipher.para.op = - cpu_to_le32(op); - - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); + sym_create_session = &ctrl->u.sym_create_session; + sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); + sym_create_session->u.cipher.para.algo = ctrl->header.algo; + sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen); + sym_create_session->u.cipher.para.op = cpu_to_le32(op); + + sg_init_one(&outhdr, ctrl, sizeof(*ctrl)); sgs[num_out++] = &outhdr; /* Set key */ @@ -160,7 +161,7 @@ static int virtio_crypto_alg_skcipher_init_session( sgs[num_out++] = &key_sg; /* Return status and session id back */ - sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input)); + sg_init_one(&inhdr, input, sizeof(*input)); sgs[num_out + num_in++] = &inhdr; err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, @@ -180,20 +181,18 @@ static int virtio_crypto_alg_skcipher_init_session( !virtqueue_is_broken(vcrypto->ctrl_vq)) cpu_relax(); - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) { + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) { spin_unlock(&vcrypto->ctrl_lock); pr_err("virtio_crypto: Create session failed status: %u\n", - le32_to_cpu(vcrypto->input.status)); + le32_to_cpu(input->status)); kfree_sensitive(cipher_key); return -EINVAL; } if (encrypt) - ctx->enc_sess_info.session_id = - le64_to_cpu(vcrypto->input.session_id); + ctx->enc_sess_info.session_id = le64_to_cpu(input->session_id); else - ctx->dec_sess_info.session_id = - le64_to_cpu(vcrypto->input.session_id); + ctx->dec_sess_info.session_id = le64_to_cpu(input->session_id); spin_unlock(&vcrypto->ctrl_lock); @@ -211,30 +210,30 @@ static int virtio_crypto_alg_skcipher_close_session( struct virtio_crypto *vcrypto = ctx->vcrypto; int err; unsigned int num_out = 0, num_in = 0; + struct virtio_crypto_op_ctrl_req *ctrl; + struct virtio_crypto_inhdr *ctrl_status; spin_lock(&vcrypto->ctrl_lock); - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR; + ctrl_status = &vcrypto->ctrl_status; + ctrl_status->status = VIRTIO_CRYPTO_ERR; /* Pad ctrl header */ - vcrypto->ctrl.header.opcode = - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION); + ctrl = &vcrypto->ctrl; + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION); /* Set the default virtqueue id to 0 */ - vcrypto->ctrl.header.queue_id = 0; + ctrl->header.queue_id = 0; - destroy_session = &vcrypto->ctrl.u.destroy_session; + destroy_session = &ctrl->u.destroy_session; if (encrypt) - destroy_session->session_id = - cpu_to_le64(ctx->enc_sess_info.session_id); + destroy_session->session_id = cpu_to_le64(ctx->enc_sess_info.session_id); else - destroy_session->session_id = - cpu_to_le64(ctx->dec_sess_info.session_id); + destroy_session->session_id = cpu_to_le64(ctx->dec_sess_info.session_id); - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl)); + sg_init_one(&outhdr, ctrl, sizeof(*ctrl)); sgs[num_out++] = &outhdr; /* Return status and session id back */ - sg_init_one(&status_sg, &vcrypto->ctrl_status.status, - sizeof(vcrypto->ctrl_status.status)); + sg_init_one(&status_sg, &ctrl_status->status, sizeof(ctrl_status->status)); sgs[num_out + num_in++] = &status_sg; err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, @@ -249,11 +248,10 @@ static int virtio_crypto_alg_skcipher_close_session( !virtqueue_is_broken(vcrypto->ctrl_vq)) cpu_relax(); - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) { + if (ctrl_status->status != VIRTIO_CRYPTO_OK) { spin_unlock(&vcrypto->ctrl_lock); pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n", - vcrypto->ctrl_status.status, - destroy_session->session_id); + ctrl_status->status, destroy_session->session_id); return -EINVAL; }
Use temporary variable to make code easy to read and maintain. /* Pad cipher's parameters */ vcrypto->ctrl.u.sym_create_session.op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo = vcrypto->ctrl.header.algo; vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen = cpu_to_le32(keylen); vcrypto->ctrl.u.sym_create_session.u.cipher.para.op = cpu_to_le32(op); --> sym_create_session = &ctrl->u.sym_create_session; sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER); sym_create_session->u.cipher.para.algo = ctrl->header.algo; sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen); sym_create_session->u.cipher.para.op = cpu_to_le32(op); The new style shows more obviously: - the variable we want to operate. - an assignment statement in a single line. Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Gonglei <arei.gonglei@huawei.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> --- .../virtio/virtio_crypto_akcipher_algs.c | 40 ++++++----- .../virtio/virtio_crypto_skcipher_algs.c | 72 +++++++++---------- 2 files changed, 59 insertions(+), 53 deletions(-)