Skip to contentSkip to conversation

RDMA/restrack: Track driver QP types in resource tracker

PATCH Superseded Gal Pressman 3 participants 4 messages Jul 30, 2019 – Jul 31, 2019 InfiniBand

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 RDMA/restrack: Track driver QP types in resource tracker

The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
As a result, "rdma resource show" user command would not show opened driver QPs which does not reflect the real state of the system.

Check QP type explicitly instead of improperly assuming enum values/ordering.

Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")

---
v2:
* Improve commit message

---
 drivers/infiniband/core/core_priv.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/infiniband/core/core_priv.h +4 -1
index 589ed805e0ad..3a8b0911c3bc 100644
@@ -321,7 +321,9 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
 					  struct ib_udata *udata,
 					  struct ib_uobject *uobj)
 {
+	enum ib_qp_type qp_type = attr->qp_type;
 	struct ib_qp *qp;
+	bool is_xrc;
 
 	if (!dev->ops.create_qp)
 		return ERR_PTR(-EOPNOTSUPP);
@@ -339,7 +341,8 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
 	 * and more importantly they are created internaly by driver,
 	 * see mlx5 create_dev_resources() as an example.
 	 */
-	if (attr->qp_type < IB_QPT_XRC_INI) {
+	is_xrc = qp_type == IB_QPT_XRC_INI || qp_type == IB_QPT_XRC_TGT;
+	if ((qp_type < IB_QPT_MAX && !is_xrc) || qp_type == IB_QPT_DRIVER) {
 		qp->res.type = RDMA_RESTRACK_QP;
 		if (uobj)
 			rdma_restrack_uadd(&qp->res);
Signature · 1 line
2.22.0
#1 lore raw
Quoted 5 lines · Gal Pressman

The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
As a result, "rdma resource show" user command would not show opened driver QPs which does not reflect the real state of the system.

Quoted 8 lines

Check QP type explicitly instead of improperly assuming enum values/ordering.

Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")

---
v2:
* Improve commit message

Please finish review of v0 and give enough time for reviewers to see patch and post their notes before resending.

Thanks

#2 lore raw
Quoted 17 lines · Leon Romanovsky
Quoted 9 lines · Gal Pressman

The check for QP type different than XRC has wrongly excluded driver QP
types from the resource tracker.
As a result, "rdma resource show" user command would not show opened driver QPs which does not reflect the real state of the system.

Check QP type explicitly instead of improperly assuming enum values/ordering.

Quoted 6 lines
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create

and destroy QPs")

---
v2:
* Improve commit message

Quoted 2 lines

Please finish review of v0 and give enough time for reviewers to see patch and post their notes before resending.

Gal, Leon was right in his comments to the v1 of this patch in terms of the original code not being broken prior to the existence of driver qp types. This fix isn't needed until after the EFA driver is merged, and the Fixes: tag is used in order for scripts to know if they need to take a patch because they've already taken the patch prior. So the Fixes tag needs to be the EFA driver, not the original resource tracking commit, as there is no issue unless the EFA driver is placed on top of the original resource tracking commit. Please resubmit with a proper commit message and fixes tag.

Signature · 3 lines
Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
#3 lore raw
Quoted 18 lines · Doug Ledford
Quoted 17 lines · Leon Romanovsky
Quoted 15 lines · Gal Pressman

The check for QP type different than XRC has wrongly excluded driver QP
types from the resource tracker.
As a result, "rdma resource show" user command would not show opened driver QPs which does not reflect the real state of the system.

Check QP type explicitly instead of improperly assuming enum values/ordering.

Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create

and destroy QPs")

---
v2:
* Improve commit message

Quoted 6 lines
Quoted 2 lines

Please finish review of v0 and give enough time for reviewers to see patch and post their notes before resending.

Gal, Leon was right in his comments to the v1 of this patch in terms of the original code not being broken prior to the existence of driver qp types.

Driver QP types existed before EFA was merged, and they existed when the restrack commit was merged. So if driver QP types should be counted the restrack commit is the one that "broke" it, not EFA.
If driver QP types were introduced in commit X, where X comes after the restrack commit then it makes sense to target the Fixes line to commit X, but this is not the case here.

Anyway, I'll change it to EFA as requested.

Quoted 7 lines

This fix isn't needed until after the EFA driver is merged, and the Fixes: tag is used in order for scripts to know if they need to take a patch because they've already taken the patch prior. So the Fixes tag needs to be the EFA driver, not the original resource tracking commit, as there is no issue unless the EFA driver is placed on top of the original resource tracking commit. Please resubmit with a proper commit message and fixes tag.

Thanks, As Leon mentioned I posted v2 before v1 discussion was finished. I'll resubmit.