diff mbox series

[1/1] crypto: hisilicon - Use one workqueue per qm instead of per qp

Message ID 20210902065845.359382-1-ike.pan@canonical.com
State New
Headers show
Series Use one workqueue per qm | expand

Commit Message

Ike Panhc Sept. 2, 2021, 6:58 a.m. UTC
From: Shukun Tan <tanshukun1@huawei.com>

BugLink: https://launchpad.net/bugs/1932117

Since SEC need not so many workqueues as our test, we just use
one workqueue created by the device driver of QM if necessary,
which will also reduce CPU waste without any throughput decreasing.

Signed-off-by: Shukun Tan <tanshukun1@huawei.com>
Signed-off-by: Zaibo Xu <xuzaibo@huawei.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
(backported from commit 57ca81245f4db4a0222d545f8f5d4709544c26cf)
Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/crypto/hisilicon/qm.c | 39 ++++++++++++++---------------------
 drivers/crypto/hisilicon/qm.h |  4 ++--
 2 files changed, 18 insertions(+), 25 deletions(-)

Comments

Thadeu Lima de Souza Cascardo Sept. 2, 2021, 11:07 a.m. UTC | #1
On Thu, Sep 02, 2021 at 02:58:45PM +0800, Ike Panhc wrote:
> From: Shukun Tan <tanshukun1@huawei.com>
> 
> BugLink: https://launchpad.net/bugs/1932117
> 
> Since SEC need not so many workqueues as our test, we just use
> one workqueue created by the device driver of QM if necessary,
> which will also reduce CPU waste without any throughput decreasing.
> 
> Signed-off-by: Shukun Tan <tanshukun1@huawei.com>
> Signed-off-by: Zaibo Xu <xuzaibo@huawei.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> (backported from commit 57ca81245f4db4a0222d545f8f5d4709544c26cf)

By a quick skim, I could not find the differences from a cherry-pick and a
backport. So I did a cherry-pick myself, and noticed the conflicts on the
header, which are minimal, and your backport matches what I would expect from
the original commit. So all fine in that front.

However, I could not find from the patch itself where the new workqueue was
being allocated. So, I looked it up, and there is a followup commit
(immediately after the one you picked, so they must have been sent as a
patchset):

a13c97118749 crypto: hisilicon/sec2 - Add workqueue for SEC driver.

I think we should probably pick that one too. The conflict is small there too
when picking it up for focal. Can you build and test it with that one? By the
way, is sec2 used in this system?

Thanks.
Cascardo.

