[bug report] RDMA/bnxt_re: Fix broken RoCE driver due to recent L2 driver changes

[bug report] RDMA/bnxt_re: Fix broken RoCE driver due to recent L2 driver changes

[Dan Carpenter]

Hello Devesh Sharma,

The patch 6e04b1035689: "RDMA/bnxt_re: Fix broken RoCE driver due to
recent L2 driver changes" from May 25, 2018, leads to the following
static checker warning:

	drivers/infiniband/hw/bnxt_re/qplib_fp.c:471 bnxt_qplib_nq_start_irq()
	warn: 'nq->msix_vec' not released on lines: 471.

drivers/infiniband/hw/bnxt_re/qplib_fp.c
   441  int bnxt_qplib_nq_start_irq(struct bnxt_qplib_nq *nq, int nq_indx,
   442                              int msix_vector, bool need_init)
   443  {
   444          int rc;
   445  
   446          if (nq->requested)
   447                  return -EFAULT;
   448  
   449          nq->msix_vec = msix_vector;
   450          if (need_init)
   451                  tasklet_setup(&nq->nq_tasklet, bnxt_qplib_service_nq);
   452          else
   453                  tasklet_enable(&nq->nq_tasklet);
   454  
   455          snprintf(nq->name, sizeof(nq->name), "bnxt_qplib_nq-%d", nq_indx);
   456          rc = request_irq(nq->msix_vec, bnxt_qplib_nq_irq, 0, nq->name, nq);
   457          if (rc)
   458                  return rc;
   459  
   460          cpumask_clear(&nq->mask);
   461          cpumask_set_cpu(nq_indx, &nq->mask);
   462          rc = irq_set_affinity_hint(nq->msix_vec, &nq->mask);
   463          if (rc) {
   464                  dev_warn(&nq->pdev->dev,
   465                           "set affinity failed; vector: %d nq_idx: %d\n",
   466                           nq->msix_vec, nq_indx);

Say irq_set_affinity_hint() fails then should we call release_irq()?

   467          }
   468          nq->requested = true;
   469          bnxt_qplib_ring_nq_db(&nq->nq_db.dbinfo, nq->res->cctx, true);
   470  
   471          return rc;

I think that maybe the intent was to "return 0;" here even if setting
the hint failed.

   472  }

regards,
dan carpenter

Re: [bug report] RDMA/bnxt_re: Fix broken RoCE driver due to recent L2 driver changes

[Devesh Sharma]

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

On Fri, Nov 13, 2020 at 1:45 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Devesh Sharma,
>
> The patch 6e04b1035689: "RDMA/bnxt_re: Fix broken RoCE driver due to
> recent L2 driver changes" from May 25, 2018, leads to the following
> static checker warning:
>
>         drivers/infiniband/hw/bnxt_re/qplib_fp.c:471 bnxt_qplib_nq_start_irq()
>         warn: 'nq->msix_vec' not released on lines: 471.
>
> drivers/infiniband/hw/bnxt_re/qplib_fp.c
>    441  int bnxt_qplib_nq_start_irq(struct bnxt_qplib_nq *nq, int nq_indx,
>    442                              int msix_vector, bool need_init)
>    443  {
>    444          int rc;
>    445
>    446          if (nq->requested)
>    447                  return -EFAULT;
>    448
>    449          nq->msix_vec = msix_vector;
>    450          if (need_init)
>    451                  tasklet_setup(&nq->nq_tasklet, bnxt_qplib_service_nq);
>    452          else
>    453                  tasklet_enable(&nq->nq_tasklet);
>    454
>    455          snprintf(nq->name, sizeof(nq->name), "bnxt_qplib_nq-%d", nq_indx);
>    456          rc = request_irq(nq->msix_vec, bnxt_qplib_nq_irq, 0, nq->name, nq);
>    457          if (rc)
>    458                  return rc;
>    459
>    460          cpumask_clear(&nq->mask);
>    461          cpumask_set_cpu(nq_indx, &nq->mask);
>    462          rc = irq_set_affinity_hint(nq->msix_vec, &nq->mask);
>    463          if (rc) {
>    464                  dev_warn(&nq->pdev->dev,
>    465                           "set affinity failed; vector: %d nq_idx: %d\n",
>    466                           nq->msix_vec, nq_indx);
>
> Say irq_set_affinity_hint() fails then should we call release_irq()?
No, the driver wants to treat it as a soft error and just flash a debug message.
>
>    467          }
>    468          nq->requested = true;
>    469          bnxt_qplib_ring_nq_db(&nq->nq_db.dbinfo, nq->res->cctx, true);
>    470
>    471          return rc;
>
> I think that maybe the intent was to "return 0;" here even if setting
> the hint failed.
Yes, you are right, this should had been return 0 instead.
>
>    472  }
>
> regards,
> dan carpenter

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4169 bytes --]