* [bug report] qedr: Add support for QP verbs
@ 2016-11-14 13:03 Dan Carpenter
2016-11-15 10:36 ` Amrani, Ram
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2016-11-14 13:03 UTC (permalink / raw)
To: Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Ram Amrani,
The patch cecbcddf6461: "qedr: Add support for QP verbs" from Oct 10,
2016, leads to the following static checker warning:
drivers/infiniband/hw/qedr/verbs.c:2067 qedr_destroy_qp()
'0x5 | 0x1' has '0x1' set on both sides
drivers/infiniband/hw/qedr/verbs.c
2056 int qedr_destroy_qp(struct ib_qp *ibqp)
2057 {
2058 struct qedr_qp *qp = get_qedr_qp(ibqp);
2059 struct qedr_dev *dev = qp->dev;
2060 struct ib_qp_attr attr;
2061 int attr_mask = 0;
2062 int rc = 0;
2063
2064 DP_DEBUG(dev, QEDR_MSG_QP, "destroy qp: destroying %p, qp type=%d\n",
2065 qp, qp->qp_type);
2066
2067 if (qp->state != (QED_ROCE_QP_STATE_RESET | QED_ROCE_QP_STATE_ERR |
2068 QED_ROCE_QP_STATE_INIT)) {
These aren't bitfields, they're just numbers. This code is pretty
suspect. Not sure what was intended.
2069 attr.qp_state = IB_QPS_ERR;
2070 attr_mask |= IB_QP_STATE;
2071
2072 /* Change the QP state to ERROR */
2073 qedr_modify_qp(ibqp, &attr, attr_mask, NULL);
2074 }
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [bug report] qedr: Add support for QP verbs
2016-11-14 13:03 [bug report] qedr: Add support for QP verbs Dan Carpenter
@ 2016-11-15 10:36 ` Amrani, Ram
0 siblings, 0 replies; 3+ messages in thread
From: Amrani, Ram @ 2016-11-15 10:36 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
> Hello Ram Amrani,
>
> The patch cecbcddf6461: "qedr: Add support for QP verbs" from Oct 10, 2016,
> leads to the following static checker warning:
>
> drivers/infiniband/hw/qedr/verbs.c:2067 qedr_destroy_qp()
> '0x5 | 0x1' has '0x1' set on both sides
>
> drivers/infiniband/hw/qedr/verbs.c
> 2056 int qedr_destroy_qp(struct ib_qp *ibqp)
> 2057 {
> 2058 struct qedr_qp *qp = get_qedr_qp(ibqp);
> 2059 struct qedr_dev *dev = qp->dev;
> 2060 struct ib_qp_attr attr;
> 2061 int attr_mask = 0;
> 2062 int rc = 0;
> 2063
> 2064 DP_DEBUG(dev, QEDR_MSG_QP, "destroy qp: destroying %p, qp
> type=%d\n",
> 2065 qp, qp->qp_type);
> 2066
> 2067 if (qp->state != (QED_ROCE_QP_STATE_RESET |
> QED_ROCE_QP_STATE_ERR |
> 2068 QED_ROCE_QP_STATE_INIT)) {
>
> These aren't bitfields, they're just numbers. This code is pretty suspect. Not
> sure what was intended.
>
> 2069 attr.qp_state = IB_QPS_ERR;
> 2070 attr_mask |= IB_QP_STATE;
> 2071
> 2072 /* Change the QP state to ERROR */
> 2073 qedr_modify_qp(ibqp, &attr, attr_mask, NULL);
> 2074 }
>
> regards,
> dan carpenter
That's a real bug. Thanks!
Will fix.
Ram
--
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [bug report] qedr: Add support for QP verbs
@ 2016-11-23 10:58 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2016-11-23 10:58 UTC (permalink / raw)
To: Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Ram Amrani,
The patch cecbcddf6461: "qedr: Add support for QP verbs" from Oct 10,
2016, leads to the following static checker warning:
drivers/infiniband/hw/qedr/verbs.c:1494 qedr_create_qp()
warn: possible memory leak of 'qp'
drivers/infiniband/hw/qedr/verbs.c
1484
1485 rc = qedr_check_qp_attrs(ibpd, dev, attrs);
1486 if (rc)
1487 return ERR_PTR(rc);
1488
1489 qp = kzalloc(sizeof(*qp), GFP_KERNEL);
1490 if (!qp)
1491 return ERR_PTR(-ENOMEM);
1492
1493 if (attrs->srq)
1494 return ERR_PTR(-EINVAL);
You should move this in front of the allocation to avoid the memory
leak.
1495
1496 DP_DEBUG(dev, QEDR_MSG_QP,
1497 "create qp: sq_cq=%p, sq_icid=%d, rq_cq=%p, rq_icid=%d\n",
1498 get_qedr_cq(attrs->send_cq),
1499 get_qedr_cq(attrs->send_cq)->icid,
1500 get_qedr_cq(attrs->recv_cq),
1501 get_qedr_cq(attrs->recv_cq)->icid);
1502
1503 qedr_set_qp_init_params(dev, qp, pd, attrs);
1504
1505 if (attrs->qp_type == IB_QPT_GSI) {
1506 if (udata) {
1507 DP_ERR(dev,
1508 "create qp: unexpected udata when creating GSI QP\n");
1509 goto err0;
Ugh... GW-BASIC style numbered labels... What does goto err0 do???
Imagine if instead of function names we should use numbers like:
one();
two();
five();
Use a meaningful label names like "goto free_qp;"
1510 }
1511 return qedr_create_gsi_qp(dev, attrs, qp);
We should free qp if qedr_create_gsi_qp() fails as well.
1512 }
1513
1514 memset(&in_params, 0, sizeof(in_params));
1515
1516 if (udata) {
1517 if (!(udata && ibpd->uobject && ibpd->uobject->context))
1518 goto err0;
1519
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-23 10:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14 13:03 [bug report] qedr: Add support for QP verbs Dan Carpenter
2016-11-15 10:36 ` Amrani, Ram
2016-11-23 10:58 Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.