On Wed, Jan 31, 2018 at 11:16:55AM +0530, Devesh Sharma wrote: > On Tue, Jan 30, 2018 at 6:15 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > 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 ] > > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
