On Mon, Nov 20, 2023 at 11:49:45AM +0000, Gonglei (Arei) wrote:
> Doing ipsec produces a spinlock recursion warning.
> This is due to crypto_finalize_request() being called in the upper half.
> Move virtual data queue processing of virtio-crypto driver to tasklet.
> 
> Fixes: dbaf0624ffa57 ("crypto: add virtio-crypto driver")
> Reported-by: Halil Pasic <pa...@linux.ibm.com>
> Signed-off-by: wangyangxin <wangyangx...@huawei.com>
> Signed-off-by: Gonglei <arei.gong...@huawei.com>
> ---
>  drivers/crypto/virtio/virtio_crypto_common.h |  2 ++
>  drivers/crypto/virtio/virtio_crypto_core.c   | 23 +++++++++++++----------
>  2 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h 
> b/drivers/crypto/virtio/virtio_crypto_common.h
> index 59a4c02..5c17c6e 100644
> --- a/drivers/crypto/virtio/virtio_crypto_common.h
> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> @@ -10,6 +10,7 @@
>  #include <linux/virtio.h>
>  #include <linux/crypto.h>
>  #include <linux/spinlock.h>
> +#include <linux/interrupt.h>
>  #include <crypto/aead.h>
>  #include <crypto/aes.h>
>  #include <crypto/engine.h>
> @@ -28,6 +29,7 @@ struct data_queue {
>       char name[32];
>  
>       struct crypto_engine *engine;
> +     struct tasklet_struct done_task;
>  };
>  
>  struct virtio_crypto {
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c 
> b/drivers/crypto/virtio/virtio_crypto_core.c
> index 1198bd3..e747f4f 100644
> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -72,27 +72,28 @@ int virtio_crypto_ctrl_vq_request(struct virtio_crypto 
> *vcrypto, struct scatterl
>       return 0;
>  }
>  
> -static void virtcrypto_dataq_callback(struct virtqueue *vq)
> +static void virtcrypto_done_task(unsigned long data)
>  {
> -     struct virtio_crypto *vcrypto = vq->vdev->priv;
> +     struct data_queue *data_vq = (struct data_queue *)data;
> +     struct virtqueue *vq = data_vq->vq;
>       struct virtio_crypto_request *vc_req;
> -     unsigned long flags;
>       unsigned int len;
> -     unsigned int qid = vq->index;
>  
> -     spin_lock_irqsave(&vcrypto->data_vq[qid].lock, flags);
>       do {
>               virtqueue_disable_cb(vq);
>               while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
> -                     spin_unlock_irqrestore(
> -                             &vcrypto->data_vq[qid].lock, flags);
>                       if (vc_req->alg_cb)
>                               vc_req->alg_cb(vc_req, len);
> -                     spin_lock_irqsave(
> -                             &vcrypto->data_vq[qid].lock, flags);
>               }
>       } while (!virtqueue_enable_cb(vq));
> -     spin_unlock_irqrestore(&vcrypto->data_vq[qid].lock, flags);
> +}
> +
> +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> +{
> +     struct virtio_crypto *vcrypto = vq->vdev->priv;
> +     struct data_queue *dq = &vcrypto->data_vq[vq->index];
> +
> +     tasklet_schedule(&dq->done_task);
>  }
>

Don't we then need to wait for tasklet to complete on
device remove?

  
>  static int virtcrypto_find_vqs(struct virtio_crypto *vi)
> @@ -150,6 +151,8 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
>                       ret = -ENOMEM;
>                       goto err_engine;
>               }
> +             tasklet_init(&vi->data_vq[i].done_task, virtcrypto_done_task,
> +                             (unsigned long)&vi->data_vq[i]);
>       }
>  
>       kfree(names);
> -- 
> 1.8.3.1
> 
> 


Reply via email to