> Signed-off-by: Ike Panhc <ike.pan@canonical.com>
> ---
>  drivers/crypto/hisilicon/qm.c | 39 ++++++++++++++---------------------
>  drivers/crypto/hisilicon/qm.h |  4 ++--
>  2 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
> index b57da5ef8b5b..d245b8fb16e1 100644
> --- a/drivers/crypto/hisilicon/qm.c
> +++ b/drivers/crypto/hisilicon/qm.c
> @@ -485,17 +485,9 @@ static void qm_poll_qp(struct hisi_qp *qp, struct hisi_qm *qm)
>  	}
>  }
>  
> -static void qm_qp_work_func(struct work_struct *work)
> +static void qm_work_process(struct work_struct *work)
>  {
> -	struct hisi_qp *qp;
> -
> -	qp = container_of(work, struct hisi_qp, work);
> -	qm_poll_qp(qp, qp->qm);
> -}
> -
> -static irqreturn_t qm_irq_handler(int irq, void *data)
> -{
> -	struct hisi_qm *qm = data;
> +	struct hisi_qm *qm = container_of(work, struct hisi_qm, work);
>  	struct qm_eqe *eqe = qm->eqe + qm->status.eq_head;
>  	struct hisi_qp *qp;
>  	int eqe_num = 0;
> @@ -504,7 +496,7 @@ static irqreturn_t qm_irq_handler(int irq, void *data)
>  		eqe_num++;
>  		qp = qm_to_hisi_qp(qm, eqe);
>  		if (qp)
> -			queue_work(qp->wq, &qp->work);
> +			qm_poll_qp(qp, qm);
>  
>  		if (qm->status.eq_head == QM_Q_DEPTH - 1) {
>  			qm->status.eqc_phase = !qm->status.eqc_phase;
> @@ -522,6 +514,17 @@ static irqreturn_t qm_irq_handler(int irq, void *data)
>  	}
>  
>  	qm_db(qm, 0, QM_DOORBELL_CMD_EQ, qm->status.eq_head, 0);
> +}
> +
> +static irqreturn_t do_qm_irq(int irq, void *data)
> +{
> +	struct hisi_qm *qm = (struct hisi_qm *)data;
> +
> +	/* the workqueue created by device driver of QM */
> +	if (qm->wq)
> +		queue_work(qm->wq, &qm->work);
> +	else
> +		schedule_work(&qm->work);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -531,7 +534,7 @@ static irqreturn_t qm_irq(int irq, void *data)
>  	struct hisi_qm *qm = data;
>  
>  	if (readl(qm->io_base + QM_VF_EQ_INT_SOURCE))
> -		return qm_irq_handler(irq, data);
> +		return do_qm_irq(irq, data);
>  
>  	dev_err(&qm->pdev->dev, "invalid int source\n");
>  	qm_db(qm, 0, QM_DOORBELL_CMD_EQ, qm->status.eq_head, 0);
> @@ -1147,20 +1150,9 @@ struct hisi_qp *hisi_qm_create_qp(struct hisi_qm *qm, u8 alg_type)
>  
>  	qp->qp_id = qp_id;
>  	qp->alg_type = alg_type;
> -	INIT_WORK(&qp->work, qm_qp_work_func);
> -	qp->wq = alloc_workqueue("hisi_qm", WQ_UNBOUND | WQ_HIGHPRI |
> -				 WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM, 0);
> -	if (!qp->wq) {
> -		ret = -EFAULT;
> -		goto err_free_qp_mem;
> -	}
>  
>  	return qp;
>  
> -err_free_qp_mem:
> -	if (qm->use_dma_api)
> -		dma_free_coherent(dev, qp->qdma.size, qp->qdma.va,
> -				  qp->qdma.dma);
>  err_clear_bit:
>  	write_lock(&qm->qps_lock);
>  	qm->qp_array[qp_id] = NULL;
> @@ -1479,6 +1471,7 @@ int hisi_qm_init(struct hisi_qm *qm)
>  	qm->qp_in_used = 0;
>  	mutex_init(&qm->mailbox_lock);
>  	rwlock_init(&qm->qps_lock);
> +	INIT_WORK(&qm->work, qm_work_process);
>  
>  	dev_dbg(dev, "init qm %s with %s\n", pdev->is_physfn ? "pf" : "vf",
>  		qm->use_dma_api ? "dma api" : "iommu api");
> diff --git a/drivers/crypto/hisilicon/qm.h b/drivers/crypto/hisilicon/qm.h
> index 078b8f1f1b77..8557974dc3a4 100644
> --- a/drivers/crypto/hisilicon/qm.h
> +++ b/drivers/crypto/hisilicon/qm.h
> @@ -162,6 +162,8 @@ struct hisi_qm {
>  	u32 error_mask;
>  	u32 msi_mask;
>  
> +	struct workqueue_struct *wq;
> +	struct work_struct work;
>  	bool use_dma_api;
>  };
>  
> @@ -192,8 +194,6 @@ struct hisi_qp {
>  	struct hisi_qp_ops *hw_ops;
>  	void *qp_ctx;
>  	void (*req_cb)(struct hisi_qp *qp, void *data);
> -	struct work_struct work;
> -	struct workqueue_struct *wq;
>  
>  	struct hisi_qm *qm;
>  };
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
index b57da5ef8b5b..d245b8fb16e1 100644
--- a/drivers/crypto/hisilicon/qm.c
+++ b/drivers/crypto/hisilicon/qm.c
@@ -485,17 +485,9 @@  static void qm_poll_qp(struct hisi_qp *qp, struct hisi_qm *qm)
 	}
 }
 
