On Mon, 2 Mar 2020 14:15:13 +0800
Zaibo Xu <xuzaibo@xxxxxxxxxx> wrote:
> From: yekai13 <yekai13@xxxxxxxxxx>
>
> Allocate one workqueue for each QM instead of one for all QMs,
> we found the throughput of SEC engine can be increased to
> the hardware limit throughput during testing sec2 performance.
> so we added this scheme.
>
> Signed-off-by: yekai13 <yekai13@xxxxxxxxxx>
> Signed-off-by: liulongfang <liulongfang@xxxxxxxxxx>
That first sign off needs fixing. Needs to be a real name.
Also missing xuzaibo's sign offf.
A question inline that might be worth a follow up patch.
With signoffs fixed
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Thanks,
Jonathan
> ---
> drivers/crypto/hisilicon/sec2/sec_main.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/hisilicon/sec2/sec_main.c b/drivers/crypto/hisilicon/sec2/sec_main.c
> index 3767fdb..ebafc1c 100644
> --- a/drivers/crypto/hisilicon/sec2/sec_main.c
> +++ b/drivers/crypto/hisilicon/sec2/sec_main.c
> @@ -774,12 +774,24 @@ static void sec_qm_uninit(struct hisi_qm *qm)
>
> static int sec_probe_init(struct hisi_qm *qm, struct sec_dev *sec)
> {
> + int ret;
> +
> + qm->wq = alloc_workqueue("%s", WQ_HIGHPRI | WQ_CPU_INTENSIVE |
> + WQ_MEM_RECLAIM | WQ_UNBOUND, num_online_cpus(),
> + pci_name(qm->pdev));
I appreciate that you have the same parameters here as were originally in
qm.c, but I would like to fully understand why some of these flags are set.
Perhaps a comment for each of them? I'm not sure I'd consider the work
to be done in this work queue CPU_INTENSIVE for example.
This could be a follow up patch though as not actually related to this
change.
> + if (!qm->wq) {
> + pci_err(qm->pdev, "fail to alloc workqueue\n");
> + return -ENOMEM;
> + }
> +
> if (qm->fun_type == QM_HW_PF) {
> qm->qp_base = SEC_PF_DEF_Q_BASE;
> qm->qp_num = pf_q_num;
> qm->debug.curr_qm_qp_num = pf_q_num;
>
> - return sec_pf_probe_init(sec);
> + ret = sec_pf_probe_init(sec);
> + if (ret)
> + goto err_probe_uninit;
> } else if (qm->fun_type == QM_HW_VF) {
> /*
> * have no way to get qm configure in VM in v1 hardware,
> @@ -792,18 +804,26 @@ static int sec_probe_init(struct hisi_qm *qm, struct sec_dev *sec)
> qm->qp_num = SEC_QUEUE_NUM_V1 - SEC_PF_DEF_Q_NUM;
> } else if (qm->ver == QM_HW_V2) {
> /* v2 starts to support get vft by mailbox */
> - return hisi_qm_get_vft(qm, &qm->qp_base, &qm->qp_num);
> + ret = hisi_qm_get_vft(qm, &qm->qp_base, &qm->qp_num);
> + if (ret)
> + goto err_probe_uninit;
> }
> } else {
> - return -ENODEV;
> + ret = -ENODEV;
> + goto err_probe_uninit;
> }
>
> return 0;
> +err_probe_uninit:
> + destroy_workqueue(qm->wq);
> + return ret;
> }
>
> static void sec_probe_uninit(struct hisi_qm *qm)
> {
> hisi_qm_dev_err_uninit(qm);
> +
> + destroy_workqueue(qm->wq);
> }
>
> static int sec_probe(struct pci_dev *pdev, const struct pci_device_id *id)
