Re: [bug report] RDMA/bnxt_re: Add SRQ support for Broadcom adapters

On Wed, Jan 31, 2018 at 11:16:55AM +0530, Devesh Sharma wrote:

On Tue, Jan 30, 2018 at 6:15 PM, Dan Carpenter [off-list ref] wrote:

quoted

Hello Devesh Sharma,

The patch 37cb11acf1f7: "RDMA/bnxt_re: Add SRQ support for Broadcom
adapters" from Jan 11, 2018, leads to the following static checker
warning:

        drivers/infiniband/hw/bnxt_re/ib_verbs.c:1317 bnxt_re_destroy_srq()
        warn: 'srq->umem' isn't an ERR_PTR

drivers/infiniband/hw/bnxt_re/ib_verbs.c
  1313                  dev_err(rdev_to_dev(rdev), "Destroy HW SRQ failed!");
  1314                  return rc;
  1315          }
  1316
  1317          if (srq->umem && !IS_ERR(srq->umem))
                                  ^^^^^^^^^^^^^^^^
We never store error pointers to srq->umem.  It's pretty consistently
checked for error pointers though so maybe that's fine.  It causes a
static checker warning because error pointer confusion is a pretty
common source of bugs.  Anyway, feel free to ignore if you want...

Thanks for reporting Dan,

Is there a way out, I want to call ib_umem_release only if it was valid.
I think if ib_umem_release checks for the validity of pointer then I
can get rid of this?
There are other places also in bnxt_re driver where such checks are present.

Yeah.  Those places generate warnings as well, but I thought one was
enough.  It's fine if you want to ignore the warning, no one will be
upset.  :P

[ snip ]

quoted

  1342          bytes = (qplib_srq->max_wqe * BNXT_QPLIB_MAX_RQE_ENTRY_SIZE);
  1343          bytes = PAGE_ALIGN(bytes);
  1344          umem = ib_umem_get(context, ureq.srqva, bytes,
  1345                             IB_ACCESS_LOCAL_WRITE, 1);
  1346          if (IS_ERR(umem))
  1347                  return PTR_ERR(umem);
  1348
  1349          srq->umem = umem;
                ^^^^^^^^^^^^^^^^
Set here, I guess.

Yeah, the checker is confused due to this.

It does bother me that you're saying the "checker is confused".  The
checker is printing a 100% accurate, factual warning...  :/  We have an
IS_ERR() check when the pointer can not possibly be an error pointer.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html