-static void qm_qp_work_func(struct work_struct *work)
+static void qm_work_process(struct work_struct *work)
 {
-	struct hisi_qp *qp;
-
-	qp = container_of(work, struct hisi_qp, work);
-	qm_poll_qp(qp, qp->qm);
-}
-
-static irqreturn_t qm_irq_handler(int irq, void *data)
-{
-	struct hisi_qm *qm = data;
+	struct hisi_qm *qm = container_of(work, struct hisi_qm, work);
 	struct qm_eqe *eqe = qm->eqe + qm->status.eq_head;
 	struct hisi_qp *qp;
 	int eqe_num = 0;
@@ -504,7 +496,7 @@  static irqreturn_t qm_irq_handler(int irq, void *data)
 		eqe_num++;
 		qp = qm_to_hisi_qp(qm, eqe);
 		if (qp)
-			queue_work(qp->wq, &qp->work);
+			qm_poll_qp(qp, qm);
 
 		if (qm->status.eq_head == QM_Q_DEPTH - 1) {
 			qm->status.eqc_phase = !qm->status.eqc_phase;
@@ -522,6 +514,17 @@  static irqreturn_t qm_irq_handler(int irq, void *data)
 	}
 
 	qm_db(qm, 0, QM_DOORBELL_CMD_EQ, qm->status.eq_head, 0);
+}
+
+static irqreturn_t do_qm_irq(int irq, void *data)
+{
+	struct hisi_qm *qm = (struct hisi_qm *)data;
+
+	/* the workqueue created by device driver of QM */
+	if (qm->wq)
+		queue_work(qm->wq, &qm->work);
+	else
+		schedule_work(&qm->work);
 
 	return IRQ_HANDLED;
 }
@@ -531,7 +534,7 @@  static irqreturn_t qm_irq(int irq, void *data)
 	struct hisi_qm *qm = data;
 
 	if (readl(qm->io_base + QM_VF_EQ_INT_SOURCE))
-		return qm_irq_handler(irq, data);
+		return do_qm_irq(irq, data);
 
 	dev_err(&qm->pdev->dev, "invalid int source\n");
 	qm_db(qm, 0, QM_DOORBELL_CMD_EQ, qm->status.eq_head, 0);
@@ -1147,20 +1150,9 @@  struct hisi_qp *hisi_qm_create_qp(struct hisi_qm *qm, u8 alg_type)
 
 	qp->qp_id = qp_id;
 	qp->alg_type = alg_type;
-	INIT_WORK(&qp->work, qm_qp_work_func);
-	qp->wq = alloc_workqueue("hisi_qm", WQ_UNBOUND | WQ_HIGHPRI |
-				 WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM, 0);
-	if (!qp->wq) {
-		ret = -EFAULT;
-		goto err_free_qp_mem;
-	}
 
 	return qp;
 
-err_free_qp_mem:
-	if (qm->use_dma_api)
-		dma_free_coherent(dev, qp->qdma.size, qp->qdma.va,
-				  qp->qdma.dma);
 err_clear_bit:
 	write_lock(&qm->qps_lock);
 	qm->qp_array[qp_id] = NULL;
@@ -1479,6 +1471,7 @@  int hisi_qm_init(struct hisi_qm *qm)
 	qm->qp_in_used = 0;
 	mutex_init(&qm->mailbox_lock);
 	rwlock_init(&qm->qps_lock);
+	INIT_WORK(&qm->work, qm_work_process);
 
 	dev_dbg(dev, "init qm %s with %s\n", pdev->is_physfn ? "pf" : "vf",
 		qm->use_dma_api ? "dma api" : "iommu api");
diff --git a/drivers/crypto/hisilicon/qm.h b/drivers/crypto/hisilicon/qm.h
index 078b8f1f1b77..8557974dc3a4 100644
--- a/drivers/crypto/hisilicon/qm.h
+++ b/drivers/crypto/hisilicon/qm.h
@@ -162,6 +162,8 @@  struct hisi_qm {
 	u32 error_mask;
 	u32 msi_mask;
 
+	struct workqueue_struct *wq;
+	struct work_struct work;
 	bool use_dma_api;
 };
 
@@ -192,8 +194,6 @@  struct hisi_qp {
 	struct hisi_qp_ops *hw_ops;
 	void *qp_ctx;
 	void (*req_cb)(struct hisi_qp *qp, void *data);
-	struct work_struct work;
-	struct workqueue_struct *wq;
 
 	struct hisi_qm *qm;
 };