RDMA/restrack: Track driver QP types in resource tracker
PATCH 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.
--- drivers/infiniband/core/core_priv.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/infiniband/core/core_priv.h +4 -1
@@ -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
Quoted 2 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
-rc commit messages need to describe the problem this is fixing from the users perspective.
Jason
Quoted 6 lines · Jason Gunthorpe
Quoted 2 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
-rc commit messages need to describe the problem this is fixing from the users perspective.
Thanks, will resubmit.
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")
It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
I would like to see the output or "rdma res" and "rdma res show qp" with running driver QP after your change.
Thanks
Quoted 9 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Quoted 3 lines
I would like to see the output or "rdma res" and "rdma res show qp" with running driver QP after your change.
gal@server [~] $ rdma res
0: efa_0: pd 2 cq 4 qp 2 cm_id 0 mr 2 ctx 2
gal@server [~] $ rdma res show qp
link efa_0/1 lqpn 0 type UNKNOWN state RTS sq-psn 13400680 pdn 0 pid 17847 comm ib_send_bw link efa_0/1 lqpn 1 type UNKNOWN state RTR sq-psn 0 pdn 1 pid 17820 comm ib_send_bw
We can change the UNKNOWN to DRIVER/something else in userspace code.
Quoted 7 lines · Gal Pressman
Quoted 6 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")
Quoted 7 lines
Quoted 3 lines
It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Yes, please remove "wrongly" and change Fixes line to be "Fixes: 40909f664d27 ("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.
Quoted 5 lines
Quoted 4 lines
I would like to see the output or "rdma res" and "rdma res show qp" with running driver QP after your change.
Quoted 7 lines
gal@server [~] $ rdma res
0:efa_0: pd 2 cq 4 qp 2cm_id0 mr 2 ctx 2
gal@server [~] $ rdma res show qp
link efa_0/1 lqpn 0 type UNKNOWN state RTS sq-psn 13400680 pdn 0 pid 17847 commib_send_bwlink efa_0/1 lqpn 1 type UNKNOWN state RTR sq-psn 0 pdn 1 pid 17820 commib_send_bwWe can change the UNKNOWN to DRIVER/something else in userspace code.
We need to change it too.
Thanks
Quoted 12 lines · Leon Romanovsky
Quoted 11 lines · Gal Pressman
Quoted 9 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
Quoted 7 lines
Quoted 3 lines
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Yes, please remove "wrongly" and change Fixes line to be "Fixes:
40909f664d27("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.
How are DC QPs being counted?
Quoted 17 lines · Gal Pressman
Quoted 16 lines · Leon Romanovsky
Quoted 14 lines · Gal Pressman
Quoted 9 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Quoted 5 lines
Quoted 3 lines
Yes, please remove "wrongly" and change Fixes line to be "Fixes:
40909f664d27("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.How are DC QPs being counted?
They were not counted on purpose. We didn't imagine acceptance of non-RDMA driver which doesn't support any standard QPs and doesn't work with kernel verbs.
Thanks
Quoted 22 lines · Leon Romanovsky
Quoted 21 lines · Gal Pressman
Quoted 19 lines · Leon Romanovsky
Quoted 7 lines · Gal Pressman
Quoted 6 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")Quoted 7 lines
Quoted 3 lines
It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Yes, please remove "wrongly" and change Fixes line to be "Fixes:
40909f664d27("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.
Quoted 5 lines
Quoted 1 line
How are DC QPs being counted?
They were not counted on purpose. We didn't imagine acceptance of non-RDMA driver which doesn't support any standard QPs and doesn't work with kernel verbs.
Running dcping/perftest over DC shows zero QPs? On purpose? Sounds like a bug to me..
Quoted 23 lines · Gal Pressman
Quoted 22 lines · Leon Romanovsky
Quoted 21 lines · Gal Pressman
Quoted 19 lines · Leon Romanovsky
Quoted 7 lines · Gal Pressman
Quoted 6 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")Quoted 7 lines
Quoted 3 lines
It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Yes, please remove "wrongly" and change Fixes line to be "Fixes:
40909f664d27("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
How are DC QPs being counted?
They were not counted on purpose. We didn't imagine acceptance of non-RDMA driver which doesn't support any standard QPs and doesn't work with kernel verbs.
Running dcping/perftest over DC shows zero QPs?
No, try it and you will see other QPs.
Quoted 2 lines
On purpose?
Sounds like a bug to me..
OK.
Thanks
Quoted 30 lines · Leon Romanovsky
Quoted 29 lines · Gal Pressman
Quoted 27 lines · Leon Romanovsky
Quoted 17 lines · Gal Pressman
Quoted 16 lines · Leon Romanovsky
Quoted 14 lines · Gal Pressman
Quoted 9 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Quoted 5 lines
Quoted 3 lines
Yes, please remove "wrongly" and change Fixes line to be "Fixes:
40909f664d27("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.How are DC QPs being counted?
They were not counted on purpose. We didn't imagine acceptance of non-RDMA driver which doesn't support any standard QPs and doesn't work with kernel verbs.
Quoted 8 lines
Quoted 1 line
Running dcping/perftest over DC shows zero QPs?
No, try it and you will see other QPs.
Quoted 2 lines
On purpose?
Sounds like a bug to me..OK.
Does OK mean you're OK with counting DC QPs after this patch?
Quoted 33 lines · Gal Pressman
Quoted 32 lines · Leon Romanovsky
Quoted 30 lines · Gal Pressman
Quoted 22 lines · Leon Romanovsky
Quoted 21 lines · Gal Pressman
Quoted 19 lines · Leon Romanovsky
Quoted 7 lines · Gal Pressman
Quoted 6 lines · Leon Romanovsky
Quoted 4 lines · Gal Pressman
The check for QP type different than XRC has wrongly excluded driver QP types from the resource tracker.
Fixes 78a0cd648a80 ("RDMA/core: Add resource tracking for create and destroy QPs")Quoted 7 lines
Quoted 3 lines
It is a little bit over to say "wrongly". At that time, we did it on purpose because it was unclear how to represent such QP types to users and we didn't have vendor specific hooks introduced by Steve later on.
It's very confusing to see a test running and zero QPs in "rdma res". I'm fine with removing the "wrongly" :), but I still think this should be targeted to for-rc as a bug fix.
Yes, please remove "wrongly" and change Fixes line to be "Fixes:
40909f664d27("RDMA/efa: Add EFA verbs implementation")", because before addition of EFA driver all other drivers had QPs.Quoted 5 lines
Quoted 1 line
How are DC QPs being counted?
They were not counted on purpose. We didn't imagine acceptance of non-RDMA driver which doesn't support any standard QPs and doesn't work with kernel verbs.
Running dcping/perftest over DC shows zero QPs?
Quoted 8 lines
Quoted 6 lines
No, try it and you will see other QPs.
Quoted 2 lines
On purpose?
Sounds like a bug to me..OK.
Does OK mean you're OK with counting DC QPs after this patch?
I'm OK with the idea, I'm not OK with the description.
Thanks