* [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-17 12:06 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 02/14] RDMA/mlx5: Don't call to restrack recursively Leon Romanovsky
` (12 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
Update the code to have similar destroy pattern like other IB objects.
This change create asymmetry to the rdma_id_private create flow to make
sure that memory is managed by restrack.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index f9ff8b7f05e7..24e09416de4f 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1821,7 +1821,6 @@ static void _destroy_id(struct rdma_id_private *id_priv,
{
cma_cancel_operation(id_priv, state);
- rdma_restrack_del(&id_priv->res);
if (id_priv->cma_dev) {
if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
if (id_priv->cm_id.ib)
@@ -1847,6 +1846,7 @@ static void _destroy_id(struct rdma_id_private *id_priv,
rdma_put_gid_attr(id_priv->id.route.addr.dev_addr.sgid_attr);
put_net(id_priv->id.route.addr.dev_addr.net);
+ rdma_restrack_del(&id_priv->res);
kfree(id_priv);
}
@@ -3729,7 +3729,6 @@ int rdma_bind_addr(struct rdma_cm_id *id, struct sockaddr *addr)
return 0;
err2:
- rdma_restrack_del(&id_priv->res);
if (id_priv->cma_dev)
cma_release_dev(id_priv);
err1:
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy
2020-09-07 12:21 ` [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy Leon Romanovsky
@ 2020-09-17 12:06 ` Jason Gunthorpe
2020-09-17 16:19 ` Leon Romanovsky
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 12:06 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma
On Mon, Sep 07, 2020 at 03:21:43PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Update the code to have similar destroy pattern like other IB objects.
>
> This change create asymmetry to the rdma_id_private create flow to make
> sure that memory is managed by restrack.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> drivers/infiniband/core/cma.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index f9ff8b7f05e7..24e09416de4f 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -1821,7 +1821,6 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> {
> cma_cancel_operation(id_priv, state);
>
> - rdma_restrack_del(&id_priv->res);
> if (id_priv->cma_dev) {
> if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
> if (id_priv->cm_id.ib)
> @@ -1847,6 +1846,7 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> rdma_put_gid_attr(id_priv->id.route.addr.dev_addr.sgid_attr);
>
> put_net(id_priv->id.route.addr.dev_addr.net);
> + rdma_restrack_del(&id_priv->res);
> kfree(id_priv);
> }
This is wrong, rdma_restrack_del() has to be called before
ib_destroy_cm_id() because restrack reaches into the cm_id and touches
that memory:
case RDMA_RESTRACK_CM_ID:
return container_of(res, struct rdma_id_private,
res)->id.device;
Which will now be freed after this change.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy
2020-09-17 12:06 ` Jason Gunthorpe
@ 2020-09-17 16:19 ` Leon Romanovsky
2020-09-17 16:32 ` Jason Gunthorpe
0 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-17 16:19 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma
On Thu, Sep 17, 2020 at 09:06:36AM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:43PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Update the code to have similar destroy pattern like other IB objects.
> >
> > This change create asymmetry to the rdma_id_private create flow to make
> > sure that memory is managed by restrack.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > drivers/infiniband/core/cma.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index f9ff8b7f05e7..24e09416de4f 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -1821,7 +1821,6 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> > {
> > cma_cancel_operation(id_priv, state);
> >
> > - rdma_restrack_del(&id_priv->res);
> > if (id_priv->cma_dev) {
> > if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
> > if (id_priv->cm_id.ib)
> > @@ -1847,6 +1846,7 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> > rdma_put_gid_attr(id_priv->id.route.addr.dev_addr.sgid_attr);
> >
> > put_net(id_priv->id.route.addr.dev_addr.net);
> > + rdma_restrack_del(&id_priv->res);
> > kfree(id_priv);
> > }
>
> This is wrong, rdma_restrack_del() has to be called before
> ib_destroy_cm_id() because restrack reaches into the cm_id and touches
> that memory:
>
> case RDMA_RESTRACK_CM_ID:
> return container_of(res, struct rdma_id_private,
> res)->id.device;
>
> Which will now be freed after this change.
We access "id" which is struct rdma_cm_id, while ib_destroy_cm_id()
releases something different struct ib_cm_id cm_id.ib.
"id" is embedded into id_priv released later in _destroy_id() function.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy
2020-09-17 16:19 ` Leon Romanovsky
@ 2020-09-17 16:32 ` Jason Gunthorpe
0 siblings, 0 replies; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-17 16:32 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma
On Thu, Sep 17, 2020 at 07:19:50PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 17, 2020 at 09:06:36AM -0300, Jason Gunthorpe wrote:
> > On Mon, Sep 07, 2020 at 03:21:43PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >
> > > Update the code to have similar destroy pattern like other IB objects.
> > >
> > > This change create asymmetry to the rdma_id_private create flow to make
> > > sure that memory is managed by restrack.
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/core/cma.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > > index f9ff8b7f05e7..24e09416de4f 100644
> > > +++ b/drivers/infiniband/core/cma.c
> > > @@ -1821,7 +1821,6 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> > > {
> > > cma_cancel_operation(id_priv, state);
> > >
> > > - rdma_restrack_del(&id_priv->res);
> > > if (id_priv->cma_dev) {
> > > if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
> > > if (id_priv->cm_id.ib)
> > > @@ -1847,6 +1846,7 @@ static void _destroy_id(struct rdma_id_private *id_priv,
> > > rdma_put_gid_attr(id_priv->id.route.addr.dev_addr.sgid_attr);
> > >
> > > put_net(id_priv->id.route.addr.dev_addr.net);
> > > + rdma_restrack_del(&id_priv->res);
> > > kfree(id_priv);
> > > }
> >
> > This is wrong, rdma_restrack_del() has to be called before
> > ib_destroy_cm_id() because restrack reaches into the cm_id and touches
> > that memory:
> >
> > case RDMA_RESTRACK_CM_ID:
> > return container_of(res, struct rdma_id_private,
> > res)->id.device;
> >
> > Which will now be freed after this change.
>
> We access "id" which is struct rdma_cm_id, while ib_destroy_cm_id()
> releases something different struct ib_cm_id cm_id.ib.
Hm, OK
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 02/14] RDMA/mlx5: Don't call to restrack recursively
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 03/14] RDMA/mlx4: Provide port number for special QPs Leon Romanovsky
` (11 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma, Mark Zhang
From: Leon Romanovsky <leonro@mellanox.com>
The restrack is going to manage memory of all IB objects and must be
called before object is created. GSI QP in the mlx5_ib separated between
creating dummy interface and HW object beneath. This was achieved by
double call to ib_create_qp().
In order to skip such reentry call to internal driver create_qp code.
Reviewed-by: Mark Zhang <markz@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/hw/mlx5/gsi.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/gsi.c b/drivers/infiniband/hw/mlx5/gsi.c
index 40d418153891..d9f300f78a82 100644
--- a/drivers/infiniband/hw/mlx5/gsi.c
+++ b/drivers/infiniband/hw/mlx5/gsi.c
@@ -182,13 +182,25 @@ struct ib_qp *mlx5_ib_gsi_create_qp(struct ib_pd *pd,
hw_init_attr.cap.max_send_sge = 0;
hw_init_attr.cap.max_inline_data = 0;
}
- gsi->rx_qp = ib_create_qp(pd, &hw_init_attr);
+
+ gsi->rx_qp = mlx5_ib_create_qp(pd, &hw_init_attr, NULL);
if (IS_ERR(gsi->rx_qp)) {
mlx5_ib_warn(dev, "unable to create hardware GSI QP. error %ld\n",
PTR_ERR(gsi->rx_qp));
ret = PTR_ERR(gsi->rx_qp);
goto err_destroy_cq;
}
+ gsi->rx_qp->device = pd->device;
+ gsi->rx_qp->pd = pd;
+ gsi->rx_qp->real_qp = gsi->rx_qp;
+
+ gsi->rx_qp->qp_type = hw_init_attr.qp_type;
+ gsi->rx_qp->send_cq = hw_init_attr.send_cq;
+ gsi->rx_qp->recv_cq = hw_init_attr.recv_cq;
+ gsi->rx_qp->event_handler = hw_init_attr.event_handler;
+ spin_lock_init(&gsi->rx_qp->mr_lock);
+ INIT_LIST_HEAD(&gsi->rx_qp->rdma_mrs);
+ INIT_LIST_HEAD(&gsi->rx_qp->sig_mrs);
dev->devr.ports[init_attr->port_num - 1].gsi = gsi;
@@ -219,7 +231,7 @@ int mlx5_ib_gsi_destroy_qp(struct ib_qp *qp)
mlx5_ib_dbg(dev, "destroying GSI QP\n");
mutex_lock(&dev->devr.mutex);
- ret = ib_destroy_qp(gsi->rx_qp);
+ ret = mlx5_ib_destroy_qp(gsi->rx_qp, NULL);
if (ret) {
mlx5_ib_warn(dev, "unable to destroy hardware GSI QP. error %d\n",
ret);
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH rdma-next v2 03/14] RDMA/mlx4: Provide port number for special QPs
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 01/14] RDMA/cma: Delete from restrack DB after successful destroy Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 02/14] RDMA/mlx5: Don't call to restrack recursively Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 04/14] RDMA/restrack: Count references to the verbs objects Leon Romanovsky
` (10 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma, Yishai Hadas
From: Leon Romanovsky <leonro@nvidia.com>
Special QPs created by mlx4 have same QP port borrowed from
the context, while they are expected to have different ones.
Fix it by using HW physical port instead.
It fixes the following error during driver init:
[ 12.074150] mlx4_core 0000:05:00.0: mlx4_ib: initializing demux service for 128 qp1 clients
[ 12.084036] <mlx4_ib> create_pv_sqp: Couldn't create special QP (-16)
[ 12.085123] <mlx4_ib> create_pv_resources: Couldn't create QP1 (-16)
[ 12.088300] mlx4_en: Mellanox ConnectX HCA Ethernet driver v4.0-0
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
drivers/infiniband/hw/mlx4/mad.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 8bd16474708f..be0057222bbc 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1792,7 +1792,7 @@ static void pv_qp_event_handler(struct ib_event *event, void *qp_context)
}
static int create_pv_sqp(struct mlx4_ib_demux_pv_ctx *ctx,
- enum ib_qp_type qp_type, int create_tun)
+ enum ib_qp_type qp_type, int port, int create_tun)
{
int i, ret;
struct mlx4_ib_demux_pv_qp *tun_qp;
@@ -1822,12 +1822,13 @@ static int create_pv_sqp(struct mlx4_ib_demux_pv_ctx *ctx,
qp_init_attr.proxy_qp_type = qp_type;
qp_attr_mask_INIT = IB_QP_STATE | IB_QP_PKEY_INDEX |
IB_QP_QKEY | IB_QP_PORT;
+ qp_init_attr.init_attr.port_num = ctx->port;
} else {
qp_init_attr.init_attr.qp_type = qp_type;
qp_init_attr.init_attr.create_flags = MLX4_IB_SRIOV_SQP;
qp_attr_mask_INIT = IB_QP_STATE | IB_QP_PKEY_INDEX | IB_QP_QKEY;
+ qp_init_attr.init_attr.port_num = port;
}
- qp_init_attr.init_attr.port_num = ctx->port;
qp_init_attr.init_attr.qp_context = ctx;
qp_init_attr.init_attr.event_handler = pv_qp_event_handler;
tun_qp->qp = ib_create_qp(ctx->pd, &qp_init_attr.init_attr);
@@ -2026,7 +2027,7 @@ static int create_pv_resources(struct ib_device *ibdev, int slave, int port,
}
if (ctx->has_smi) {
- ret = create_pv_sqp(ctx, IB_QPT_SMI, create_tun);
+ ret = create_pv_sqp(ctx, IB_QPT_SMI, port, create_tun);
if (ret) {
pr_err("Couldn't create %s QP0 (%d)\n",
create_tun ? "tunnel for" : "", ret);
@@ -2034,7 +2035,7 @@ static int create_pv_resources(struct ib_device *ibdev, int slave, int port,
}
}
- ret = create_pv_sqp(ctx, IB_QPT_GSI, create_tun);
+ ret = create_pv_sqp(ctx, IB_QPT_GSI, port, create_tun);
if (ret) {
pr_err("Couldn't create %s QP1 (%d)\n",
create_tun ? "tunnel for" : "", ret);
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH rdma-next v2 04/14] RDMA/restrack: Count references to the verbs objects
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (2 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 03/14] RDMA/mlx4: Provide port number for special QPs Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-18 17:00 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 05/14] RDMA/restrack: Simplify restrack tracking in kernel flows Leon Romanovsky
` (9 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
Refactor the restrack code to make sure that kref inside restrack entry
properly kref the object in which it is embedded. This slight change is
needed for future conversions of MR and QP which are refcounted before
the release and kfree.
The ideal flow from ib_core perspective as follows:
* Allocate ib_* structure with rdma_zalloc_*.
* Set everything that is known to ib_core to that newly created object.
* Initialize kref with restrack help
* Call to driver specific allocation functions.
* Insert into restrack DB
....
* Return and release restrack with restrack_put.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cma.c | 5 ++-
drivers/infiniband/core/core_priv.h | 3 +-
drivers/infiniband/core/counters.c | 6 ++-
drivers/infiniband/core/cq.c | 7 ++--
drivers/infiniband/core/restrack.c | 42 ++++++++++++++-----
drivers/infiniband/core/restrack.h | 3 ++
drivers/infiniband/core/uverbs_cmd.c | 13 ++++--
drivers/infiniband/core/uverbs_std_types_cq.c | 4 +-
drivers/infiniband/core/verbs.c | 18 ++++----
include/rdma/restrack.h | 7 ----
10 files changed, 70 insertions(+), 38 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 24e09416de4f..a483d906c2e2 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -855,8 +855,6 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
if (!id_priv)
return ERR_PTR(-ENOMEM);
- rdma_restrack_set_task(&id_priv->res, caller);
- id_priv->res.type = RDMA_RESTRACK_CM_ID;
id_priv->state = RDMA_CM_IDLE;
id_priv->id.context = context;
id_priv->id.event_handler = event_handler;
@@ -876,6 +874,9 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
id_priv->id.route.addr.dev_addr.net = get_net(net);
id_priv->seq_num &= 0x00ffffff;
+ rdma_restrack_new(&id_priv->res, RDMA_RESTRACK_CM_ID);
+ rdma_restrack_set_task(&id_priv->res, caller);
+
return &id_priv->id;
}
EXPORT_SYMBOL(__rdma_create_id);
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index a1e6a67b2c4a..2ad276cd9781 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -44,6 +44,7 @@
#include <rdma/ib_mad.h>
#include <rdma/restrack.h>
#include "mad_priv.h"
+#include "restrack.h"
/* Total number of ports combined across all struct ib_devices's */
#define RDMA_MAX_PORTS 8192
@@ -352,6 +353,7 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
INIT_LIST_HEAD(&qp->rdma_mrs);
INIT_LIST_HEAD(&qp->sig_mrs);
+ rdma_restrack_new(&qp->res, RDMA_RESTRACK_QP);
/*
* We don't track XRC QPs for now, because they don't have PD
* and more importantly they are created internaly by driver,
@@ -359,7 +361,6 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
*/
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);
else
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index 636166880442..c059de99d19c 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -80,8 +80,9 @@ static struct rdma_counter *rdma_counter_alloc(struct ib_device *dev, u8 port,
counter->device = dev;
counter->port = port;
- counter->res.type = RDMA_RESTRACK_COUNTER;
- counter->stats = dev->ops.counter_alloc_stats(counter);
+
+ rdma_restrack_new(&counter->res, RDMA_RESTRACK_COUNTER);
+ counter->stats = dev->ops.counter_alloc_stats(counter);
if (!counter->stats)
goto err_stats;
@@ -107,6 +108,7 @@ static struct rdma_counter *rdma_counter_alloc(struct ib_device *dev, u8 port,
mutex_unlock(&port_counter->lock);
kfree(counter->stats);
err_stats:
+ rdma_restrack_put(&counter->res);
kfree(counter);
return NULL;
}
diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c
index 11edf7308eac..8bebd29a81bd 100644
--- a/drivers/infiniband/core/cq.c
+++ b/drivers/infiniband/core/cq.c
@@ -235,15 +235,13 @@ struct ib_cq *__ib_alloc_cq(struct ib_device *dev, void *private, int nr_cqe,
if (!cq->wc)
goto out_free_cq;
- cq->res.type = RDMA_RESTRACK_CQ;
+ rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
rdma_restrack_set_task(&cq->res, caller);
ret = dev->ops.create_cq(cq, &cq_attr, NULL);
if (ret)
goto out_free_wc;
- rdma_restrack_kadd(&cq->res);
-
rdma_dim_init(cq);
switch (cq->poll_ctx) {
@@ -269,14 +267,15 @@ struct ib_cq *__ib_alloc_cq(struct ib_device *dev, void *private, int nr_cqe,
goto out_destroy_cq;
}
+ rdma_restrack_kadd(&cq->res);
trace_cq_alloc(cq, nr_cqe, comp_vector, poll_ctx);
return cq;
out_destroy_cq:
rdma_dim_destroy(cq);
- rdma_restrack_del(&cq->res);
cq->device->ops.destroy_cq(cq, NULL);
out_free_wc:
+ rdma_restrack_put(&cq->res);
kfree(cq->wc);
out_free_cq:
kfree(cq);
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 62fbb0ae9cb4..22c658e8c157 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -202,6 +202,21 @@ void rdma_restrack_attach_task(struct rdma_restrack_entry *res,
res->task = task;
}
+/**
+ * rdma_restrack_new() - Initializes new restrack entry to allow _put() interface
+ * to release memory in fully automatic way.
+ * @res - Entry to initialize
+ * @type - REstrack type
+ */
+void rdma_restrack_new(struct rdma_restrack_entry *res,
+ enum rdma_restrack_type type)
+{
+ kref_init(&res->kref);
+ init_completion(&res->comp);
+ res->type = type;
+}
+EXPORT_SYMBOL(rdma_restrack_new);
+
static void rdma_restrack_add(struct rdma_restrack_entry *res)
{
struct ib_device *dev = res_to_dev(res);
@@ -213,8 +228,6 @@ static void rdma_restrack_add(struct rdma_restrack_entry *res)
rt = &dev->res[res->type];
- kref_init(&res->kref);
- init_completion(&res->comp);
if (res->type == RDMA_RESTRACK_QP) {
/* Special case to ensure that LQPN points to right QP */
struct ib_qp *qp = container_of(res, struct ib_qp, res);
@@ -305,6 +318,10 @@ static void restrack_release(struct kref *kref)
struct rdma_restrack_entry *res;
res = container_of(kref, struct rdma_restrack_entry, kref);
+ if (res->task) {
+ put_task_struct(res->task);
+ res->task = NULL;
+ }
complete(&res->comp);
}
@@ -314,14 +331,23 @@ int rdma_restrack_put(struct rdma_restrack_entry *res)
}
EXPORT_SYMBOL(rdma_restrack_put);
+/**
+ * rdma_restrack_del() - delete object from the reource tracking database
+ * @res: resource entry
+ */
void rdma_restrack_del(struct rdma_restrack_entry *res)
{
struct rdma_restrack_entry *old;
struct rdma_restrack_root *rt;
struct ib_device *dev;
- if (!res->valid)
- goto out;
+ if (!res->valid) {
+ if (res->task) {
+ put_task_struct(res->task);
+ res->task = NULL;
+ }
+ return;
+ }
dev = res_to_dev(res);
if (WARN_ON(!dev))
@@ -330,16 +356,12 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
rt = &dev->res[res->type];
old = xa_erase(&rt->xa, res->id);
+ if (res->type == RDMA_RESTRACK_MR || res->type == RDMA_RESTRACK_QP)
+ return;
WARN_ON(old != res);
res->valid = false;
rdma_restrack_put(res);
wait_for_completion(&res->comp);
-
-out:
- if (res->task) {
- put_task_struct(res->task);
- res->task = NULL;
- }
}
EXPORT_SYMBOL(rdma_restrack_del);
diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
index d084e5f89849..16006a5e7408 100644
--- a/drivers/infiniband/core/restrack.h
+++ b/drivers/infiniband/core/restrack.h
@@ -25,6 +25,9 @@ struct rdma_restrack_root {
int rdma_restrack_init(struct ib_device *dev);
void rdma_restrack_clean(struct ib_device *dev);
+void rdma_restrack_del(struct rdma_restrack_entry *res);
+void rdma_restrack_new(struct rdma_restrack_entry *res,
+ enum rdma_restrack_type type);
void rdma_restrack_attach_task(struct rdma_restrack_entry *res,
struct task_struct *task);
#endif /* _RDMA_CORE_RESTRACK_H_ */
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 0f359f8ae4db..5fbf05ce7ccb 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -218,10 +218,11 @@ int ib_alloc_ucontext(struct uverbs_attr_bundle *attrs)
if (!ucontext)
return -ENOMEM;
- ucontext->res.type = RDMA_RESTRACK_CTX;
ucontext->device = ib_dev;
ucontext->ufile = ufile;
xa_init_flags(&ucontext->mmap_xa, XA_FLAGS_ALLOC);
+
+ rdma_restrack_new(&ucontext->res, RDMA_RESTRACK_CTX);
attrs->context = ucontext;
return 0;
}
@@ -313,6 +314,7 @@ static int ib_uverbs_get_context(struct uverbs_attr_bundle *attrs)
err_uobj:
rdma_alloc_abort_uobject(uobj, attrs, false);
err_ucontext:
+ rdma_restrack_put(&attrs->context->res);
kfree(attrs->context);
attrs->context = NULL;
return ret;
@@ -439,8 +441,8 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
pd->device = ib_dev;
pd->uobject = uobj;
atomic_set(&pd->usecnt, 0);
- pd->res.type = RDMA_RESTRACK_PD;
+ rdma_restrack_new(&pd->res, RDMA_RESTRACK_PD);
ret = ib_dev->ops.alloc_pd(pd, &attrs->driver_udata);
if (ret)
goto err_alloc;
@@ -453,6 +455,7 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
return uverbs_response(attrs, &resp, sizeof(resp));
err_alloc:
+ rdma_restrack_put(&pd->res);
kfree(pd);
err:
uobj_alloc_abort(uobj, attrs);
@@ -742,8 +745,9 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
mr->sig_attrs = NULL;
mr->uobject = uobj;
atomic_inc(&pd->usecnt);
- mr->res.type = RDMA_RESTRACK_MR;
mr->iova = cmd.hca_va;
+
+ rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
rdma_restrack_uadd(&mr->res);
uobj->object = mr;
@@ -994,8 +998,8 @@ static int create_cq(struct uverbs_attr_bundle *attrs,
cq->event_handler = ib_uverbs_cq_event_handler;
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
atomic_set(&cq->usecnt, 0);
- cq->res.type = RDMA_RESTRACK_CQ;
+ rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
ret = ib_dev->ops.create_cq(cq, &attr, &attrs->driver_udata);
if (ret)
goto err_free;
@@ -1013,6 +1017,7 @@ static int create_cq(struct uverbs_attr_bundle *attrs,
return uverbs_response(attrs, &resp, sizeof(resp));
err_free:
+ rdma_restrack_put(&cq->res);
kfree(cq);
err_file:
if (ev_file)
diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
index b1c7dacc02de..3a5fd6c9ba72 100644
--- a/drivers/infiniband/core/uverbs_std_types_cq.c
+++ b/drivers/infiniband/core/uverbs_std_types_cq.c
@@ -33,6 +33,7 @@
#include <rdma/uverbs_std_types.h>
#include "rdma_core.h"
#include "uverbs.h"
+#include "restrack.h"
static int uverbs_free_cq(struct ib_uobject *uobject,
enum rdma_remove_reason why,
@@ -123,8 +124,8 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
cq->event_handler = ib_uverbs_cq_event_handler;
cq->cq_context = ev_file ? &ev_file->ev_queue : NULL;
atomic_set(&cq->usecnt, 0);
- cq->res.type = RDMA_RESTRACK_CQ;
+ rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
ret = ib_dev->ops.create_cq(cq, &attr, &attrs->driver_udata);
if (ret)
goto err_free;
@@ -139,6 +140,7 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
return ret;
err_free:
+ rdma_restrack_put(&cq->res);
kfree(cq);
err_event_file:
if (obj->uevent.event_file)
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 8fb5c5c40c8b..f613f60299eb 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -272,11 +272,12 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
atomic_set(&pd->usecnt, 0);
pd->flags = flags;
- pd->res.type = RDMA_RESTRACK_PD;
+ rdma_restrack_new(&pd->res, RDMA_RESTRACK_PD);
rdma_restrack_set_task(&pd->res, caller);
ret = device->ops.alloc_pd(pd, NULL);
if (ret) {
+ rdma_restrack_put(&pd->res);
kfree(pd);
return ERR_PTR(ret);
}
@@ -1996,11 +1997,13 @@ struct ib_cq *__ib_create_cq(struct ib_device *device,
cq->event_handler = event_handler;
cq->cq_context = cq_context;
atomic_set(&cq->usecnt, 0);
- cq->res.type = RDMA_RESTRACK_CQ;
+
+ rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
rdma_restrack_set_task(&cq->res, caller);
ret = device->ops.create_cq(cq, cq_attr, NULL);
if (ret) {
+ rdma_restrack_put(&cq->res);
kfree(cq);
return ERR_PTR(ret);
}
@@ -2076,7 +2079,8 @@ struct ib_mr *ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
mr->pd = pd;
mr->dm = NULL;
atomic_inc(&pd->usecnt);
- mr->res.type = RDMA_RESTRACK_MR;
+
+ rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
rdma_restrack_kadd(&mr->res);
return mr;
@@ -2156,11 +2160,11 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
mr->uobject = NULL;
atomic_inc(&pd->usecnt);
mr->need_inval = false;
- mr->res.type = RDMA_RESTRACK_MR;
- rdma_restrack_kadd(&mr->res);
mr->type = mr_type;
mr->sig_attrs = NULL;
+ rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
+ rdma_restrack_kadd(&mr->res);
out:
trace_mr_alloc(pd, mr_type, max_num_sg, mr);
return mr;
@@ -2216,11 +2220,11 @@ struct ib_mr *ib_alloc_mr_integrity(struct ib_pd *pd,
mr->uobject = NULL;
atomic_inc(&pd->usecnt);
mr->need_inval = false;
- mr->res.type = RDMA_RESTRACK_MR;
- rdma_restrack_kadd(&mr->res);
mr->type = IB_MR_TYPE_INTEGRITY;
mr->sig_attrs = sig_attrs;
+ rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
+ rdma_restrack_kadd(&mr->res);
out:
trace_mr_integ_alloc(pd, max_num_data_sg, max_num_meta_sg, mr);
return mr;
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index 7682d1bcf789..d7c166237939 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -110,13 +110,6 @@ int rdma_restrack_count(struct ib_device *dev,
void rdma_restrack_kadd(struct rdma_restrack_entry *res);
void rdma_restrack_uadd(struct rdma_restrack_entry *res);
-/**
- * rdma_restrack_del() - delete object from the reource tracking database
- * @res: resource entry
- * @type: actual type of object to operate
- */
-void rdma_restrack_del(struct rdma_restrack_entry *res);
-
/**
* rdma_is_kernel_res() - check the owner of resource
* @res: resource entry
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 04/14] RDMA/restrack: Count references to the verbs objects
2020-09-07 12:21 ` [PATCH rdma-next v2 04/14] RDMA/restrack: Count references to the verbs objects Leon Romanovsky
@ 2020-09-18 17:00 ` Jason Gunthorpe
2020-09-19 8:30 ` Leon Romanovsky
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-18 17:00 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma
On Mon, Sep 07, 2020 at 03:21:46PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Refactor the restrack code to make sure that kref inside restrack entry
> properly kref the object in which it is embedded. This slight change is
> needed for future conversions of MR and QP which are refcounted before
> the release and kfree.
>
> The ideal flow from ib_core perspective as follows:
> * Allocate ib_* structure with rdma_zalloc_*.
Given how things are going it would be good to eventually include the
kref initialization in the allocation:
struct rdma_restrack_entry *_rdma_zalloc_drv_obj_gfp(struct ib_device *ibdev,
size_t size,
size_t res_offset,
gfp_t flags);
{
struct rdma_restrack_entry res = kzalloc(size, flags) + res_offset;
[..]
return res;
}
#define rdma_zalloc_drv_obj_gfp(ib_dev, ib_type, gfp) \
container_of(_rdma_zalloc_drv_obj_gfp( \
ib_dev, (ib_dev)->ops.size_##ib_type, \
offsetof(struct ib_type, res), gfp), \
struct ib_type, res)
As the idea is every drv_obj will have the kref
The patch looks OK, the rdma_restrack_new calls are all near their
matching allocations
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 04/14] RDMA/restrack: Count references to the verbs objects
2020-09-18 17:00 ` Jason Gunthorpe
@ 2020-09-19 8:30 ` Leon Romanovsky
0 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-19 8:30 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma
On Fri, Sep 18, 2020 at 02:00:20PM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:46PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Refactor the restrack code to make sure that kref inside restrack entry
> > properly kref the object in which it is embedded. This slight change is
> > needed for future conversions of MR and QP which are refcounted before
> > the release and kfree.
> >
> > The ideal flow from ib_core perspective as follows:
> > * Allocate ib_* structure with rdma_zalloc_*.
>
> Given how things are going it would be good to eventually include the
> kref initialization in the allocation:
Yep, this is the plan.
>
> struct rdma_restrack_entry *_rdma_zalloc_drv_obj_gfp(struct ib_device *ibdev,
> size_t size,
> size_t res_offset,
> gfp_t flags);
> {
> struct rdma_restrack_entry res = kzalloc(size, flags) + res_offset;
> [..]
> return res;
> }
>
> #define rdma_zalloc_drv_obj_gfp(ib_dev, ib_type, gfp) \
> container_of(_rdma_zalloc_drv_obj_gfp( \
> ib_dev, (ib_dev)->ops.size_##ib_type, \
> offsetof(struct ib_type, res), gfp), \
> struct ib_type, res)
>
> As the idea is every drv_obj will have the kref
>
> The patch looks OK, the rdma_restrack_new calls are all near their
> matching allocations
>
> Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 05/14] RDMA/restrack: Simplify restrack tracking in kernel flows
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (3 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 04/14] RDMA/restrack: Count references to the verbs objects Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 06/14] RDMA/restrack: Improve readability in task name management Leon Romanovsky
` (8 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
There is no need to do an extra function indirection just to add
restrack entry to the DB. This patch prepares the code to the future
requirement of making restrack is mandatory for managing ib objects.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cma.c | 2 +-
drivers/infiniband/core/core_priv.h | 6 ++--
drivers/infiniband/core/counters.c | 2 +-
drivers/infiniband/core/cq.c | 2 +-
drivers/infiniband/core/restrack.c | 46 ++++-------------------------
drivers/infiniband/core/restrack.h | 1 +
drivers/infiniband/core/verbs.c | 13 ++++----
include/rdma/restrack.h | 1 -
8 files changed, 22 insertions(+), 51 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index a483d906c2e2..feed3c04979a 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -468,7 +468,7 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
rdma_node_get_transport(cma_dev->device->node_type);
list_add_tail(&id_priv->list, &cma_dev->id_list);
if (id_priv->res.kern_name)
- rdma_restrack_kadd(&id_priv->res);
+ rdma_restrack_add(&id_priv->res);
else
rdma_restrack_uadd(&id_priv->res);
trace_cm_id_attach(id_priv, cma_dev->device);
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 2ad276cd9781..cf5a50cefa39 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -363,8 +363,10 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
if ((qp_type < IB_QPT_MAX && !is_xrc) || qp_type == IB_QPT_DRIVER) {
if (uobj)
rdma_restrack_uadd(&qp->res);
- else
- rdma_restrack_kadd(&qp->res);
+ else {
+ rdma_restrack_set_task(&qp->res, pd->res.kern_name);
+ rdma_restrack_add(&qp->res);
+ }
} else
qp->res.valid = false;
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index c059de99d19c..e13c500a9ec0 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -252,7 +252,7 @@ static void rdma_counter_res_add(struct rdma_counter *counter,
{
if (rdma_is_kernel_res(&qp->res)) {
rdma_restrack_set_task(&counter->res, qp->res.kern_name);
- rdma_restrack_kadd(&counter->res);
+ rdma_restrack_add(&counter->res);
} else {
rdma_restrack_attach_task(&counter->res, qp->res.task);
rdma_restrack_uadd(&counter->res);
diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c
index 8bebd29a81bd..e0e92441323f 100644
--- a/drivers/infiniband/core/cq.c
+++ b/drivers/infiniband/core/cq.c
@@ -267,7 +267,7 @@ struct ib_cq *__ib_alloc_cq(struct ib_device *dev, void *private, int nr_cqe,
goto out_destroy_cq;
}
- rdma_restrack_kadd(&cq->res);
+ rdma_restrack_add(&cq->res);
trace_cq_alloc(cq, nr_cqe, comp_vector, poll_ctx);
return cq;
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 22c658e8c157..88d3852676a9 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -123,32 +123,6 @@ int rdma_restrack_count(struct ib_device *dev, enum rdma_restrack_type type)
}
EXPORT_SYMBOL(rdma_restrack_count);
-static void set_kern_name(struct rdma_restrack_entry *res)
-{
- struct ib_pd *pd;
-
- switch (res->type) {
- case RDMA_RESTRACK_QP:
- pd = container_of(res, struct ib_qp, res)->pd;
- if (!pd) {
- WARN_ONCE(true, "XRC QPs are not supported\n");
- /* Survive, despite the programmer's error */
- res->kern_name = " ";
- }
- break;
- case RDMA_RESTRACK_MR:
- pd = container_of(res, struct ib_mr, res)->pd;
- break;
- default:
- /* Other types set kern_name directly */
- pd = NULL;
- break;
- }
-
- if (pd)
- res->kern_name = pd->res.kern_name;
-}
-
static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
{
switch (res->type) {
@@ -217,7 +191,11 @@ void rdma_restrack_new(struct rdma_restrack_entry *res,
}
EXPORT_SYMBOL(rdma_restrack_new);
-static void rdma_restrack_add(struct rdma_restrack_entry *res)
+/**
+ * rdma_restrack_add() - add object to the reource tracking database
+ * @res: resource entry
+ */
+void rdma_restrack_add(struct rdma_restrack_entry *res)
{
struct ib_device *dev = res_to_dev(res);
struct rdma_restrack_root *rt;
@@ -249,19 +227,7 @@ static void rdma_restrack_add(struct rdma_restrack_entry *res)
if (!ret)
res->valid = true;
}
-
-/**
- * rdma_restrack_kadd() - add kernel object to the reource tracking database
- * @res: resource entry
- */
-void rdma_restrack_kadd(struct rdma_restrack_entry *res)
-{
- res->task = NULL;
- set_kern_name(res);
- res->user = false;
- rdma_restrack_add(res);
-}
-EXPORT_SYMBOL(rdma_restrack_kadd);
+EXPORT_SYMBOL(rdma_restrack_add);
/**
* rdma_restrack_uadd() - add user object to the reource tracking database
diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
index 16006a5e7408..d35c4c41d2ff 100644
--- a/drivers/infiniband/core/restrack.h
+++ b/drivers/infiniband/core/restrack.h
@@ -25,6 +25,7 @@ struct rdma_restrack_root {
int rdma_restrack_init(struct ib_device *dev);
void rdma_restrack_clean(struct ib_device *dev);
+void rdma_restrack_add(struct rdma_restrack_entry *res);
void rdma_restrack_del(struct rdma_restrack_entry *res);
void rdma_restrack_new(struct rdma_restrack_entry *res,
enum rdma_restrack_type type);
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index f613f60299eb..9c4dd59b9cf9 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -281,7 +281,7 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
kfree(pd);
return ERR_PTR(ret);
}
- rdma_restrack_kadd(&pd->res);
+ rdma_restrack_add(&pd->res);
if (device->attrs.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
pd->local_dma_lkey = device->local_dma_lkey;
@@ -2008,7 +2008,7 @@ struct ib_cq *__ib_create_cq(struct ib_device *device,
return ERR_PTR(ret);
}
- rdma_restrack_kadd(&cq->res);
+ rdma_restrack_add(&cq->res);
return cq;
}
EXPORT_SYMBOL(__ib_create_cq);
@@ -2081,7 +2081,8 @@ struct ib_mr *ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
atomic_inc(&pd->usecnt);
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_kadd(&mr->res);
+ rdma_restrack_set_task(&mr->res, pd->res.kern_name);
+ rdma_restrack_add(&mr->res);
return mr;
}
@@ -2164,7 +2165,8 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
mr->sig_attrs = NULL;
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_kadd(&mr->res);
+ rdma_restrack_set_task(&mr->res, pd->res.kern_name);
+ rdma_restrack_add(&mr->res);
out:
trace_mr_alloc(pd, mr_type, max_num_sg, mr);
return mr;
@@ -2224,7 +2226,8 @@ struct ib_mr *ib_alloc_mr_integrity(struct ib_pd *pd,
mr->sig_attrs = sig_attrs;
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_kadd(&mr->res);
+ rdma_restrack_set_task(&mr->res, pd->res.kern_name);
+ rdma_restrack_add(&mr->res);
out:
trace_mr_integ_alloc(pd, max_num_data_sg, max_num_meta_sg, mr);
return mr;
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index d7c166237939..db59e208f5e8 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -107,7 +107,6 @@ struct rdma_restrack_entry {
int rdma_restrack_count(struct ib_device *dev,
enum rdma_restrack_type type);
-void rdma_restrack_kadd(struct rdma_restrack_entry *res);
void rdma_restrack_uadd(struct rdma_restrack_entry *res);
/**
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH rdma-next v2 06/14] RDMA/restrack: Improve readability in task name management
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (4 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 05/14] RDMA/restrack: Simplify restrack tracking in kernel flows Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-18 23:17 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device Leon Romanovsky
` (7 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
Reduce ambiguity in interfaces to set resource names and manage
struct task reference counters.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cma.c | 15 ++--
drivers/infiniband/core/core_priv.h | 12 +--
drivers/infiniband/core/counters.c | 9 +--
drivers/infiniband/core/cq.c | 2 +-
drivers/infiniband/core/restrack.c | 73 ++++++++++---------
drivers/infiniband/core/restrack.h | 6 +-
drivers/infiniband/core/uverbs_cmd.c | 14 +++-
drivers/infiniband/core/uverbs_std_types_cq.c | 4 +-
drivers/infiniband/core/verbs.c | 10 +--
include/rdma/restrack.h | 11 ---
10 files changed, 75 insertions(+), 81 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index feed3c04979a..3fc3c821743d 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -467,10 +467,13 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
id_priv->id.route.addr.dev_addr.transport =
rdma_node_get_transport(cma_dev->device->node_type);
list_add_tail(&id_priv->list, &cma_dev->id_list);
- if (id_priv->res.kern_name)
- rdma_restrack_add(&id_priv->res);
- else
- rdma_restrack_uadd(&id_priv->res);
+ /*
+ * For example UCMA doesn't set kern_name and below function will
+ * attach to "current" task.
+ */
+ rdma_restrack_set_name(&id_priv->res, id_priv->res.kern_name);
+ rdma_restrack_add(&id_priv->res);
+
trace_cm_id_attach(id_priv, cma_dev->device);
}
@@ -875,7 +878,7 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
id_priv->seq_num &= 0x00ffffff;
rdma_restrack_new(&id_priv->res, RDMA_RESTRACK_CM_ID);
- rdma_restrack_set_task(&id_priv->res, caller);
+ rdma_restrack_set_name(&id_priv->res, caller);
return &id_priv->id;
}
@@ -4161,7 +4164,7 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
lockdep_assert_held(&id_priv->handler_mutex);
- rdma_restrack_set_task(&id_priv->res, caller);
+ rdma_restrack_set_name(&id_priv->res, caller);
if (READ_ONCE(id_priv->state) != RDMA_CM_CONNECT)
return -EINVAL;
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index cf5a50cefa39..e84b0fedaacb 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -361,15 +361,9 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
*/
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) {
- if (uobj)
- rdma_restrack_uadd(&qp->res);
- else {
- rdma_restrack_set_task(&qp->res, pd->res.kern_name);
- rdma_restrack_add(&qp->res);
- }
- } else
- qp->res.valid = false;
-
+ rdma_restrack_parent_name(&qp->res, &pd->res);
+ rdma_restrack_add(&qp->res);
+ }
return qp;
}
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index e13c500a9ec0..e4ff0d3328b6 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -250,13 +250,8 @@ static struct rdma_counter *rdma_get_counter_auto_mode(struct ib_qp *qp,
static void rdma_counter_res_add(struct rdma_counter *counter,
struct ib_qp *qp)
{
- if (rdma_is_kernel_res(&qp->res)) {
- rdma_restrack_set_task(&counter->res, qp->res.kern_name);
- rdma_restrack_add(&counter->res);
- } else {
- rdma_restrack_attach_task(&counter->res, qp->res.task);
- rdma_restrack_uadd(&counter->res);
- }
+ rdma_restrack_parent_name(&counter->res, &qp->res);
+ rdma_restrack_add(&counter->res);
}
static void counter_release(struct kref *kref)
diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c
index e0e92441323f..704613b19eb3 100644
--- a/drivers/infiniband/core/cq.c
+++ b/drivers/infiniband/core/cq.c
@@ -236,7 +236,7 @@ struct ib_cq *__ib_alloc_cq(struct ib_device *dev, void *private, int nr_cqe,
goto out_free_cq;
rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
- rdma_restrack_set_task(&cq->res, caller);
+ rdma_restrack_set_name(&cq->res, caller);
ret = dev->ops.create_cq(cq, &cq_attr, NULL);
if (ret)
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 88d3852676a9..0c67acf2169d 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -147,34 +147,56 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
}
}
-void rdma_restrack_set_task(struct rdma_restrack_entry *res,
- const char *caller)
+/**
+ * rdma_restrack_attach_task() - attach the task onto this resource,
+ * valid for user space restrack entries.
+ * @res: resource entry
+ * @task: the task to attach
+ */
+static void rdma_restrack_attach_task(struct rdma_restrack_entry *res,
+ struct task_struct *task)
{
- if (caller) {
- res->kern_name = caller;
+ if (WARN_ON_ONCE(!task))
return;
- }
if (res->task)
put_task_struct(res->task);
- get_task_struct(current);
- res->task = current;
+ get_task_struct(task);
+ res->task = task;
+ res->user = true;
}
-EXPORT_SYMBOL(rdma_restrack_set_task);
/**
- * rdma_restrack_attach_task() - attach the task onto this resource
+ * rdma_restrack_set_name() - set the task for this resource
* @res: resource entry
- * @task: the task to attach, the current task will be used if it is NULL.
+ * @caller: kernel name, the current task will be used if the caller is NULL.
*/
-void rdma_restrack_attach_task(struct rdma_restrack_entry *res,
- struct task_struct *task)
+void rdma_restrack_set_name(struct rdma_restrack_entry *res, const char *caller)
{
- if (res->task)
- put_task_struct(res->task);
- get_task_struct(task);
- res->task = task;
+ if (caller) {
+ res->kern_name = caller;
+ return;
+ }
+
+ rdma_restrack_attach_task(res, current);
+}
+EXPORT_SYMBOL(rdma_restrack_set_name);
+
+/**
+ * rdma_restrack_parent_name() - set the restrack name properties based
+ * on parent restrack
+ * @dst: destination resource entry
+ * @parent: parent resource entry
+ */
+void rdma_restrack_parent_name(struct rdma_restrack_entry *dst,
+ struct rdma_restrack_entry *parent)
+{
+ if (rdma_is_kernel_res(parent))
+ dst->kern_name = parent->kern_name;
+ else
+ rdma_restrack_attach_task(dst, parent->task);
}
+EXPORT_SYMBOL(rdma_restrack_parent_name);
/**
* rdma_restrack_new() - Initializes new restrack entry to allow _put() interface
@@ -229,25 +251,6 @@ void rdma_restrack_add(struct rdma_restrack_entry *res)
}
EXPORT_SYMBOL(rdma_restrack_add);
-/**
- * rdma_restrack_uadd() - add user object to the reource tracking database
- * @res: resource entry
- */
-void rdma_restrack_uadd(struct rdma_restrack_entry *res)
-{
- if ((res->type != RDMA_RESTRACK_CM_ID) &&
- (res->type != RDMA_RESTRACK_COUNTER))
- res->task = NULL;
-
- if (!res->task)
- rdma_restrack_set_task(res, NULL);
- res->kern_name = NULL;
-
- res->user = true;
- rdma_restrack_add(res);
-}
-EXPORT_SYMBOL(rdma_restrack_uadd);
-
int __must_check rdma_restrack_get(struct rdma_restrack_entry *res)
{
return kref_get_unless_zero(&res->kref);
diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
index d35c4c41d2ff..49c1d84cca2d 100644
--- a/drivers/infiniband/core/restrack.h
+++ b/drivers/infiniband/core/restrack.h
@@ -29,6 +29,8 @@ void rdma_restrack_add(struct rdma_restrack_entry *res);
void rdma_restrack_del(struct rdma_restrack_entry *res);
void rdma_restrack_new(struct rdma_restrack_entry *res,
enum rdma_restrack_type type);
-void rdma_restrack_attach_task(struct rdma_restrack_entry *res,
- struct task_struct *task);
+void rdma_restrack_set_name(struct rdma_restrack_entry *res,
+ const char *caller);
+void rdma_restrack_parent_name(struct rdma_restrack_entry *dst,
+ struct rdma_restrack_entry *parent);
#endif /* _RDMA_CORE_RESTRACK_H_ */
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 5fbf05ce7ccb..5dd3d72d594d 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -223,6 +223,7 @@ int ib_alloc_ucontext(struct uverbs_attr_bundle *attrs)
xa_init_flags(&ucontext->mmap_xa, XA_FLAGS_ALLOC);
rdma_restrack_new(&ucontext->res, RDMA_RESTRACK_CTX);
+ rdma_restrack_set_name(&ucontext->res, NULL);
attrs->context = ucontext;
return 0;
}
@@ -251,7 +252,7 @@ int ib_init_ucontext(struct uverbs_attr_bundle *attrs)
if (ret)
goto err_uncharge;
- rdma_restrack_uadd(&ucontext->res);
+ rdma_restrack_add(&ucontext->res);
/*
* Make sure that ib_uverbs_get_ucontext() sees the pointer update
@@ -443,10 +444,12 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
atomic_set(&pd->usecnt, 0);
rdma_restrack_new(&pd->res, RDMA_RESTRACK_PD);
+ rdma_restrack_set_name(&pd->res, NULL);
+
ret = ib_dev->ops.alloc_pd(pd, &attrs->driver_udata);
if (ret)
goto err_alloc;
- rdma_restrack_uadd(&pd->res);
+ rdma_restrack_add(&pd->res);
uobj->object = pd;
uobj_finalize_uobj_create(uobj, attrs);
@@ -748,7 +751,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
mr->iova = cmd.hca_va;
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_uadd(&mr->res);
+ rdma_restrack_set_name(&mr->res, NULL);
+ rdma_restrack_add(&mr->res);
uobj->object = mr;
uobj_put_obj_read(pd);
@@ -1000,10 +1004,12 @@ static int create_cq(struct uverbs_attr_bundle *attrs,
atomic_set(&cq->usecnt, 0);
rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
+ rdma_restrack_set_name(&cq->res, NULL);
+
ret = ib_dev->ops.create_cq(cq, &attr, &attrs->driver_udata);
if (ret)
goto err_free;
- rdma_restrack_uadd(&cq->res);
+ rdma_restrack_add(&cq->res);
obj->uevent.uobject.object = cq;
obj->uevent.event_file = READ_ONCE(attrs->ufile->default_async_file);
diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
index 3a5fd6c9ba72..8dabd05988b2 100644
--- a/drivers/infiniband/core/uverbs_std_types_cq.c
+++ b/drivers/infiniband/core/uverbs_std_types_cq.c
@@ -126,13 +126,15 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
atomic_set(&cq->usecnt, 0);
rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
+ rdma_restrack_set_name(&cq->res, NULL);
+
ret = ib_dev->ops.create_cq(cq, &attr, &attrs->driver_udata);
if (ret)
goto err_free;
obj->uevent.uobject.object = cq;
obj->uevent.uobject.user_handle = user_handle;
- rdma_restrack_uadd(&cq->res);
+ rdma_restrack_add(&cq->res);
uverbs_finalize_uobj_create(attrs, UVERBS_ATTR_CREATE_CQ_HANDLE);
ret = uverbs_copy_to(attrs, UVERBS_ATTR_CREATE_CQ_RESP_CQE, &cq->cqe,
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 9c4dd59b9cf9..c1ab7d8eced6 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -273,7 +273,7 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
pd->flags = flags;
rdma_restrack_new(&pd->res, RDMA_RESTRACK_PD);
- rdma_restrack_set_task(&pd->res, caller);
+ rdma_restrack_set_name(&pd->res, caller);
ret = device->ops.alloc_pd(pd, NULL);
if (ret) {
@@ -1999,7 +1999,7 @@ struct ib_cq *__ib_create_cq(struct ib_device *device,
atomic_set(&cq->usecnt, 0);
rdma_restrack_new(&cq->res, RDMA_RESTRACK_CQ);
- rdma_restrack_set_task(&cq->res, caller);
+ rdma_restrack_set_name(&cq->res, caller);
ret = device->ops.create_cq(cq, cq_attr, NULL);
if (ret) {
@@ -2081,7 +2081,7 @@ struct ib_mr *ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
atomic_inc(&pd->usecnt);
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_set_task(&mr->res, pd->res.kern_name);
+ rdma_restrack_parent_name(&mr->res, &pd->res);
rdma_restrack_add(&mr->res);
return mr;
@@ -2165,7 +2165,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
mr->sig_attrs = NULL;
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_set_task(&mr->res, pd->res.kern_name);
+ rdma_restrack_parent_name(&mr->res, &pd->res);
rdma_restrack_add(&mr->res);
out:
trace_mr_alloc(pd, mr_type, max_num_sg, mr);
@@ -2226,7 +2226,7 @@ struct ib_mr *ib_alloc_mr_integrity(struct ib_pd *pd,
mr->sig_attrs = sig_attrs;
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
- rdma_restrack_set_task(&mr->res, pd->res.kern_name);
+ rdma_restrack_parent_name(&mr->res, &pd->res);
rdma_restrack_add(&mr->res);
out:
trace_mr_integ_alloc(pd, max_num_data_sg, max_num_meta_sg, mr);
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index db59e208f5e8..10bfed0fcd32 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -106,9 +106,6 @@ struct rdma_restrack_entry {
int rdma_restrack_count(struct ib_device *dev,
enum rdma_restrack_type type);
-
-void rdma_restrack_uadd(struct rdma_restrack_entry *res);
-
/**
* rdma_is_kernel_res() - check the owner of resource
* @res: resource entry
@@ -130,14 +127,6 @@ int __must_check rdma_restrack_get(struct rdma_restrack_entry *res);
*/
int rdma_restrack_put(struct rdma_restrack_entry *res);
-/**
- * rdma_restrack_set_task() - set the task for this resource
- * @res: resource entry
- * @caller: kernel name, the current task will be used if the caller is NULL.
- */
-void rdma_restrack_set_task(struct rdma_restrack_entry *res,
- const char *caller);
-
/*
* Helper functions for rdma drivers when filling out
* nldev driver attributes.
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 06/14] RDMA/restrack: Improve readability in task name management
2020-09-07 12:21 ` [PATCH rdma-next v2 06/14] RDMA/restrack: Improve readability in task name management Leon Romanovsky
@ 2020-09-18 23:17 ` Jason Gunthorpe
2020-09-19 8:42 ` Leon Romanovsky
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-18 23:17 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma
On Mon, Sep 07, 2020 at 03:21:48PM +0300, Leon Romanovsky wrote:
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index feed3c04979a..3fc3c821743d 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -467,10 +467,13 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> id_priv->id.route.addr.dev_addr.transport =
> rdma_node_get_transport(cma_dev->device->node_type);
> list_add_tail(&id_priv->list, &cma_dev->id_list);
> - if (id_priv->res.kern_name)
> - rdma_restrack_add(&id_priv->res);
> - else
> - rdma_restrack_uadd(&id_priv->res);
> + /*
> + * For example UCMA doesn't set kern_name and below function will
> + * attach to "current" task.
> + */
> + rdma_restrack_set_name(&id_priv->res, id_priv->res.kern_name);
> + rdma_restrack_add(&id_priv->res);
I don't understand why the set_name was added here, looks wrong. For
the non-null case we either already have a task set because
rdma_create_id did it, or this is spawned from a listening_id and this
is WQ, so no reason to capture a WQ as the task.
I suppose the idea is that the rdma_restrack_set_name() in
rdma_accept() fixes this, but that isn't allowed either, there is no
locking so calling rdma_restrack_set_name() after rdma_restrack_add()
can't be done.
Without adding a bunch of locking someplace I think everything must
flow from the orignial rdma_cm_listen which was created in a
reasonable context, ie instead of passing name around the parent
restrack should be passed.
I came up with something like this
Can you put this and the patches here in a series please, up to this
patch all looks OK otherwise.
Jason
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index aecc60a5f8c3fe..b123811f33234a 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -457,7 +457,6 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
* For example UCMA doesn't set kern_name and below function will
* attach to "current" task.
*/
- rdma_restrack_set_name(&id_priv->res, id_priv->res.kern_name);
rdma_restrack_add(&id_priv->res);
trace_cm_id_attach(id_priv, cma_dev->device);
@@ -825,10 +824,10 @@ static void cma_id_put(struct rdma_id_private *id_priv)
complete(&id_priv->comp);
}
-struct rdma_cm_id *__rdma_create_id(struct net *net,
- rdma_cm_event_handler event_handler,
- void *context, enum rdma_ucm_port_space ps,
- enum ib_qp_type qp_type, const char *caller)
+static struct rdma_id_private *
+__rdma_create_id(struct net *net, rdma_cm_event_handler event_handler,
+ void *context, enum rdma_ucm_port_space ps,
+ enum ib_qp_type qp_type, const struct rdma_id_private *parent)
{
struct rdma_id_private *id_priv;
@@ -856,11 +855,44 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
id_priv->seq_num &= 0x00ffffff;
rdma_restrack_new(&id_priv->res, RDMA_RESTRACK_CM_ID);
- rdma_restrack_set_name(&id_priv->res, caller);
+ if (parent)
+ rdma_restrack_parent_name(&id_priv->res, &parent->res);
- return &id_priv->id;
+ return id_priv;
+}
+
+struct rdma_cm_id *
+__rdma_create_kernel_id(struct net *net, rdma_cm_event_handler event_handler,
+ void *context, enum rdma_ucm_port_space ps,
+ enum ib_qp_type qp_type, const char *caller)
+{
+ struct rdma_id_private *ret;
+
+ ret = __rdma_create_id(net, event_handler, context, ps, qp_type, NULL);
+ if (IS_ERR(ret))
+ return ERR_CAST(ret);
+
+ rdma_restrack_set_name(&ret->res, caller);
+ return &ret->id;
+}
+EXPORT_SYMBOL(__rdma_create_kernel_id);
+
+struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
+ void *context,
+ enum rdma_ucm_port_space ps,
+ enum ib_qp_type qp_type)
+{
+ struct rdma_id_private *ret;
+
+ ret = __rdma_create_id(current->nsproxy->net_ns, event_handler, context,
+ ps, qp_type, NULL);
+ if (IS_ERR(ret))
+ return ERR_CAST(ret);
+
+ rdma_restrack_set_name(&ret->res, NULL);
+ return &ret->id;
}
-EXPORT_SYMBOL(__rdma_create_id);
+EXPORT_SYMBOL(rdma_create_user_id);
static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
{
@@ -2032,14 +2064,15 @@ cma_ib_new_conn_id(const struct rdma_cm_id *listen_id,
int ret;
listen_id_priv = container_of(listen_id, struct rdma_id_private, id);
- id = __rdma_create_id(listen_id->route.addr.dev_addr.net,
- listen_id->event_handler, listen_id->context,
- listen_id->ps, ib_event->param.req_rcvd.qp_type,
- listen_id_priv->res.kern_name);
- if (IS_ERR(id))
+ id_priv = __rdma_create_id(listen_id->route.addr.dev_addr.net,
+ listen_id->event_handler, listen_id->context,
+ listen_id->ps,
+ ib_event->param.req_rcvd.qp_type,
+ listen_id_priv);
+ if (IS_ERR(id_priv))
return NULL;
- id_priv = container_of(id, struct rdma_id_private, id);
+ id = &id_priv->id;
if (cma_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
(struct sockaddr *)&id->route.addr.dst_addr,
listen_id, ib_event, ss_family, service_id))
@@ -2093,13 +2126,13 @@ cma_ib_new_udp_id(const struct rdma_cm_id *listen_id,
int ret;
listen_id_priv = container_of(listen_id, struct rdma_id_private, id);
- id = __rdma_create_id(net, listen_id->event_handler, listen_id->context,
- listen_id->ps, IB_QPT_UD,
- listen_id_priv->res.kern_name);
- if (IS_ERR(id))
+ id_priv = __rdma_create_id(net, listen_id->event_handler,
+ listen_id->context, listen_id->ps, IB_QPT_UD,
+ listen_id_priv);
+ if (IS_ERR(id_priv))
return NULL;
- id_priv = container_of(id, struct rdma_id_private, id);
+ id = &id_priv->id;
if (cma_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
(struct sockaddr *)&id->route.addr.dst_addr,
listen_id, ib_event, ss_family,
@@ -2335,7 +2368,6 @@ static int cma_iw_handler(struct iw_cm_id *iw_id, struct iw_cm_event *iw_event)
static int iw_conn_req_handler(struct iw_cm_id *cm_id,
struct iw_cm_event *iw_event)
{
- struct rdma_cm_id *new_cm_id;
struct rdma_id_private *listen_id, *conn_id;
struct rdma_cm_event event = {};
int ret = -ECONNABORTED;
@@ -2355,16 +2387,14 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
goto out;
/* Create a new RDMA id for the new IW CM ID */
- new_cm_id = __rdma_create_id(listen_id->id.route.addr.dev_addr.net,
- listen_id->id.event_handler,
- listen_id->id.context,
- RDMA_PS_TCP, IB_QPT_RC,
- listen_id->res.kern_name);
- if (IS_ERR(new_cm_id)) {
+ conn_id = __rdma_create_id(listen_id->id.route.addr.dev_addr.net,
+ listen_id->id.event_handler,
+ listen_id->id.context, RDMA_PS_TCP,
+ IB_QPT_RC, listen_id);
+ if (IS_ERR(conn_id)) {
ret = -ENOMEM;
goto out;
}
- conn_id = container_of(new_cm_id, struct rdma_id_private, id);
mutex_lock_nested(&conn_id->handler_mutex, SINGLE_DEPTH_NESTING);
conn_id->state = RDMA_CM_CONNECT;
@@ -2469,7 +2499,6 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
struct cma_device *cma_dev)
{
struct rdma_id_private *dev_id_priv;
- struct rdma_cm_id *id;
struct net *net = id_priv->id.route.addr.dev_addr.net;
int ret;
@@ -2478,13 +2507,12 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
if (cma_family(id_priv) == AF_IB && !rdma_cap_ib_cm(cma_dev->device, 1))
return;
- id = __rdma_create_id(net, cma_listen_handler, id_priv, id_priv->id.ps,
- id_priv->id.qp_type, id_priv->res.kern_name);
- if (IS_ERR(id))
+ dev_id_priv =
+ __rdma_create_id(net, cma_listen_handler, id_priv,
+ id_priv->id.ps, id_priv->id.qp_type, id_priv);
+ if (IS_ERR(dev_id_priv))
return;
- dev_id_priv = container_of(id, struct rdma_id_private, id);
-
dev_id_priv->state = RDMA_CM_ADDR_BOUND;
memcpy(cma_src_addr(dev_id_priv), cma_src_addr(id_priv),
rdma_addr_size(cma_src_addr(id_priv)));
@@ -2497,7 +2525,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
dev_id_priv->tos_set = id_priv->tos_set;
dev_id_priv->tos = id_priv->tos;
- ret = rdma_listen(id, id_priv->backlog);
+ ret = rdma_listen(&dev_id_priv->id, id_priv->backlog);
if (ret)
dev_warn(&cma_dev->device->dev,
"RDMA CMA: cma_listen_on_dev, error %d\n", ret);
@@ -4152,8 +4180,25 @@ static int cma_send_sidr_rep(struct rdma_id_private *id_priv,
return ib_send_cm_sidr_rep(id_priv->cm_id.ib, &rep);
}
-int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
- const char *caller)
+/**
+ * rdma_accept - Called to accept a connection request or response.
+ * @id: Connection identifier associated with the request.
+ * @conn_param: Information needed to establish the connection. This must be
+ * provided if accepting a connection request. If accepting a connection
+ * response, this parameter must be NULL.
+ *
+ * Typically, this routine is only called by the listener to accept a connection
+ * request. It must also be called on the active side of a connection if the
+ * user is performing their own QP transitions.
+ *
+ * In the case of error, a reject message is sent to the remote side and the
+ * state of the qp associated with the id is modified to error, such that any
+ * previously posted receive buffers would be flushed.
+ *
+ * This function is for use by kernel ULPs and must be called from under the
+ * handler callback.
+ */
+int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)
{
struct rdma_id_private *id_priv =
container_of(id, struct rdma_id_private, id);
@@ -4161,8 +4206,6 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
lockdep_assert_held(&id_priv->handler_mutex);
- rdma_restrack_set_name(&id_priv->res, caller);
-
if (READ_ONCE(id_priv->state) != RDMA_CM_CONNECT)
return -EINVAL;
@@ -4201,10 +4244,10 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
rdma_reject(id, NULL, 0, IB_CM_REJ_CONSUMER_DEFINED);
return ret;
}
-EXPORT_SYMBOL(__rdma_accept);
+EXPORT_SYMBOL(rdma_accept);
-int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
- const char *caller, struct rdma_ucm_ece *ece)
+int rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
+ struct rdma_ucm_ece *ece)
{
struct rdma_id_private *id_priv =
container_of(id, struct rdma_id_private, id);
@@ -4212,9 +4255,9 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
id_priv->ece.vendor_id = ece->vendor_id;
id_priv->ece.attr_mod = ece->attr_mod;
- return __rdma_accept(id, conn_param, caller);
+ return rdma_accept(id, conn_param);
}
-EXPORT_SYMBOL(__rdma_accept_ece);
+EXPORT_SYMBOL(rdma_accept_ece);
void rdma_lock_handler(struct rdma_cm_id *id)
{
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 0c67acf2169d69..4aeeaaed0f17dd 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -189,7 +189,7 @@ EXPORT_SYMBOL(rdma_restrack_set_name);
* @parent: parent resource entry
*/
void rdma_restrack_parent_name(struct rdma_restrack_entry *dst,
- struct rdma_restrack_entry *parent)
+ const struct rdma_restrack_entry *parent)
{
if (rdma_is_kernel_res(parent))
dst->kern_name = parent->kern_name;
diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
index 49c1d84cca2da4..6a04fc41f73801 100644
--- a/drivers/infiniband/core/restrack.h
+++ b/drivers/infiniband/core/restrack.h
@@ -32,5 +32,5 @@ void rdma_restrack_new(struct rdma_restrack_entry *res,
void rdma_restrack_set_name(struct rdma_restrack_entry *res,
const char *caller);
void rdma_restrack_parent_name(struct rdma_restrack_entry *dst,
- struct rdma_restrack_entry *parent);
+ const struct rdma_restrack_entry *parent);
#endif /* _RDMA_CORE_RESTRACK_H_ */
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index a5595bd489b089..2901847a01021a 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -456,8 +456,7 @@ static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf,
return -ENOMEM;
ctx->uid = cmd.uid;
- cm_id = __rdma_create_id(current->nsproxy->net_ns,
- ucma_event_handler, ctx, cmd.ps, qp_type, NULL);
+ cm_id = rdma_create_user_id(ucma_event_handler, ctx, cmd.ps, qp_type);
if (IS_ERR(cm_id)) {
ret = PTR_ERR(cm_id);
goto err1;
@@ -1126,7 +1125,7 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
mutex_lock(&ctx->mutex);
rdma_lock_handler(ctx->cm_id);
- ret = __rdma_accept_ece(ctx->cm_id, &conn_param, NULL, &ece);
+ ret = rdma_accept_ece(ctx->cm_id, &conn_param, &ece);
if (!ret) {
/* The uid must be set atomically with the handler */
ctx->uid = cmd.uid;
@@ -1136,7 +1135,7 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
} else {
mutex_lock(&ctx->mutex);
rdma_lock_handler(ctx->cm_id);
- ret = __rdma_accept_ece(ctx->cm_id, NULL, NULL, &ece);
+ ret = rdma_accept_ece(ctx->cm_id, NULL, &ece);
rdma_unlock_handler(ctx->cm_id);
mutex_unlock(&ctx->mutex);
}
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index c1334c9a7aa858..c672ae1da26bb5 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -110,11 +110,14 @@ struct rdma_cm_id {
u8 port_num;
};
-struct rdma_cm_id *__rdma_create_id(struct net *net,
- rdma_cm_event_handler event_handler,
- void *context, enum rdma_ucm_port_space ps,
- enum ib_qp_type qp_type,
- const char *caller);
+struct rdma_cm_id *
+__rdma_create_kernel_id(struct net *net, rdma_cm_event_handler event_handler,
+ void *context, enum rdma_ucm_port_space ps,
+ enum ib_qp_type qp_type, const char *caller);
+struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
+ void *context,
+ enum rdma_ucm_port_space ps,
+ enum ib_qp_type qp_type);
/**
* rdma_create_id - Create an RDMA identifier.
@@ -132,9 +135,9 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
* The event handler callback serializes on the id's mutex and is
* allowed to sleep.
*/
-#define rdma_create_id(net, event_handler, context, ps, qp_type) \
- __rdma_create_id((net), (event_handler), (context), (ps), (qp_type), \
- KBUILD_MODNAME)
+#define rdma_create_id(net, event_handler, context, ps, qp_type) \
+ __rdma_create_kernel_id(net, event_handler, context, ps, qp_type, \
+ KBUILD_MODNAME)
/**
* rdma_destroy_id - Destroys an RDMA identifier.
@@ -250,34 +253,12 @@ int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
*/
int rdma_listen(struct rdma_cm_id *id, int backlog);
-int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
- const char *caller);
+int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
void rdma_lock_handler(struct rdma_cm_id *id);
void rdma_unlock_handler(struct rdma_cm_id *id);
-int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
- const char *caller, struct rdma_ucm_ece *ece);
-
-/**
- * rdma_accept - Called to accept a connection request or response.
- * @id: Connection identifier associated with the request.
- * @conn_param: Information needed to establish the connection. This must be
- * provided if accepting a connection request. If accepting a connection
- * response, this parameter must be NULL.
- *
- * Typically, this routine is only called by the listener to accept a connection
- * request. It must also be called on the active side of a connection if the
- * user is performing their own QP transitions.
- *
- * In the case of error, a reject message is sent to the remote side and the
- * state of the qp associated with the id is modified to error, such that any
- * previously posted receive buffers would be flushed.
- *
- * This function is for use by kernel ULPs and must be called from under the
- * handler callback.
- */
-#define rdma_accept(id, conn_param) \
- __rdma_accept((id), (conn_param), KBUILD_MODNAME)
+int rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
+ struct rdma_ucm_ece *ece);
/**
* rdma_notify - Notifies the RDMA CM of an asynchronous event that has
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index 10bfed0fcd3262..d3a1cc5be7bcef 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -110,7 +110,7 @@ int rdma_restrack_count(struct ib_device *dev,
* rdma_is_kernel_res() - check the owner of resource
* @res: resource entry
*/
-static inline bool rdma_is_kernel_res(struct rdma_restrack_entry *res)
+static inline bool rdma_is_kernel_res(const struct rdma_restrack_entry *res)
{
return !res->user;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 06/14] RDMA/restrack: Improve readability in task name management
2020-09-18 23:17 ` Jason Gunthorpe
@ 2020-09-19 8:42 ` Leon Romanovsky
0 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-19 8:42 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma
On Fri, Sep 18, 2020 at 08:17:21PM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:48PM +0300, Leon Romanovsky wrote:
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index feed3c04979a..3fc3c821743d 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -467,10 +467,13 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > id_priv->id.route.addr.dev_addr.transport =
> > rdma_node_get_transport(cma_dev->device->node_type);
> > list_add_tail(&id_priv->list, &cma_dev->id_list);
> > - if (id_priv->res.kern_name)
> > - rdma_restrack_add(&id_priv->res);
> > - else
> > - rdma_restrack_uadd(&id_priv->res);
> > + /*
> > + * For example UCMA doesn't set kern_name and below function will
> > + * attach to "current" task.
> > + */
> > + rdma_restrack_set_name(&id_priv->res, id_priv->res.kern_name);
> > + rdma_restrack_add(&id_priv->res);
>
> I don't understand why the set_name was added here, looks wrong. For
> the non-null case we either already have a task set because
> rdma_create_id did it, or this is spawned from a listening_id and this
> is WQ, so no reason to capture a WQ as the task.
>
> I suppose the idea is that the rdma_restrack_set_name() in
> rdma_accept() fixes this, but that isn't allowed either, there is no
> locking so calling rdma_restrack_set_name() after rdma_restrack_add()
> can't be done.
>
> Without adding a bunch of locking someplace I think everything must
> flow from the orignial rdma_cm_listen which was created in a
> reasonable context, ie instead of passing name around the parent
> restrack should be passed.
>
> I came up with something like this
>
> Can you put this and the patches here in a series please, up to this
> patch all looks OK otherwise.
It is better than before and will try it.
Thanks
>
> Jason
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index aecc60a5f8c3fe..b123811f33234a 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -457,7 +457,6 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> * For example UCMA doesn't set kern_name and below function will
> * attach to "current" task.
> */
> - rdma_restrack_set_name(&id_priv->res, id_priv->res.kern_name);
> rdma_restrack_add(&id_priv->res);
>
> trace_cm_id_attach(id_priv, cma_dev->device);
> @@ -825,10 +824,10 @@ static void cma_id_put(struct rdma_id_private *id_priv)
> complete(&id_priv->comp);
> }
>
> -struct rdma_cm_id *__rdma_create_id(struct net *net,
> - rdma_cm_event_handler event_handler,
> - void *context, enum rdma_ucm_port_space ps,
> - enum ib_qp_type qp_type, const char *caller)
> +static struct rdma_id_private *
> +__rdma_create_id(struct net *net, rdma_cm_event_handler event_handler,
> + void *context, enum rdma_ucm_port_space ps,
> + enum ib_qp_type qp_type, const struct rdma_id_private *parent)
> {
> struct rdma_id_private *id_priv;
>
> @@ -856,11 +855,44 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
> id_priv->seq_num &= 0x00ffffff;
>
> rdma_restrack_new(&id_priv->res, RDMA_RESTRACK_CM_ID);
> - rdma_restrack_set_name(&id_priv->res, caller);
> + if (parent)
> + rdma_restrack_parent_name(&id_priv->res, &parent->res);
>
> - return &id_priv->id;
> + return id_priv;
> +}
> +
> +struct rdma_cm_id *
> +__rdma_create_kernel_id(struct net *net, rdma_cm_event_handler event_handler,
> + void *context, enum rdma_ucm_port_space ps,
> + enum ib_qp_type qp_type, const char *caller)
> +{
> + struct rdma_id_private *ret;
> +
> + ret = __rdma_create_id(net, event_handler, context, ps, qp_type, NULL);
> + if (IS_ERR(ret))
> + return ERR_CAST(ret);
> +
> + rdma_restrack_set_name(&ret->res, caller);
> + return &ret->id;
> +}
> +EXPORT_SYMBOL(__rdma_create_kernel_id);
> +
> +struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
> + void *context,
> + enum rdma_ucm_port_space ps,
> + enum ib_qp_type qp_type)
> +{
> + struct rdma_id_private *ret;
> +
> + ret = __rdma_create_id(current->nsproxy->net_ns, event_handler, context,
> + ps, qp_type, NULL);
> + if (IS_ERR(ret))
> + return ERR_CAST(ret);
> +
> + rdma_restrack_set_name(&ret->res, NULL);
> + return &ret->id;
> }
> -EXPORT_SYMBOL(__rdma_create_id);
> +EXPORT_SYMBOL(rdma_create_user_id);
>
> static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp)
> {
> @@ -2032,14 +2064,15 @@ cma_ib_new_conn_id(const struct rdma_cm_id *listen_id,
> int ret;
>
> listen_id_priv = container_of(listen_id, struct rdma_id_private, id);
> - id = __rdma_create_id(listen_id->route.addr.dev_addr.net,
> - listen_id->event_handler, listen_id->context,
> - listen_id->ps, ib_event->param.req_rcvd.qp_type,
> - listen_id_priv->res.kern_name);
> - if (IS_ERR(id))
> + id_priv = __rdma_create_id(listen_id->route.addr.dev_addr.net,
> + listen_id->event_handler, listen_id->context,
> + listen_id->ps,
> + ib_event->param.req_rcvd.qp_type,
> + listen_id_priv);
> + if (IS_ERR(id_priv))
> return NULL;
>
> - id_priv = container_of(id, struct rdma_id_private, id);
> + id = &id_priv->id;
> if (cma_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
> (struct sockaddr *)&id->route.addr.dst_addr,
> listen_id, ib_event, ss_family, service_id))
> @@ -2093,13 +2126,13 @@ cma_ib_new_udp_id(const struct rdma_cm_id *listen_id,
> int ret;
>
> listen_id_priv = container_of(listen_id, struct rdma_id_private, id);
> - id = __rdma_create_id(net, listen_id->event_handler, listen_id->context,
> - listen_id->ps, IB_QPT_UD,
> - listen_id_priv->res.kern_name);
> - if (IS_ERR(id))
> + id_priv = __rdma_create_id(net, listen_id->event_handler,
> + listen_id->context, listen_id->ps, IB_QPT_UD,
> + listen_id_priv);
> + if (IS_ERR(id_priv))
> return NULL;
>
> - id_priv = container_of(id, struct rdma_id_private, id);
> + id = &id_priv->id;
> if (cma_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
> (struct sockaddr *)&id->route.addr.dst_addr,
> listen_id, ib_event, ss_family,
> @@ -2335,7 +2368,6 @@ static int cma_iw_handler(struct iw_cm_id *iw_id, struct iw_cm_event *iw_event)
> static int iw_conn_req_handler(struct iw_cm_id *cm_id,
> struct iw_cm_event *iw_event)
> {
> - struct rdma_cm_id *new_cm_id;
> struct rdma_id_private *listen_id, *conn_id;
> struct rdma_cm_event event = {};
> int ret = -ECONNABORTED;
> @@ -2355,16 +2387,14 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
> goto out;
>
> /* Create a new RDMA id for the new IW CM ID */
> - new_cm_id = __rdma_create_id(listen_id->id.route.addr.dev_addr.net,
> - listen_id->id.event_handler,
> - listen_id->id.context,
> - RDMA_PS_TCP, IB_QPT_RC,
> - listen_id->res.kern_name);
> - if (IS_ERR(new_cm_id)) {
> + conn_id = __rdma_create_id(listen_id->id.route.addr.dev_addr.net,
> + listen_id->id.event_handler,
> + listen_id->id.context, RDMA_PS_TCP,
> + IB_QPT_RC, listen_id);
> + if (IS_ERR(conn_id)) {
> ret = -ENOMEM;
> goto out;
> }
> - conn_id = container_of(new_cm_id, struct rdma_id_private, id);
> mutex_lock_nested(&conn_id->handler_mutex, SINGLE_DEPTH_NESTING);
> conn_id->state = RDMA_CM_CONNECT;
>
> @@ -2469,7 +2499,6 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> struct cma_device *cma_dev)
> {
> struct rdma_id_private *dev_id_priv;
> - struct rdma_cm_id *id;
> struct net *net = id_priv->id.route.addr.dev_addr.net;
> int ret;
>
> @@ -2478,13 +2507,12 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> if (cma_family(id_priv) == AF_IB && !rdma_cap_ib_cm(cma_dev->device, 1))
> return;
>
> - id = __rdma_create_id(net, cma_listen_handler, id_priv, id_priv->id.ps,
> - id_priv->id.qp_type, id_priv->res.kern_name);
> - if (IS_ERR(id))
> + dev_id_priv =
> + __rdma_create_id(net, cma_listen_handler, id_priv,
> + id_priv->id.ps, id_priv->id.qp_type, id_priv);
> + if (IS_ERR(dev_id_priv))
> return;
>
> - dev_id_priv = container_of(id, struct rdma_id_private, id);
> -
> dev_id_priv->state = RDMA_CM_ADDR_BOUND;
> memcpy(cma_src_addr(dev_id_priv), cma_src_addr(id_priv),
> rdma_addr_size(cma_src_addr(id_priv)));
> @@ -2497,7 +2525,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> dev_id_priv->tos_set = id_priv->tos_set;
> dev_id_priv->tos = id_priv->tos;
>
> - ret = rdma_listen(id, id_priv->backlog);
> + ret = rdma_listen(&dev_id_priv->id, id_priv->backlog);
> if (ret)
> dev_warn(&cma_dev->device->dev,
> "RDMA CMA: cma_listen_on_dev, error %d\n", ret);
> @@ -4152,8 +4180,25 @@ static int cma_send_sidr_rep(struct rdma_id_private *id_priv,
> return ib_send_cm_sidr_rep(id_priv->cm_id.ib, &rep);
> }
>
> -int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> - const char *caller)
> +/**
> + * rdma_accept - Called to accept a connection request or response.
> + * @id: Connection identifier associated with the request.
> + * @conn_param: Information needed to establish the connection. This must be
> + * provided if accepting a connection request. If accepting a connection
> + * response, this parameter must be NULL.
> + *
> + * Typically, this routine is only called by the listener to accept a connection
> + * request. It must also be called on the active side of a connection if the
> + * user is performing their own QP transitions.
> + *
> + * In the case of error, a reject message is sent to the remote side and the
> + * state of the qp associated with the id is modified to error, such that any
> + * previously posted receive buffers would be flushed.
> + *
> + * This function is for use by kernel ULPs and must be called from under the
> + * handler callback.
> + */
> +int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)
> {
> struct rdma_id_private *id_priv =
> container_of(id, struct rdma_id_private, id);
> @@ -4161,8 +4206,6 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
>
> lockdep_assert_held(&id_priv->handler_mutex);
>
> - rdma_restrack_set_name(&id_priv->res, caller);
> -
> if (READ_ONCE(id_priv->state) != RDMA_CM_CONNECT)
> return -EINVAL;
>
> @@ -4201,10 +4244,10 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> rdma_reject(id, NULL, 0, IB_CM_REJ_CONSUMER_DEFINED);
> return ret;
> }
> -EXPORT_SYMBOL(__rdma_accept);
> +EXPORT_SYMBOL(rdma_accept);
>
> -int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> - const char *caller, struct rdma_ucm_ece *ece)
> +int rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> + struct rdma_ucm_ece *ece)
> {
> struct rdma_id_private *id_priv =
> container_of(id, struct rdma_id_private, id);
> @@ -4212,9 +4255,9 @@ int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> id_priv->ece.vendor_id = ece->vendor_id;
> id_priv->ece.attr_mod = ece->attr_mod;
>
> - return __rdma_accept(id, conn_param, caller);
> + return rdma_accept(id, conn_param);
> }
> -EXPORT_SYMBOL(__rdma_accept_ece);
> +EXPORT_SYMBOL(rdma_accept_ece);
>
> void rdma_lock_handler(struct rdma_cm_id *id)
> {
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index 0c67acf2169d69..4aeeaaed0f17dd 100644
> --- a/drivers/infiniband/core/restrack.c
> +++ b/drivers/infiniband/core/restrack.c
> @@ -189,7 +189,7 @@ EXPORT_SYMBOL(rdma_restrack_set_name);
> * @parent: parent resource entry
> */
> void rdma_restrack_parent_name(struct rdma_restrack_entry *dst,
> - struct rdma_restrack_entry *parent)
> + const struct rdma_restrack_entry *parent)
> {
> if (rdma_is_kernel_res(parent))
> dst->kern_name = parent->kern_name;
> diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
> index 49c1d84cca2da4..6a04fc41f73801 100644
> --- a/drivers/infiniband/core/restrack.h
> +++ b/drivers/infiniband/core/restrack.h
> @@ -32,5 +32,5 @@ void rdma_restrack_new(struct rdma_restrack_entry *res,
> void rdma_restrack_set_name(struct rdma_restrack_entry *res,
> const char *caller);
> void rdma_restrack_parent_name(struct rdma_restrack_entry *dst,
> - struct rdma_restrack_entry *parent);
> + const struct rdma_restrack_entry *parent);
> #endif /* _RDMA_CORE_RESTRACK_H_ */
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index a5595bd489b089..2901847a01021a 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -456,8 +456,7 @@ static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf,
> return -ENOMEM;
>
> ctx->uid = cmd.uid;
> - cm_id = __rdma_create_id(current->nsproxy->net_ns,
> - ucma_event_handler, ctx, cmd.ps, qp_type, NULL);
> + cm_id = rdma_create_user_id(ucma_event_handler, ctx, cmd.ps, qp_type);
> if (IS_ERR(cm_id)) {
> ret = PTR_ERR(cm_id);
> goto err1;
> @@ -1126,7 +1125,7 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
> ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
> mutex_lock(&ctx->mutex);
> rdma_lock_handler(ctx->cm_id);
> - ret = __rdma_accept_ece(ctx->cm_id, &conn_param, NULL, &ece);
> + ret = rdma_accept_ece(ctx->cm_id, &conn_param, &ece);
> if (!ret) {
> /* The uid must be set atomically with the handler */
> ctx->uid = cmd.uid;
> @@ -1136,7 +1135,7 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
> } else {
> mutex_lock(&ctx->mutex);
> rdma_lock_handler(ctx->cm_id);
> - ret = __rdma_accept_ece(ctx->cm_id, NULL, NULL, &ece);
> + ret = rdma_accept_ece(ctx->cm_id, NULL, &ece);
> rdma_unlock_handler(ctx->cm_id);
> mutex_unlock(&ctx->mutex);
> }
> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> index c1334c9a7aa858..c672ae1da26bb5 100644
> --- a/include/rdma/rdma_cm.h
> +++ b/include/rdma/rdma_cm.h
> @@ -110,11 +110,14 @@ struct rdma_cm_id {
> u8 port_num;
> };
>
> -struct rdma_cm_id *__rdma_create_id(struct net *net,
> - rdma_cm_event_handler event_handler,
> - void *context, enum rdma_ucm_port_space ps,
> - enum ib_qp_type qp_type,
> - const char *caller);
> +struct rdma_cm_id *
> +__rdma_create_kernel_id(struct net *net, rdma_cm_event_handler event_handler,
> + void *context, enum rdma_ucm_port_space ps,
> + enum ib_qp_type qp_type, const char *caller);
> +struct rdma_cm_id *rdma_create_user_id(rdma_cm_event_handler event_handler,
> + void *context,
> + enum rdma_ucm_port_space ps,
> + enum ib_qp_type qp_type);
>
> /**
> * rdma_create_id - Create an RDMA identifier.
> @@ -132,9 +135,9 @@ struct rdma_cm_id *__rdma_create_id(struct net *net,
> * The event handler callback serializes on the id's mutex and is
> * allowed to sleep.
> */
> -#define rdma_create_id(net, event_handler, context, ps, qp_type) \
> - __rdma_create_id((net), (event_handler), (context), (ps), (qp_type), \
> - KBUILD_MODNAME)
> +#define rdma_create_id(net, event_handler, context, ps, qp_type) \
> + __rdma_create_kernel_id(net, event_handler, context, ps, qp_type, \
> + KBUILD_MODNAME)
>
> /**
> * rdma_destroy_id - Destroys an RDMA identifier.
> @@ -250,34 +253,12 @@ int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> */
> int rdma_listen(struct rdma_cm_id *id, int backlog);
>
> -int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> - const char *caller);
> +int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
>
> void rdma_lock_handler(struct rdma_cm_id *id);
> void rdma_unlock_handler(struct rdma_cm_id *id);
> -int __rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> - const char *caller, struct rdma_ucm_ece *ece);
> -
> -/**
> - * rdma_accept - Called to accept a connection request or response.
> - * @id: Connection identifier associated with the request.
> - * @conn_param: Information needed to establish the connection. This must be
> - * provided if accepting a connection request. If accepting a connection
> - * response, this parameter must be NULL.
> - *
> - * Typically, this routine is only called by the listener to accept a connection
> - * request. It must also be called on the active side of a connection if the
> - * user is performing their own QP transitions.
> - *
> - * In the case of error, a reject message is sent to the remote side and the
> - * state of the qp associated with the id is modified to error, such that any
> - * previously posted receive buffers would be flushed.
> - *
> - * This function is for use by kernel ULPs and must be called from under the
> - * handler callback.
> - */
> -#define rdma_accept(id, conn_param) \
> - __rdma_accept((id), (conn_param), KBUILD_MODNAME)
> +int rdma_accept_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,
> + struct rdma_ucm_ece *ece);
>
> /**
> * rdma_notify - Notifies the RDMA CM of an asynchronous event that has
> diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> index 10bfed0fcd3262..d3a1cc5be7bcef 100644
> --- a/include/rdma/restrack.h
> +++ b/include/rdma/restrack.h
> @@ -110,7 +110,7 @@ int rdma_restrack_count(struct ib_device *dev,
> * rdma_is_kernel_res() - check the owner of resource
> * @res: resource entry
> */
> -static inline bool rdma_is_kernel_res(struct rdma_restrack_entry *res)
> +static inline bool rdma_is_kernel_res(const struct rdma_restrack_entry *res)
> {
> return !res->user;
> }
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (5 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 06/14] RDMA/restrack: Improve readability in task name management Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-18 23:26 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 08/14] RDMA/core: Allow drivers to disable restrack DB Leon Romanovsky
` (6 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
The RDMA-CM code wasn't consistent in flows that attached to cma_dev,
this caused to situations where failure during attach to listen on such
device leave RDMA-CM in non-consistent state.
Update the listen/attach flow to correctly deal with failures.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cma.c | 197 ++++++++++++++++++++--------------
1 file changed, 114 insertions(+), 83 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 3fc3c821743d..ab1f8b707a5b 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -458,8 +458,8 @@ static int cma_igmp_send(struct net_device *ndev, union ib_gid *mgid, bool join)
return (in_dev) ? 0 : -ENODEV;
}
-static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
- struct cma_device *cma_dev)
+static int _cma_attach_to_dev(struct rdma_id_private *id_priv,
+ struct cma_device *cma_dev)
{
cma_dev_get(cma_dev);
id_priv->cma_dev = cma_dev;
@@ -475,15 +475,22 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
rdma_restrack_add(&id_priv->res);
trace_cm_id_attach(id_priv, cma_dev->device);
+ return 0;
}
-static void cma_attach_to_dev(struct rdma_id_private *id_priv,
- struct cma_device *cma_dev)
+static int cma_attach_to_dev(struct rdma_id_private *id_priv,
+ struct cma_device *cma_dev)
{
- _cma_attach_to_dev(id_priv, cma_dev);
+ int ret;
+
+ ret = _cma_attach_to_dev(id_priv, cma_dev);
+ if (ret)
+ return ret;
+
id_priv->gid_type =
cma_dev->default_gid_type[id_priv->id.port_num -
rdma_start_port(cma_dev->device)];
+ return 0;
}
static inline void release_mc(struct kref *kref)
@@ -656,8 +663,7 @@ static int cma_acquire_dev_by_src_ip(struct rdma_id_private *id_priv)
if (!IS_ERR(sgid_attr)) {
id_priv->id.port_num = port;
cma_bind_sgid_attr(id_priv, sgid_attr);
- cma_attach_to_dev(id_priv, cma_dev);
- ret = 0;
+ ret = cma_attach_to_dev(id_priv, cma_dev);
goto out;
}
}
@@ -686,6 +692,7 @@ static int cma_ib_acquire_dev(struct rdma_id_private *id_priv,
const struct ib_gid_attr *sgid_attr;
enum ib_gid_type gid_type;
union ib_gid gid;
+ int ret;
if (dev_addr->dev_type != ARPHRD_INFINIBAND &&
id_priv->id.ps == RDMA_PS_IPOIB)
@@ -711,9 +718,9 @@ static int cma_ib_acquire_dev(struct rdma_id_private *id_priv,
* cma_process_remove().
*/
mutex_lock(&lock);
- cma_attach_to_dev(id_priv, listen_id_priv->cma_dev);
+ ret = cma_attach_to_dev(id_priv, listen_id_priv->cma_dev);
mutex_unlock(&lock);
- return 0;
+ return ret;
}
static int cma_iw_acquire_dev(struct rdma_id_private *id_priv,
@@ -768,7 +775,7 @@ static int cma_iw_acquire_dev(struct rdma_id_private *id_priv,
out:
if (!ret)
- cma_attach_to_dev(id_priv, cma_dev);
+ ret = cma_attach_to_dev(id_priv, cma_dev);
mutex_unlock(&lock);
return ret;
@@ -785,7 +792,7 @@ static int cma_resolve_ib_dev(struct rdma_id_private *id_priv)
unsigned int p;
u16 pkey, index;
enum ib_port_state port_state;
- int i;
+ int i, ret;
cma_dev = NULL;
addr = (struct sockaddr_ib *) cma_dst_addr(id_priv);
@@ -828,8 +835,10 @@ static int cma_resolve_ib_dev(struct rdma_id_private *id_priv)
return -ENODEV;
found:
- cma_attach_to_dev(id_priv, cma_dev);
+ ret = cma_attach_to_dev(id_priv, cma_dev);
mutex_unlock(&lock);
+ if (ret)
+ return ret;
addr = (struct sockaddr_ib *)cma_src_addr(id_priv);
memcpy(&addr->sib_addr, &sgid, sizeof(sgid));
cma_translate_ib(addr, &id_priv->id.route.addr.dev_addr);
@@ -2480,8 +2489,8 @@ static int cma_listen_handler(struct rdma_cm_id *id,
return id_priv->id.event_handler(id, event);
}
-static void cma_listen_on_dev(struct rdma_id_private *id_priv,
- struct cma_device *cma_dev)
+static int cma_listen_on_dev(struct rdma_id_private *id_priv,
+ struct cma_device *cma_dev)
{
struct rdma_id_private *dev_id_priv;
struct rdma_cm_id *id;
@@ -2491,12 +2500,12 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
lockdep_assert_held(&lock);
if (cma_family(id_priv) == AF_IB && !rdma_cap_ib_cm(cma_dev->device, 1))
- return;
+ return 0;
id = __rdma_create_id(net, cma_listen_handler, id_priv, id_priv->id.ps,
id_priv->id.qp_type, id_priv->res.kern_name);
if (IS_ERR(id))
- return;
+ return PTR_ERR(id);
dev_id_priv = container_of(id, struct rdma_id_private, id);
@@ -2504,7 +2513,9 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
memcpy(cma_src_addr(dev_id_priv), cma_src_addr(id_priv),
rdma_addr_size(cma_src_addr(id_priv)));
- _cma_attach_to_dev(dev_id_priv, cma_dev);
+ ret = _cma_attach_to_dev(dev_id_priv, cma_dev);
+ if (ret)
+ goto err_attach;
list_add_tail(&dev_id_priv->listen_list, &id_priv->listen_list);
cma_id_get(id_priv);
dev_id_priv->internal_id = 1;
@@ -2514,8 +2525,14 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
ret = rdma_listen(id, id_priv->backlog);
if (ret)
- dev_warn(&cma_dev->device->dev,
- "RDMA CMA: cma_listen_on_dev, error %d\n", ret);
+ goto err_listen;
+ return 0;
+err_listen:
+ list_del(&id_priv->listen_list);
+err_attach:
+ dev_warn(&cma_dev->device->dev, "RDMA CMA: %s, error %d\n", __func__, ret);
+ rdma_destroy_id(id);
+ return ret;
}
static void cma_listen_on_all(struct rdma_id_private *id_priv)
@@ -3113,7 +3130,9 @@ static int cma_bind_loopback(struct rdma_id_private *id_priv)
rdma_addr_set_sgid(&id_priv->id.route.addr.dev_addr, &gid);
ib_addr_set_pkey(&id_priv->id.route.addr.dev_addr, pkey);
id_priv->id.port_num = p;
- cma_attach_to_dev(id_priv, cma_dev);
+ ret = cma_attach_to_dev(id_priv, cma_dev);
+ if (ret)
+ goto out;
cma_set_loopback(cma_src_addr(id_priv));
out:
mutex_unlock(&lock);
@@ -4729,69 +4748,6 @@ static struct notifier_block cma_nb = {
.notifier_call = cma_netdev_callback
};
-static int cma_add_one(struct ib_device *device)
-{
- struct cma_device *cma_dev;
- struct rdma_id_private *id_priv;
- unsigned int i;
- unsigned long supported_gids = 0;
- int ret;
-
- cma_dev = kmalloc(sizeof *cma_dev, GFP_KERNEL);
- if (!cma_dev)
- return -ENOMEM;
-
- cma_dev->device = device;
- cma_dev->default_gid_type = kcalloc(device->phys_port_cnt,
- sizeof(*cma_dev->default_gid_type),
- GFP_KERNEL);
- if (!cma_dev->default_gid_type) {
- ret = -ENOMEM;
- goto free_cma_dev;
- }
-
- cma_dev->default_roce_tos = kcalloc(device->phys_port_cnt,
- sizeof(*cma_dev->default_roce_tos),
- GFP_KERNEL);
- if (!cma_dev->default_roce_tos) {
- ret = -ENOMEM;
- goto free_gid_type;
- }
-
- rdma_for_each_port (device, i) {
- supported_gids = roce_gid_type_mask_support(device, i);
- WARN_ON(!supported_gids);
- if (supported_gids & (1 << CMA_PREFERRED_ROCE_GID_TYPE))
- cma_dev->default_gid_type[i - rdma_start_port(device)] =
- CMA_PREFERRED_ROCE_GID_TYPE;
- else
- cma_dev->default_gid_type[i - rdma_start_port(device)] =
- find_first_bit(&supported_gids, BITS_PER_LONG);
- cma_dev->default_roce_tos[i - rdma_start_port(device)] = 0;
- }
-
- init_completion(&cma_dev->comp);
- refcount_set(&cma_dev->refcount, 1);
- INIT_LIST_HEAD(&cma_dev->id_list);
- ib_set_client_data(device, &cma_client, cma_dev);
-
- mutex_lock(&lock);
- list_add_tail(&cma_dev->list, &dev_list);
- list_for_each_entry(id_priv, &listen_any_list, list)
- cma_listen_on_dev(id_priv, cma_dev);
- mutex_unlock(&lock);
-
- trace_cm_add_one(device);
- return 0;
-
-free_gid_type:
- kfree(cma_dev->default_gid_type);
-
-free_cma_dev:
- kfree(cma_dev);
- return ret;
-}
-
static void cma_send_device_removal_put(struct rdma_id_private *id_priv)
{
struct rdma_cm_event event = { .event = RDMA_CM_EVENT_DEVICE_REMOVAL };
@@ -4854,6 +4810,81 @@ static void cma_process_remove(struct cma_device *cma_dev)
wait_for_completion(&cma_dev->comp);
}
+static int cma_add_one(struct ib_device *device)
+{
+ struct cma_device *cma_dev;
+ struct rdma_id_private *id_priv;
+ unsigned int i;
+ unsigned long supported_gids = 0;
+ int ret;
+
+ cma_dev = kmalloc(sizeof(*cma_dev), GFP_KERNEL);
+ if (!cma_dev)
+ return -ENOMEM;
+
+ cma_dev->device = device;
+ cma_dev->default_gid_type = kcalloc(device->phys_port_cnt,
+ sizeof(*cma_dev->default_gid_type),
+ GFP_KERNEL);
+ if (!cma_dev->default_gid_type) {
+ ret = -ENOMEM;
+ goto free_cma_dev;
+ }
+
+ cma_dev->default_roce_tos = kcalloc(device->phys_port_cnt,
+ sizeof(*cma_dev->default_roce_tos),
+ GFP_KERNEL);
+ if (!cma_dev->default_roce_tos) {
+ ret = -ENOMEM;
+ goto free_gid_type;
+ }
+
+ rdma_for_each_port (device, i) {
+ supported_gids = roce_gid_type_mask_support(device, i);
+ WARN_ON(!supported_gids);
+ if (supported_gids & (1 << CMA_PREFERRED_ROCE_GID_TYPE))
+ cma_dev->default_gid_type[i - rdma_start_port(device)] =
+ CMA_PREFERRED_ROCE_GID_TYPE;
+ else
+ cma_dev->default_gid_type[i - rdma_start_port(device)] =
+ find_first_bit(&supported_gids, BITS_PER_LONG);
+ cma_dev->default_roce_tos[i - rdma_start_port(device)] = 0;
+ }
+
+ init_completion(&cma_dev->comp);
+ refcount_set(&cma_dev->refcount, 1);
+ INIT_LIST_HEAD(&cma_dev->id_list);
+ ib_set_client_data(device, &cma_client, cma_dev);
+
+ mutex_lock(&lock);
+ list_add_tail(&cma_dev->list, &dev_list);
+ list_for_each_entry(id_priv, &listen_any_list, list) {
+ ret = cma_listen_on_dev(id_priv, cma_dev);
+ if (ret) {
+ mutex_unlock(&lock);
+ goto free_listen;
+ }
+ }
+ mutex_unlock(&lock);
+
+ trace_cm_add_one(device);
+ return 0;
+
+free_listen:
+ mutex_lock(&lock);
+ list_del(&cma_dev->list);
+ mutex_unlock(&lock);
+
+ cma_process_remove(cma_dev);
+ kfree(cma_dev->default_roce_tos);
+free_gid_type:
+ kfree(cma_dev->default_gid_type);
+
+free_cma_dev:
+ kfree(cma_dev);
+ return ret;
+}
+
static void cma_remove_one(struct ib_device *device, void *client_data)
{
struct cma_device *cma_dev = client_data;
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device
2020-09-07 12:21 ` [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device Leon Romanovsky
@ 2020-09-18 23:26 ` Jason Gunthorpe
2020-09-19 9:03 ` Leon Romanovsky
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-18 23:26 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma
On Mon, Sep 07, 2020 at 03:21:49PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> The RDMA-CM code wasn't consistent in flows that attached to cma_dev,
> this caused to situations where failure during attach to listen on such
> device leave RDMA-CM in non-consistent state.
>
> Update the listen/attach flow to correctly deal with failures.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> drivers/infiniband/core/cma.c | 197 ++++++++++++++++++++--------------
> 1 file changed, 114 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 3fc3c821743d..ab1f8b707a5b 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -458,8 +458,8 @@ static int cma_igmp_send(struct net_device *ndev, union ib_gid *mgid, bool join)
> return (in_dev) ? 0 : -ENODEV;
> }
>
> -static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> - struct cma_device *cma_dev)
> +static int _cma_attach_to_dev(struct rdma_id_private *id_priv,
> + struct cma_device *cma_dev)
> {
> cma_dev_get(cma_dev);
> id_priv->cma_dev = cma_dev;
> @@ -475,15 +475,22 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> rdma_restrack_add(&id_priv->res);
>
> trace_cm_id_attach(id_priv, cma_dev->device);
> + return 0;
> }
This commit message doesn't explain this patch at all. This is adding
a return code to _cma_attach_to_dev because some later patch needs
it. This should also be ordered directly before the later patch
> -static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> - struct cma_device *cma_dev)
> +static int cma_listen_on_dev(struct rdma_id_private *id_priv,
> + struct cma_device *cma_dev)
> {
> struct rdma_id_private *dev_id_priv;
> struct rdma_cm_id *id;
> @@ -2491,12 +2500,12 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> lockdep_assert_held(&lock);
>
> if (cma_family(id_priv) == AF_IB && !rdma_cap_ib_cm(cma_dev->device, 1))
> - return;
> + return 0;
>
> id = __rdma_create_id(net, cma_listen_handler, id_priv, id_priv->id.ps,
> id_priv->id.qp_type, id_priv->res.kern_name);
> if (IS_ERR(id))
> - return;
> + return PTR_ERR(id);
And there here it is fixing already missing error handling, seems like
it could be two patches
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device
2020-09-18 23:26 ` Jason Gunthorpe
@ 2020-09-19 9:03 ` Leon Romanovsky
2020-09-24 19:46 ` Jason Gunthorpe
0 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-19 9:03 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma
On Fri, Sep 18, 2020 at 08:26:19PM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:49PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > The RDMA-CM code wasn't consistent in flows that attached to cma_dev,
> > this caused to situations where failure during attach to listen on such
> > device leave RDMA-CM in non-consistent state.
> >
> > Update the listen/attach flow to correctly deal with failures.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > drivers/infiniband/core/cma.c | 197 ++++++++++++++++++++--------------
> > 1 file changed, 114 insertions(+), 83 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index 3fc3c821743d..ab1f8b707a5b 100644
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -458,8 +458,8 @@ static int cma_igmp_send(struct net_device *ndev, union ib_gid *mgid, bool join)
> > return (in_dev) ? 0 : -ENODEV;
> > }
> >
> > -static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > - struct cma_device *cma_dev)
> > +static int _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > + struct cma_device *cma_dev)
> > {
> > cma_dev_get(cma_dev);
> > id_priv->cma_dev = cma_dev;
> > @@ -475,15 +475,22 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > rdma_restrack_add(&id_priv->res);
> >
> > trace_cm_id_attach(id_priv, cma_dev->device);
> > + return 0;
> > }
>
> This commit message doesn't explain this patch at all. This is adding
> a return code to _cma_attach_to_dev because some later patch needs
> it. This should also be ordered directly before the later patch
>
> > -static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> > - struct cma_device *cma_dev)
> > +static int cma_listen_on_dev(struct rdma_id_private *id_priv,
> > + struct cma_device *cma_dev)
> > {
> > struct rdma_id_private *dev_id_priv;
> > struct rdma_cm_id *id;
> > @@ -2491,12 +2500,12 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> > lockdep_assert_held(&lock);
> >
> > if (cma_family(id_priv) == AF_IB && !rdma_cap_ib_cm(cma_dev->device, 1))
> > - return;
> > + return 0;
> >
> > id = __rdma_create_id(net, cma_listen_handler, id_priv, id_priv->id.ps,
> > id_priv->id.qp_type, id_priv->res.kern_name);
> > if (IS_ERR(id))
> > - return;
> > + return PTR_ERR(id);
>
> And there here it is fixing already missing error handling, seems like
> it could be two patches
I don't think so, it is "visible" only after I changed cma_listen_on_dev() function return type,
which is an outcome of _cma_attach_to_dev() change.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device
2020-09-19 9:03 ` Leon Romanovsky
@ 2020-09-24 19:46 ` Jason Gunthorpe
0 siblings, 0 replies; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-24 19:46 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma
On Sat, Sep 19, 2020 at 12:03:40PM +0300, Leon Romanovsky wrote:
> On Fri, Sep 18, 2020 at 08:26:19PM -0300, Jason Gunthorpe wrote:
> > On Mon, Sep 07, 2020 at 03:21:49PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >
> > > The RDMA-CM code wasn't consistent in flows that attached to cma_dev,
> > > this caused to situations where failure during attach to listen on such
> > > device leave RDMA-CM in non-consistent state.
> > >
> > > Update the listen/attach flow to correctly deal with failures.
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/core/cma.c | 197 ++++++++++++++++++++--------------
> > > 1 file changed, 114 insertions(+), 83 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > > index 3fc3c821743d..ab1f8b707a5b 100644
> > > +++ b/drivers/infiniband/core/cma.c
> > > @@ -458,8 +458,8 @@ static int cma_igmp_send(struct net_device *ndev, union ib_gid *mgid, bool join)
> > > return (in_dev) ? 0 : -ENODEV;
> > > }
> > >
> > > -static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > > - struct cma_device *cma_dev)
> > > +static int _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > > + struct cma_device *cma_dev)
> > > {
> > > cma_dev_get(cma_dev);
> > > id_priv->cma_dev = cma_dev;
> > > @@ -475,15 +475,22 @@ static void _cma_attach_to_dev(struct rdma_id_private *id_priv,
> > > rdma_restrack_add(&id_priv->res);
> > >
> > > trace_cm_id_attach(id_priv, cma_dev->device);
> > > + return 0;
> > > }
> >
> > This commit message doesn't explain this patch at all. This is adding
> > a return code to _cma_attach_to_dev because some later patch needs
> > it. This should also be ordered directly before the later patch
> >
> > > -static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> > > - struct cma_device *cma_dev)
> > > +static int cma_listen_on_dev(struct rdma_id_private *id_priv,
> > > + struct cma_device *cma_dev)
> > > {
> > > struct rdma_id_private *dev_id_priv;
> > > struct rdma_cm_id *id;
> > > @@ -2491,12 +2500,12 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> > > lockdep_assert_held(&lock);
> > >
> > > if (cma_family(id_priv) == AF_IB && !rdma_cap_ib_cm(cma_dev->device, 1))
> > > - return;
> > > + return 0;
> > >
> > > id = __rdma_create_id(net, cma_listen_handler, id_priv, id_priv->id.ps,
> > > id_priv->id.qp_type, id_priv->res.kern_name);
> > > if (IS_ERR(id))
> > > - return;
> > > + return PTR_ERR(id);
> >
> > And there here it is fixing already missing error handling, seems like
> > it could be two patches
>
> I don't think so, it is "visible" only after I changed cma_listen_on_dev() function return type,
> which is an outcome of _cma_attach_to_dev() change.
If the first patch adds this missing error handling to
cam_listen_on_dev, then the 2nd patch adds new error handling to
_cma_attach_to_dev() it would be much clearer what this is all about.
Jason
> Thanks
>
> >
> > Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 08/14] RDMA/core: Allow drivers to disable restrack DB
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (6 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 07/14] RDMA/cma: Be strict with attaching to CMA device Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 09/14] RDMA/counter: Combine allocation and bind logic Leon Romanovsky
` (5 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, Gal Pressman, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
Driver QP types are special case with no IBTA restrictions. For example,
EFA implemented creation of this QP type as regular one, while mlx5
separated create to two step: create and modify. That separation causes
to the situation where DC QP (mlx5) is always added to the same xarray
index zero.
This change allows to drivers like mlx5 simply disable restrack DB
tracking, but it doesn't disable kref on the memory.
Fixes: 52e0a118a203 ("RDMA/restrack: Track driver QP types in resource tracker")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/counters.c | 2 +-
drivers/infiniband/core/restrack.c | 12 ++++++++++--
drivers/infiniband/hw/mlx5/qp.c | 2 +-
include/rdma/restrack.h | 24 ++++++++++++++++++++++++
4 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index e4ff0d3328b6..fa1a4a318fd7 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -275,7 +275,7 @@ int rdma_counter_bind_qp_auto(struct ib_qp *qp, u8 port)
struct rdma_counter *counter;
int ret;
- if (!qp->res.valid || rdma_is_kernel_res(&qp->res))
+ if (!rdma_restrack_is_tracked(&qp->res) || rdma_is_kernel_res(&qp->res))
return 0;
if (!rdma_is_port_valid(dev, port))
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 0c67acf2169d..05ea7c61ae0b 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -221,11 +221,14 @@ void rdma_restrack_add(struct rdma_restrack_entry *res)
{
struct ib_device *dev = res_to_dev(res);
struct rdma_restrack_root *rt;
- int ret;
+ int ret = 0;
if (!dev)
return;
+ if (res->no_track)
+ goto out;
+
rt = &dev->res[res->type];
if (res->type == RDMA_RESTRACK_QP) {
@@ -246,6 +249,7 @@ void rdma_restrack_add(struct rdma_restrack_entry *res)
&rt->next_id, GFP_KERNEL);
}
+out:
if (!ret)
res->valid = true;
}
@@ -318,6 +322,9 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
return;
}
+ if (res->no_track)
+ goto out;
+
dev = res_to_dev(res);
if (WARN_ON(!dev))
return;
@@ -328,8 +335,9 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
if (res->type == RDMA_RESTRACK_MR || res->type == RDMA_RESTRACK_QP)
return;
WARN_ON(old != res);
- res->valid = false;
+out:
+ res->valid = false;
rdma_restrack_put(res);
wait_for_completion(&res->comp);
}
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index edcd54b7603c..8a754a8e2558 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -2434,7 +2434,7 @@ static int create_dct(struct mlx5_ib_dev *dev, struct ib_pd *pd,
}
qp->state = IB_QPS_RESET;
-
+ rdma_restrack_no_track(&qp->ibqp.res);
return 0;
}
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index 10bfed0fcd32..d52f7ad6641f 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -68,6 +68,14 @@ struct rdma_restrack_entry {
* As an example for that, see mlx5 QPs with type MLX5_IB_QPT_HW_GSI
*/
bool valid;
+ /**
+ * @no_track: don't add this entry to restrack DB
+ *
+ * This field is used to mark an entry that doesn't need to be added to
+ * internal restrack DB and presented later to the users at the nldev
+ * query stage.
+ */
+ u8 no_track : 1;
/*
* @kref: Protect destroy of the resource
*/
@@ -145,4 +153,20 @@ int rdma_nl_stat_hwcounter_entry(struct sk_buff *msg, const char *name,
struct rdma_restrack_entry *rdma_restrack_get_byid(struct ib_device *dev,
enum rdma_restrack_type type,
u32 id);
+
+/**
+ * rdma_restrack_no_track() - don't add resource to the DB
+ * @res: resource entry
+ *
+ * Every user of thie API should be cross examined.
+ * Probaby you don't need to use this function.
+ */
+static inline void rdma_restrack_no_track(struct rdma_restrack_entry *res)
+{
+ res->no_track = true;
+}
+static inline bool rdma_restrack_is_tracked(struct rdma_restrack_entry *res)
+{
+ return !res->no_track;
+}
#endif /* _RDMA_RESTRACK_H_ */
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH rdma-next v2 09/14] RDMA/counter: Combine allocation and bind logic
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (7 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 08/14] RDMA/core: Allow drivers to disable restrack DB Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 10/14] RDMA/restrack: Store all special QPs in restrack DB Leon Romanovsky
` (4 subsequent siblings)
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@nvidia.com>
RDMA counters are allocated and bounded to QP immediately after that.
Only after this two step process they are really usable. By combining
the logic, we are ensuring that once counter is returned to the caller,
it will have everything set.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
drivers/infiniband/core/counters.c | 130 +++++++++++++----------------
1 file changed, 58 insertions(+), 72 deletions(-)
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index fa1a4a318fd7..e208cc6d3cb0 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -64,8 +64,40 @@ int rdma_counter_set_auto_mode(struct ib_device *dev, u8 port,
return ret;
}
-static struct rdma_counter *rdma_counter_alloc(struct ib_device *dev, u8 port,
- enum rdma_nl_counter_mode mode)
+static void auto_mode_init_counter(struct rdma_counter *counter,
+ const struct ib_qp *qp,
+ enum rdma_nl_counter_mask new_mask)
+{
+ struct auto_mode_param *param = &counter->mode.param;
+
+ counter->mode.mode = RDMA_COUNTER_MODE_AUTO;
+ counter->mode.mask = new_mask;
+
+ if (new_mask & RDMA_COUNTER_MASK_QP_TYPE)
+ param->qp_type = qp->qp_type;
+}
+
+static int __rdma_counter_bind_qp(struct rdma_counter *counter,
+ struct ib_qp *qp)
+{
+ int ret;
+
+ if (qp->counter)
+ return -EINVAL;
+
+ if (!qp->device->ops.counter_bind_qp)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&counter->lock);
+ ret = qp->device->ops.counter_bind_qp(counter, qp);
+ mutex_unlock(&counter->lock);
+
+ return ret;
+}
+
+static struct rdma_counter *alloc_and_bind(struct ib_device *dev, u8 port,
+ struct ib_qp *qp,
+ enum rdma_nl_counter_mode mode)
{
struct rdma_port_counter *port_counter;
struct rdma_counter *counter;
@@ -88,11 +120,19 @@ static struct rdma_counter *rdma_counter_alloc(struct ib_device *dev, u8 port,
port_counter = &dev->port_data[port].port_counter;
mutex_lock(&port_counter->lock);
- if (mode == RDMA_COUNTER_MODE_MANUAL) {
+ switch (mode) {
+ case RDMA_COUNTER_MODE_MANUAL:
ret = __counter_set_mode(&port_counter->mode,
RDMA_COUNTER_MODE_MANUAL, 0);
if (ret)
goto err_mode;
+ break;
+ case RDMA_COUNTER_MODE_AUTO:
+ auto_mode_init_counter(counter, qp, port_counter->mode.mask);
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ goto err_mode;
}
port_counter->num_counters++;
@@ -102,6 +142,12 @@ static struct rdma_counter *rdma_counter_alloc(struct ib_device *dev, u8 port,
kref_init(&counter->kref);
mutex_init(&counter->lock);
+ ret = __rdma_counter_bind_qp(counter, qp);
+ if (ret)
+ goto err_mode;
+
+ rdma_restrack_parent_name(&counter->res, &qp->res);
+ rdma_restrack_add(&counter->res);
return counter;
err_mode:
@@ -132,19 +178,6 @@ static void rdma_counter_free(struct rdma_counter *counter)
kfree(counter);
}
-static void auto_mode_init_counter(struct rdma_counter *counter,
- const struct ib_qp *qp,
- enum rdma_nl_counter_mask new_mask)
-{
- struct auto_mode_param *param = &counter->mode.param;
-
- counter->mode.mode = RDMA_COUNTER_MODE_AUTO;
- counter->mode.mask = new_mask;
-
- if (new_mask & RDMA_COUNTER_MASK_QP_TYPE)
- param->qp_type = qp->qp_type;
-}
-
static bool auto_mode_match(struct ib_qp *qp, struct rdma_counter *counter,
enum rdma_nl_counter_mask auto_mask)
{
@@ -161,24 +194,6 @@ static bool auto_mode_match(struct ib_qp *qp, struct rdma_counter *counter,
return match;
}
-static int __rdma_counter_bind_qp(struct rdma_counter *counter,
- struct ib_qp *qp)
-{
- int ret;
-
- if (qp->counter)
- return -EINVAL;
-
- if (!qp->device->ops.counter_bind_qp)
- return -EOPNOTSUPP;
-
- mutex_lock(&counter->lock);
- ret = qp->device->ops.counter_bind_qp(counter, qp);
- mutex_unlock(&counter->lock);
-
- return ret;
-}
-
static int __rdma_counter_unbind_qp(struct ib_qp *qp)
{
struct rdma_counter *counter = qp->counter;
@@ -247,13 +262,6 @@ static struct rdma_counter *rdma_get_counter_auto_mode(struct ib_qp *qp,
return counter;
}
-static void rdma_counter_res_add(struct rdma_counter *counter,
- struct ib_qp *qp)
-{
- rdma_restrack_parent_name(&counter->res, &qp->res);
- rdma_restrack_add(&counter->res);
-}
-
static void counter_release(struct kref *kref)
{
struct rdma_counter *counter;
@@ -293,19 +301,9 @@ int rdma_counter_bind_qp_auto(struct ib_qp *qp, u8 port)
return ret;
}
} else {
- counter = rdma_counter_alloc(dev, port, RDMA_COUNTER_MODE_AUTO);
+ counter = alloc_and_bind(dev, port, qp, RDMA_COUNTER_MODE_AUTO);
if (!counter)
return -ENOMEM;
-
- auto_mode_init_counter(counter, qp, port_counter->mode.mask);
-
- ret = __rdma_counter_bind_qp(counter, qp);
- if (ret) {
- rdma_counter_free(counter);
- return ret;
- }
-
- rdma_counter_res_add(counter, qp);
}
return 0;
@@ -419,15 +417,6 @@ static struct ib_qp *rdma_counter_get_qp(struct ib_device *dev, u32 qp_num)
return NULL;
}
-static int rdma_counter_bind_qp_manual(struct rdma_counter *counter,
- struct ib_qp *qp)
-{
- if ((counter->device != qp->device) || (counter->port != qp->port))
- return -EINVAL;
-
- return __rdma_counter_bind_qp(counter, qp);
-}
-
static struct rdma_counter *rdma_get_counter_by_id(struct ib_device *dev,
u32 counter_id)
{
@@ -475,7 +464,12 @@ int rdma_counter_bind_qpn(struct ib_device *dev, u8 port,
goto err_task;
}
- ret = rdma_counter_bind_qp_manual(counter, qp);
+ if ((counter->device != qp->device) || (counter->port != qp->port)) {
+ ret = -EINVAL;
+ goto err_task;
+ }
+
+ ret = __rdma_counter_bind_qp(counter, qp);
if (ret)
goto err_task;
@@ -520,26 +514,18 @@ int rdma_counter_bind_qpn_alloc(struct ib_device *dev, u8 port,
goto err;
}
- counter = rdma_counter_alloc(dev, port, RDMA_COUNTER_MODE_MANUAL);
+ counter = alloc_and_bind(dev, port, qp, RDMA_COUNTER_MODE_MANUAL);
if (!counter) {
ret = -ENOMEM;
goto err;
}
- ret = rdma_counter_bind_qp_manual(counter, qp);
- if (ret)
- goto err_bind;
-
if (counter_id)
*counter_id = counter->id;
- rdma_counter_res_add(counter, qp);
-
rdma_restrack_put(&qp->res);
- return ret;
+ return 0;
-err_bind:
- rdma_counter_free(counter);
err:
rdma_restrack_put(&qp->res);
return ret;
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH rdma-next v2 10/14] RDMA/restrack: Store all special QPs in restrack DB
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (8 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 09/14] RDMA/counter: Combine allocation and bind logic Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-18 23:30 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects Leon Romanovsky
` (3 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
Special QPs (SMI and GSI) have different rules in regards of their QP
numbers. While all other QP numbers are unique per-device, the QP0 and QP1
are created per-port as requested by IBTA.
In multiple port devices, the number of SMI and GSI QPs with be equal
to the number ports.
[leonro@vm ~]$ rdma dev
0: ibp0s9: node_type ca fw 4.4.9999 node_guid 5254:00c0:fe12:3455 sys_image_guid 5254:00c0:fe12:3455
[leonro@vm ~]$ rdma link
0/1: ibp0s9/1: subnet_prefix fe80:0000:0000:0000 lid 13397 sm_lid 49151 lmc 0 state ACTIVE physical_state LINK_UP
0/2: ibp0s9/2: subnet_prefix fe80:0000:0000:0000 lid 13397 sm_lid 49151 lmc 0 state UNKNOWN physical_state UNKNOWN
Before:
[leonro@mtl-leonro-l-vm ~]$ rdma res show qp type SMI,GSI
link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
After:
[leonro@vm ~]$ rdma res show qp type SMI,GSI
link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
link ibp0s9/2 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
link ibp0s9/2 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/core_priv.h | 2 ++
drivers/infiniband/core/restrack.c | 11 +++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index e84b0fedaacb..7c4752c47f80 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -347,6 +347,8 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
qp->srq = attr->srq;
qp->rwq_ind_tbl = attr->rwq_ind_tbl;
qp->event_handler = attr->event_handler;
+ qp->qp_type = attr->qp_type;
+ qp->port = attr->port_num;
atomic_set(&qp->usecnt, 0);
spin_lock_init(&qp->mr_lock);
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 05ea7c61ae0b..2cc8d63e93ae 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -235,8 +235,15 @@ void rdma_restrack_add(struct rdma_restrack_entry *res)
/* Special case to ensure that LQPN points to right QP */
struct ib_qp *qp = container_of(res, struct ib_qp, res);
- ret = xa_insert(&rt->xa, qp->qp_num, res, GFP_KERNEL);
- res->id = ret ? 0 : qp->qp_num;
+ WARN_ONCE(qp->qp_num >> 24 || qp->port >> 8,
+ "QP number 0x%0X and port 0x%0X", qp->qp_num,
+ qp->port);
+ res->id = qp->qp_num;
+ if (qp->qp_type == IB_QPT_SMI || qp->qp_type == IB_QPT_GSI)
+ res->id |= qp->port << 24;
+ ret = xa_insert(&rt->xa, res->id, res, GFP_KERNEL);
+ if (ret)
+ res->id = 0;
} else if (res->type == RDMA_RESTRACK_COUNTER) {
/* Special case to ensure that cntn points to right counter */
struct rdma_counter *counter;
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 10/14] RDMA/restrack: Store all special QPs in restrack DB
2020-09-07 12:21 ` [PATCH rdma-next v2 10/14] RDMA/restrack: Store all special QPs in restrack DB Leon Romanovsky
@ 2020-09-18 23:30 ` Jason Gunthorpe
2020-09-19 8:27 ` Leon Romanovsky
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-18 23:30 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma
On Mon, Sep 07, 2020 at 03:21:52PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Special QPs (SMI and GSI) have different rules in regards of their QP
> numbers. While all other QP numbers are unique per-device, the QP0 and QP1
> are created per-port as requested by IBTA.
>
> In multiple port devices, the number of SMI and GSI QPs with be equal
> to the number ports.
>
> [leonro@vm ~]$ rdma dev
> 0: ibp0s9: node_type ca fw 4.4.9999 node_guid 5254:00c0:fe12:3455 sys_image_guid 5254:00c0:fe12:3455
> [leonro@vm ~]$ rdma link
> 0/1: ibp0s9/1: subnet_prefix fe80:0000:0000:0000 lid 13397 sm_lid 49151 lmc 0 state ACTIVE physical_state LINK_UP
> 0/2: ibp0s9/2: subnet_prefix fe80:0000:0000:0000 lid 13397 sm_lid 49151 lmc 0 state UNKNOWN physical_state UNKNOWN
>
> Before:
> [leonro@mtl-leonro-l-vm ~]$ rdma res show qp type SMI,GSI
> link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
> link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
>
> After:
> [leonro@vm ~]$ rdma res show qp type SMI,GSI
> link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
> link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> link ibp0s9/2 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
> link ibp0s9/2 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/core/core_priv.h | 2 ++
> drivers/infiniband/core/restrack.c | 11 +++++++++--
> 2 files changed, 11 insertions(+), 2 deletions(-)
Isn't this a pretty good stand alone bug fix? Add a fixes line?
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH rdma-next v2 10/14] RDMA/restrack: Store all special QPs in restrack DB
2020-09-18 23:30 ` Jason Gunthorpe
@ 2020-09-19 8:27 ` Leon Romanovsky
0 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-19 8:27 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma
On Fri, Sep 18, 2020 at 08:30:11PM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:52PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Special QPs (SMI and GSI) have different rules in regards of their QP
> > numbers. While all other QP numbers are unique per-device, the QP0 and QP1
> > are created per-port as requested by IBTA.
> >
> > In multiple port devices, the number of SMI and GSI QPs with be equal
> > to the number ports.
> >
> > [leonro@vm ~]$ rdma dev
> > 0: ibp0s9: node_type ca fw 4.4.9999 node_guid 5254:00c0:fe12:3455 sys_image_guid 5254:00c0:fe12:3455
> > [leonro@vm ~]$ rdma link
> > 0/1: ibp0s9/1: subnet_prefix fe80:0000:0000:0000 lid 13397 sm_lid 49151 lmc 0 state ACTIVE physical_state LINK_UP
> > 0/2: ibp0s9/2: subnet_prefix fe80:0000:0000:0000 lid 13397 sm_lid 49151 lmc 0 state UNKNOWN physical_state UNKNOWN
> >
> > Before:
> > [leonro@mtl-leonro-l-vm ~]$ rdma res show qp type SMI,GSI
> > link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
> > link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> >
> > After:
> > [leonro@vm ~]$ rdma res show qp type SMI,GSI
> > link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
> > link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> > link ibp0s9/2 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
> > link ibp0s9/2 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > drivers/infiniband/core/core_priv.h | 2 ++
> > drivers/infiniband/core/restrack.c | 11 +++++++++--
> > 2 files changed, 11 insertions(+), 2 deletions(-)
>
> Isn't this a pretty good stand alone bug fix? Add a fixes line?
I see this patch as part of this series, and have no idea if it works
as standalone fix that can be taken automatically to the stable@ by
autosel bot later.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (9 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 10/14] RDMA/restrack: Store all special QPs in restrack DB Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-18 23:31 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 12/14] RDMA/restrack: Support all QP types Leon Romanovsky
` (2 subsequent siblings)
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma, Mark Zhang
From: Leon Romanovsky <leonro@mellanox.com>
Restrack stores the IB objects in the internal xarray and insertion
where can fail. As long as restrack was helper tool for the
debuggability, such failure were ignored.
In the following patches, the ib_core will be changed to manage allocated
IB objects in restrack DB for the proper memory lifetime. It requires to
ensure that insertion errors are not ignored.
Reviewed-by: Mark Zhang <markz@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/cma.c | 9 ++-
drivers/infiniband/core/core_priv.h | 9 ++-
drivers/infiniband/core/counters.c | 37 ++++++-----
drivers/infiniband/core/cq.c | 17 ++++-
drivers/infiniband/core/rdma_core.c | 3 +-
drivers/infiniband/core/restrack.c | 26 +++++---
drivers/infiniband/core/restrack.h | 2 +-
drivers/infiniband/core/uverbs_cmd.c | 27 ++++++--
drivers/infiniband/core/uverbs_std_types_cq.c | 6 +-
drivers/infiniband/core/verbs.c | 63 ++++++++++++++-----
10 files changed, 147 insertions(+), 52 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index ab1f8b707a5b..f1c45f67e2eb 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -461,6 +461,8 @@ static int cma_igmp_send(struct net_device *ndev, union ib_gid *mgid, bool join)
static int _cma_attach_to_dev(struct rdma_id_private *id_priv,
struct cma_device *cma_dev)
{
+ int ret;
+
cma_dev_get(cma_dev);
id_priv->cma_dev = cma_dev;
id_priv->id.device = cma_dev->device;
@@ -472,7 +474,12 @@ static int _cma_attach_to_dev(struct rdma_id_private *id_priv,
* attach to "current" task.
*/
rdma_restrack_set_name(&id_priv->res, id_priv->res.kern_name);
- rdma_restrack_add(&id_priv->res);
+ ret = rdma_restrack_add(&id_priv->res);
+ if (ret) {
+ list_del(&cma_dev->id_list);
+ cma_dev_put(cma_dev);
+ return ret;
+ }
trace_cm_id_attach(id_priv, cma_dev->device);
return 0;
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 7c4752c47f80..7f4bb09efab8 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -327,6 +327,7 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
enum ib_qp_type qp_type = attr->qp_type;
struct ib_qp *qp;
bool is_xrc;
+ int ret;
if (!dev->ops.create_qp)
return ERR_PTR(-EOPNOTSUPP);
@@ -364,9 +365,15 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
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) {
rdma_restrack_parent_name(&qp->res, &pd->res);
- rdma_restrack_add(&qp->res);
+ ret = rdma_restrack_add(&qp->res);
+ if (ret)
+ goto err;
}
return qp;
+err:
+ rdma_restrack_put(&qp->res);
+ dev->ops.destroy_qp(qp, udata);
+ return ERR_PTR(ret);
}
struct rdma_dev_addr;
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index e208cc6d3cb0..4a0607872e54 100644
--- a/drivers/infiniband/core/counters.c
+++ b/drivers/infiniband/core/counters.c
@@ -95,6 +95,21 @@ static int __rdma_counter_bind_qp(struct rdma_counter *counter,
return ret;
}
+static int __rdma_counter_unbind_qp(struct ib_qp *qp)
+{
+ struct rdma_counter *counter = qp->counter;
+ int ret;
+
+ if (!qp->device->ops.counter_unbind_qp)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&counter->lock);
+ ret = qp->device->ops.counter_unbind_qp(qp);
+ mutex_unlock(&counter->lock);
+
+ return ret;
+}
+
static struct rdma_counter *alloc_and_bind(struct ib_device *dev, u8 port,
struct ib_qp *qp,
enum rdma_nl_counter_mode mode)
@@ -147,9 +162,14 @@ static struct rdma_counter *alloc_and_bind(struct ib_device *dev, u8 port,
goto err_mode;
rdma_restrack_parent_name(&counter->res, &qp->res);
- rdma_restrack_add(&counter->res);
+ ret = rdma_restrack_add(&counter->res);
+ if (ret)
+ goto err_restrack;
+
return counter;
+err_restrack:
+ __rdma_counter_unbind_qp(qp);
err_mode:
mutex_unlock(&port_counter->lock);
kfree(counter->stats);
@@ -194,21 +214,6 @@ static bool auto_mode_match(struct ib_qp *qp, struct rdma_counter *counter,
return match;
}
-static int __rdma_counter_unbind_qp(struct ib_qp *qp)
-{
- struct rdma_counter *counter = qp->counter;
- int ret;
-
- if (!qp->device->ops.counter_unbind_qp)
- return -EOPNOTSUPP;
-
- mutex_lock(&counter->lock);
- ret = qp->device->ops.counter_unbind_qp(qp);
- mutex_unlock(&counter->lock);
-
- return ret;
-}
-
static void counter_history_stat_update(struct rdma_counter *counter)
{
struct ib_device *dev = counter->device;
diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c
index 704613b19eb3..4afd11db46b9 100644
--- a/drivers/infiniband/core/cq.c
+++ b/drivers/infiniband/core/cq.c
@@ -267,10 +267,25 @@ struct ib_cq *__ib_alloc_cq(struct ib_device *dev, void *private, int nr_cqe,
goto out_destroy_cq;
}
- rdma_restrack_add(&cq->res);
+ ret = rdma_restrack_add(&cq->res);
+ if (ret)
+ goto out_poll_cq;
+
trace_cq_alloc(cq, nr_cqe, comp_vector, poll_ctx);
return cq;
+out_poll_cq:
+ switch (cq->poll_ctx) {
+ case IB_POLL_SOFTIRQ:
+ irq_poll_disable(&cq->iop);
+ break;
+ case IB_POLL_WORKQUEUE:
+ case IB_POLL_UNBOUND_WORKQUEUE:
+ cancel_work_sync(&cq->work);
+ break;
+ default:
+ break;
+ }
out_destroy_cq:
rdma_dim_destroy(cq);
cq->device->ops.destroy_cq(cq, NULL);
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index e1c616e47d2b..d5bf25c6eadc 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -831,10 +831,9 @@ static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile,
ib_rdmacg_uncharge(&ucontext->cg_obj, ib_dev,
RDMACG_RESOURCE_HCA_HANDLE);
- rdma_restrack_del(&ucontext->res);
-
ib_dev->ops.dealloc_ucontext(ucontext);
WARN_ON(!xa_empty(&ucontext->mmap_xa));
+ rdma_restrack_del(&ucontext->res);
kfree(ucontext);
ufile->ucontext = NULL;
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 2cc8d63e93ae..acf06c29b176 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -217,21 +217,22 @@ EXPORT_SYMBOL(rdma_restrack_new);
* rdma_restrack_add() - add object to the reource tracking database
* @res: resource entry
*/
-void rdma_restrack_add(struct rdma_restrack_entry *res)
+int __must_check rdma_restrack_add(struct rdma_restrack_entry *res)
{
struct ib_device *dev = res_to_dev(res);
struct rdma_restrack_root *rt;
int ret = 0;
if (!dev)
- return;
+ return -ENODEV;
if (res->no_track)
goto out;
rt = &dev->res[res->type];
- if (res->type == RDMA_RESTRACK_QP) {
+ switch (res->type) {
+ case RDMA_RESTRACK_QP: {
/* Special case to ensure that LQPN points to right QP */
struct ib_qp *qp = container_of(res, struct ib_qp, res);
@@ -244,21 +245,26 @@ void rdma_restrack_add(struct rdma_restrack_entry *res)
ret = xa_insert(&rt->xa, res->id, res, GFP_KERNEL);
if (ret)
res->id = 0;
- } else if (res->type == RDMA_RESTRACK_COUNTER) {
+ } break;
+ case RDMA_RESTRACK_COUNTER: {
/* Special case to ensure that cntn points to right counter */
- struct rdma_counter *counter;
+ struct rdma_counter *counter = container_of(res, struct rdma_counter, res);
- counter = container_of(res, struct rdma_counter, res);
ret = xa_insert(&rt->xa, counter->id, res, GFP_KERNEL);
- res->id = ret ? 0 : counter->id;
- } else {
+ if (ret)
+ break;
+ res->id = counter->id;
+ } break;
+ default:
ret = xa_alloc_cyclic(&rt->xa, &res->id, res, xa_limit_32b,
&rt->next_id, GFP_KERNEL);
}
out:
- if (!ret)
- res->valid = true;
+ if (ret)
+ return ret;
+ res->valid = true;
+ return 0;
}
EXPORT_SYMBOL(rdma_restrack_add);
diff --git a/drivers/infiniband/core/restrack.h b/drivers/infiniband/core/restrack.h
index 49c1d84cca2d..7e593ed8999f 100644
--- a/drivers/infiniband/core/restrack.h
+++ b/drivers/infiniband/core/restrack.h
@@ -25,7 +25,7 @@ struct rdma_restrack_root {
int rdma_restrack_init(struct ib_device *dev);
void rdma_restrack_clean(struct ib_device *dev);
-void rdma_restrack_add(struct rdma_restrack_entry *res);
+int __must_check rdma_restrack_add(struct rdma_restrack_entry *res);
void rdma_restrack_del(struct rdma_restrack_entry *res);
void rdma_restrack_new(struct rdma_restrack_entry *res,
enum rdma_restrack_type type);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 5dd3d72d594d..d7c532154bbd 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -252,7 +252,9 @@ int ib_init_ucontext(struct uverbs_attr_bundle *attrs)
if (ret)
goto err_uncharge;
- rdma_restrack_add(&ucontext->res);
+ ret = rdma_restrack_add(&ucontext->res);
+ if (ret)
+ goto err_restrack;
/*
* Make sure that ib_uverbs_get_ucontext() sees the pointer update
@@ -264,6 +266,8 @@ int ib_init_ucontext(struct uverbs_attr_bundle *attrs)
up_read(&file->hw_destroy_rwsem);
return 0;
+err_restrack:
+ ucontext->device->ops.dealloc_ucontext(ucontext);
err_uncharge:
ib_rdmacg_uncharge(&ucontext->cg_obj, ucontext->device,
RDMACG_RESOURCE_HCA_HANDLE);
@@ -449,7 +453,10 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
ret = ib_dev->ops.alloc_pd(pd, &attrs->driver_udata);
if (ret)
goto err_alloc;
- rdma_restrack_add(&pd->res);
+
+ ret = rdma_restrack_add(&pd->res);
+ if (ret)
+ goto err_restrack;
uobj->object = pd;
uobj_finalize_uobj_create(uobj, attrs);
@@ -457,6 +464,9 @@ static int ib_uverbs_alloc_pd(struct uverbs_attr_bundle *attrs)
resp.pd_handle = uobj->id;
return uverbs_response(attrs, &resp, sizeof(resp));
+err_restrack:
+ ib_dev->ops.dealloc_pd(pd, &attrs->driver_udata);
+
err_alloc:
rdma_restrack_put(&pd->res);
kfree(pd);
@@ -752,7 +762,9 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
rdma_restrack_set_name(&mr->res, NULL);
- rdma_restrack_add(&mr->res);
+ ret = rdma_restrack_add(&mr->res);
+ if (ret)
+ goto err_restrack;
uobj->object = mr;
uobj_put_obj_read(pd);
@@ -763,6 +775,8 @@ static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)
resp.mr_handle = uobj->id;
return uverbs_response(attrs, &resp, sizeof(resp));
+err_restrack:
+ pd->device->ops.dereg_mr(mr, &attrs->driver_udata);
err_put:
uobj_put_obj_read(pd);
err_free:
@@ -1009,7 +1023,10 @@ static int create_cq(struct uverbs_attr_bundle *attrs,
ret = ib_dev->ops.create_cq(cq, &attr, &attrs->driver_udata);
if (ret)
goto err_free;
- rdma_restrack_add(&cq->res);
+
+ ret = rdma_restrack_add(&cq->res);
+ if (ret)
+ goto err_restrack;
obj->uevent.uobject.object = cq;
obj->uevent.event_file = READ_ONCE(attrs->ufile->default_async_file);
@@ -1022,6 +1039,8 @@ static int create_cq(struct uverbs_attr_bundle *attrs,
resp.response_length = uverbs_response_length(attrs, sizeof(resp));
return uverbs_response(attrs, &resp, sizeof(resp));
+err_restrack:
+ ib_dev->ops.destroy_cq(cq, &attrs->driver_udata);
err_free:
rdma_restrack_put(&cq->res);
kfree(cq);
diff --git a/drivers/infiniband/core/uverbs_std_types_cq.c b/drivers/infiniband/core/uverbs_std_types_cq.c
index 8dabd05988b2..838201f5e921 100644
--- a/drivers/infiniband/core/uverbs_std_types_cq.c
+++ b/drivers/infiniband/core/uverbs_std_types_cq.c
@@ -131,16 +131,20 @@ static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(
ret = ib_dev->ops.create_cq(cq, &attr, &attrs->driver_udata);
if (ret)
goto err_free;
+ ret = rdma_restrack_add(&cq->res);
+ if (ret)
+ goto err_restrack;
obj->uevent.uobject.object = cq;
obj->uevent.uobject.user_handle = user_handle;
- rdma_restrack_add(&cq->res);
uverbs_finalize_uobj_create(attrs, UVERBS_ATTR_CREATE_CQ_HANDLE);
ret = uverbs_copy_to(attrs, UVERBS_ATTR_CREATE_CQ_RESP_CQE, &cq->cqe,
sizeof(cq->cqe));
return ret;
+err_restrack:
+ ib_dev->ops.destroy_cq(cq, &attrs->driver_udata);
err_free:
rdma_restrack_put(&cq->res);
kfree(cq);
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index c1ab7d8eced6..7802247c2a67 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -276,12 +276,12 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
rdma_restrack_set_name(&pd->res, caller);
ret = device->ops.alloc_pd(pd, NULL);
- if (ret) {
- rdma_restrack_put(&pd->res);
- kfree(pd);
- return ERR_PTR(ret);
- }
- rdma_restrack_add(&pd->res);
+ if (ret)
+ goto err_alloc;
+
+ ret = rdma_restrack_add(&pd->res);
+ if (ret)
+ goto err_restrack;
if (device->attrs.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
pd->local_dma_lkey = device->local_dma_lkey;
@@ -318,6 +318,13 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,
}
return pd;
+
+err_restrack:
+ device->ops.dealloc_pd(pd, NULL);
+err_alloc:
+ rdma_restrack_put(&pd->res);
+ kfree(pd);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(__ib_alloc_pd);
@@ -2002,14 +2009,21 @@ struct ib_cq *__ib_create_cq(struct ib_device *device,
rdma_restrack_set_name(&cq->res, caller);
ret = device->ops.create_cq(cq, cq_attr, NULL);
- if (ret) {
- rdma_restrack_put(&cq->res);
- kfree(cq);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto err_create;
+
+ ret = rdma_restrack_add(&cq->res);
+ if (ret)
+ goto err_restrack;
- rdma_restrack_add(&cq->res);
return cq;
+
+err_restrack:
+ device->ops.destroy_cq(cq, NULL);
+err_create:
+ rdma_restrack_put(&cq->res);
+ kfree(cq);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(__ib_create_cq);
@@ -2060,6 +2074,7 @@ struct ib_mr *ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
u64 virt_addr, int access_flags)
{
struct ib_mr *mr;
+ int ret;
if (access_flags & IB_ACCESS_ON_DEMAND) {
if (!(pd->device->attrs.device_cap_flags &
@@ -2082,7 +2097,12 @@ struct ib_mr *ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
rdma_restrack_parent_name(&mr->res, &pd->res);
- rdma_restrack_add(&mr->res);
+ ret = rdma_restrack_add(&mr->res);
+ if (ret) {
+ rdma_restrack_put(&mr->res);
+ pd->device->ops.dereg_mr(mr, NULL);
+ mr = ERR_PTR(ret);
+ }
return mr;
}
@@ -2139,6 +2159,7 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
u32 max_num_sg)
{
struct ib_mr *mr;
+ int ret;
if (!pd->device->ops.alloc_mr) {
mr = ERR_PTR(-EOPNOTSUPP);
@@ -2166,7 +2187,13 @@ struct ib_mr *ib_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
rdma_restrack_parent_name(&mr->res, &pd->res);
- rdma_restrack_add(&mr->res);
+ ret = rdma_restrack_add(&mr->res);
+ if (ret) {
+ rdma_restrack_put(&mr->res);
+ pd->device->ops.dereg_mr(mr, NULL);
+ mr = ERR_PTR(ret);
+ }
+
out:
trace_mr_alloc(pd, mr_type, max_num_sg, mr);
return mr;
@@ -2191,6 +2218,7 @@ struct ib_mr *ib_alloc_mr_integrity(struct ib_pd *pd,
{
struct ib_mr *mr;
struct ib_sig_attrs *sig_attrs;
+ int ret;
if (!pd->device->ops.alloc_mr_integrity ||
!pd->device->ops.map_mr_sg_pi) {
@@ -2227,7 +2255,12 @@ struct ib_mr *ib_alloc_mr_integrity(struct ib_pd *pd,
rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
rdma_restrack_parent_name(&mr->res, &pd->res);
- rdma_restrack_add(&mr->res);
+ ret = rdma_restrack_add(&mr->res);
+ if (ret) {
+ rdma_restrack_put(&mr->res);
+ pd->device->ops.dereg_mr(mr, NULL);
+ mr = ERR_PTR(ret);
+ }
out:
trace_mr_integ_alloc(pd, max_num_data_sg, max_num_meta_sg, mr);
return mr;
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects
2020-09-07 12:21 ` [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects Leon Romanovsky
@ 2020-09-18 23:31 ` Jason Gunthorpe
2020-09-19 9:09 ` Leon Romanovsky
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-18 23:31 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma, Mark Zhang
On Mon, Sep 07, 2020 at 03:21:53PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Restrack stores the IB objects in the internal xarray and insertion
> where can fail. As long as restrack was helper tool for the
> debuggability, such failure were ignored.
>
> In the following patches, the ib_core will be changed to manage allocated
> IB objects in restrack DB for the proper memory lifetime. It requires to
> ensure that insertion errors are not ignored.
Why? This looks like it is all about removing valid, not sure what the
kref has to do with it..
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects
2020-09-18 23:31 ` Jason Gunthorpe
@ 2020-09-19 9:09 ` Leon Romanovsky
2020-09-24 19:50 ` Jason Gunthorpe
0 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-19 9:09 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma, Mark Zhang
On Fri, Sep 18, 2020 at 08:31:52PM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 07, 2020 at 03:21:53PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Restrack stores the IB objects in the internal xarray and insertion
> > where can fail. As long as restrack was helper tool for the
> > debuggability, such failure were ignored.
> >
> > In the following patches, the ib_core will be changed to manage allocated
> > IB objects in restrack DB for the proper memory lifetime. It requires to
> > ensure that insertion errors are not ignored.
>
> Why? This looks like it is all about removing valid, not sure what the
> kref has to do with it..
This DB is going to be main source of all HW objects and their memory
allocations. We want to be sure that everything there is valid.
Thanks
>
> Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects
2020-09-19 9:09 ` Leon Romanovsky
@ 2020-09-24 19:50 ` Jason Gunthorpe
2020-09-24 19:59 ` Jason Gunthorpe
0 siblings, 1 reply; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-24 19:50 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma, Mark Zhang
On Sat, Sep 19, 2020 at 12:09:28PM +0300, Leon Romanovsky wrote:
> On Fri, Sep 18, 2020 at 08:31:52PM -0300, Jason Gunthorpe wrote:
> > On Mon, Sep 07, 2020 at 03:21:53PM +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >
> > > Restrack stores the IB objects in the internal xarray and insertion
> > > where can fail. As long as restrack was helper tool for the
> > > debuggability, such failure were ignored.
> > >
> > > In the following patches, the ib_core will be changed to manage allocated
> > > IB objects in restrack DB for the proper memory lifetime. It requires to
> > > ensure that insertion errors are not ignored.
> >
> > Why? This looks like it is all about removing valid, not sure what the
> > kref has to do with it..
>
> This DB is going to be main source of all HW objects and their memory
> allocations. We want to be sure that everything there is valid.
Not really, what has happened here is no_track replaces valid. valid
used to mean the entry was in the xarray, now no_track means the same
thing. The patches in between had both because of how the conversion
ended up
This commit message should just explain that valid is no longer needed
and no_track now indicates if the entry is in the xarray or not so
destruction knows what to do.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects
2020-09-24 19:50 ` Jason Gunthorpe
@ 2020-09-24 19:59 ` Jason Gunthorpe
0 siblings, 0 replies; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-24 19:59 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma, Mark Zhang
On Thu, Sep 24, 2020 at 04:50:01PM -0300, Jason Gunthorpe wrote:
> On Sat, Sep 19, 2020 at 12:09:28PM +0300, Leon Romanovsky wrote:
> > On Fri, Sep 18, 2020 at 08:31:52PM -0300, Jason Gunthorpe wrote:
> > > On Mon, Sep 07, 2020 at 03:21:53PM +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@mellanox.com>
> > > >
> > > > Restrack stores the IB objects in the internal xarray and insertion
> > > > where can fail. As long as restrack was helper tool for the
> > > > debuggability, such failure were ignored.
> > > >
> > > > In the following patches, the ib_core will be changed to manage allocated
> > > > IB objects in restrack DB for the proper memory lifetime. It requires to
> > > > ensure that insertion errors are not ignored.
> > >
> > > Why? This looks like it is all about removing valid, not sure what the
> > > kref has to do with it..
> >
> > This DB is going to be main source of all HW objects and their memory
> > allocations. We want to be sure that everything there is valid.
>
> Not really, what has happened here is no_track replaces valid. valid
> used to mean the entry was in the xarray, now no_track means the same
> thing. The patches in between had both because of how the conversion
> ended up
>
> This commit message should just explain that valid is no longer needed
> and no_track now indicates if the entry is in the xarray or not so
> destruction knows what to do.
er I mean the message of patch #14
This commit message should explain that this adds error handling for
the rdma_restrack_add(). As far as I can tell this isn't strictly
required as a failing add could just set no_track and things would
still be fine.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 12/14] RDMA/restrack: Support all QP types
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (10 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 11/14] RDMA/restrack: Make restrack DB mandatory for IB objects Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 13/14] RDMA/core: Track device memory MRs Leon Romanovsky
2020-09-07 12:21 ` [PATCH rdma-next v2 14/14] RDMA/restrack: Drop valid restrack field as source of ambiguity Leon Romanovsky
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma, Mark Zhang
From: Leon Romanovsky <leonro@mellanox.com>
The latest changes in restrack name handling allowed to simplify
the QP creation code to support all types of QPs.
For example XRC QP are presented with inbox rdmatool.
[leonro@vm ~]$ ibv_xsrq_pingpong &
[leonro@vm ~]$ rdma res show qp
link ibp0s9/1 lqpn 0 type SMI state RTS sq-psn 0 comm [ib_core]
link ibp0s9/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
link ibp0s9/1 lqpn 7 type UD state RTS sq-psn 0 comm [mlx5_ib]
link ibp0s9/1 lqpn 42 type XRC_TGT state INIT sq-psn 0 path-mig-state MIGRATED comm [ib_uverbs]
link ibp0s9/1 lqpn 43 type XRC_INI state INIT sq-psn 0 path-mig-state MIGRATED pdn 197 pid 419 comm ibv_xsrq_pingpong
Reviewed-by: Mark Zhang <markz@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/core_priv.h | 29 ++++++-------------
drivers/infiniband/core/uverbs_cmd.c | 4 +--
drivers/infiniband/core/uverbs_std_types_qp.c | 4 +--
drivers/infiniband/core/verbs.c | 11 +++----
include/rdma/ib_verbs.h | 10 +++++--
5 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 7f4bb09efab8..3962a54f9178 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -318,15 +318,12 @@ struct ib_device *ib_device_get_by_index(const struct net *net, u32 index);
void nldev_init(void);
void nldev_exit(void);
-static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
- struct ib_pd *pd,
- struct ib_qp_init_attr *attr,
- struct ib_udata *udata,
- struct ib_uqp_object *uobj)
+static inline struct ib_qp *
+_ib_create_qp(struct ib_device *dev, struct ib_pd *pd,
+ struct ib_qp_init_attr *attr, struct ib_udata *udata,
+ struct ib_uqp_object *uobj, const char *caller)
{
- enum ib_qp_type qp_type = attr->qp_type;
struct ib_qp *qp;
- bool is_xrc;
int ret;
if (!dev->ops.create_qp)
@@ -348,7 +345,6 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
qp->srq = attr->srq;
qp->rwq_ind_tbl = attr->rwq_ind_tbl;
qp->event_handler = attr->event_handler;
- qp->qp_type = attr->qp_type;
qp->port = attr->port_num;
atomic_set(&qp->usecnt, 0);
@@ -357,18 +353,11 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev,
INIT_LIST_HEAD(&qp->sig_mrs);
rdma_restrack_new(&qp->res, RDMA_RESTRACK_QP);
- /*
- * We don't track XRC QPs for now, because they don't have PD
- * and more importantly they are created internaly by driver,
- * see mlx5 create_dev_resources() as an example.
- */
- 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) {
- rdma_restrack_parent_name(&qp->res, &pd->res);
- ret = rdma_restrack_add(&qp->res);
- if (ret)
- goto err;
- }
+ WARN_ONCE(!udata && !caller, "Missing kernel QP owner");
+ rdma_restrack_set_name(&qp->res, udata ? NULL : caller);
+ ret = rdma_restrack_add(&qp->res);
+ if (ret)
+ goto err;
return qp;
err:
rdma_restrack_put(&qp->res);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index d7c532154bbd..432e42b3c931 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1399,8 +1399,8 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
if (cmd->qp_type == IB_QPT_XRC_TGT)
qp = ib_create_qp(pd, &attr);
else
- qp = _ib_create_qp(device, pd, &attr, &attrs->driver_udata,
- obj);
+ qp = _ib_create_qp(device, pd, &attr, &attrs->driver_udata, obj,
+ NULL);
if (IS_ERR(qp)) {
ret = PTR_ERR(qp);
diff --git a/drivers/infiniband/core/uverbs_std_types_qp.c b/drivers/infiniband/core/uverbs_std_types_qp.c
index 3bf8dcdfe7eb..198c3cd6f8b9 100644
--- a/drivers/infiniband/core/uverbs_std_types_qp.c
+++ b/drivers/infiniband/core/uverbs_std_types_qp.c
@@ -251,8 +251,8 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QP_CREATE)(
if (attr.qp_type == IB_QPT_XRC_TGT)
qp = ib_create_qp(pd, &attr);
else
- qp = _ib_create_qp(device, pd, &attr, &attrs->driver_udata,
- obj);
+ qp = _ib_create_qp(device, pd, &attr, &attrs->driver_udata, obj,
+ NULL);
if (IS_ERR(qp)) {
ret = PTR_ERR(qp);
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 7802247c2a67..b3b1805c2e84 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1195,7 +1195,7 @@ static struct ib_qp *create_xrc_qp_user(struct ib_qp *qp,
}
/**
- * ib_create_qp - Creates a kernel QP associated with the specified protection
+ * ib_create_named_qp - Creates a kernel QP associated with the specified protection
* domain.
* @pd: The protection domain associated with the QP.
* @qp_init_attr: A list of initial attributes required to create the
@@ -1204,8 +1204,9 @@ static struct ib_qp *create_xrc_qp_user(struct ib_qp *qp,
*
* NOTE: for user qp use ib_create_qp_user with valid udata!
*/
-struct ib_qp *ib_create_qp(struct ib_pd *pd,
- struct ib_qp_init_attr *qp_init_attr)
+struct ib_qp *ib_create_named_qp(struct ib_pd *pd,
+ struct ib_qp_init_attr *qp_init_attr,
+ const char *caller)
{
struct ib_device *device = pd ? pd->device : qp_init_attr->xrcd->device;
struct ib_qp *qp;
@@ -1230,7 +1231,7 @@ struct ib_qp *ib_create_qp(struct ib_pd *pd,
if (qp_init_attr->cap.max_rdma_ctxs)
rdma_rw_init_qp(device, qp_init_attr);
- qp = _ib_create_qp(device, pd, qp_init_attr, NULL, NULL);
+ qp = _ib_create_qp(device, pd, qp_init_attr, NULL, NULL, caller);
if (IS_ERR(qp))
return qp;
@@ -1296,7 +1297,7 @@ struct ib_qp *ib_create_qp(struct ib_pd *pd,
return ERR_PTR(ret);
}
-EXPORT_SYMBOL(ib_create_qp);
+EXPORT_SYMBOL(ib_create_named_qp);
static const struct {
int valid;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index ad2f8dfa2e66..73fb9f2455d9 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -3684,8 +3684,14 @@ static inline int ib_post_srq_recv(struct ib_srq *srq,
bad_recv_wr ? : &dummy);
}
-struct ib_qp *ib_create_qp(struct ib_pd *pd,
- struct ib_qp_init_attr *qp_init_attr);
+struct ib_qp *ib_create_named_qp(struct ib_pd *pd,
+ struct ib_qp_init_attr *qp_init_attr,
+ const char *caller);
+static inline struct ib_qp *ib_create_qp(struct ib_pd *pd,
+ struct ib_qp_init_attr *init_attr)
+{
+ return ib_create_named_qp(pd, init_attr, KBUILD_MODNAME);
+}
/**
* ib_modify_qp_with_udata - Modifies the attributes for the specified QP.
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH rdma-next v2 13/14] RDMA/core: Track device memory MRs
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (11 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 12/14] RDMA/restrack: Support all QP types Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
2020-09-24 20:02 ` Jason Gunthorpe
2020-09-07 12:21 ` [PATCH rdma-next v2 14/14] RDMA/restrack: Drop valid restrack field as source of ambiguity Leon Romanovsky
13 siblings, 1 reply; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
Device memory (DM) are registered to MR during initialization flow,
these MRs were not tracked by resource tracker and had res->valid set
as a false. Update the code to manage them too.
Before this change:
[leonro@vm ~]$ ibv_rc_pingpong -j &
[leonro@vm ~]$ rdma res show mr <-- shows nothing
After this change:
[leonro@mtl-leonro-l-vm ~]$ ibv_rc_pingpong -j &
[leonro@mtl-leonro-l-vm ~]$ rdma res show mr
dev ibp0s9 mrn 0 mrlen 4096 pdn 3 pid 734 comm ibv_rc_pingpong
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/uverbs_std_types_mr.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/infiniband/core/uverbs_std_types_mr.c b/drivers/infiniband/core/uverbs_std_types_mr.c
index 9b22bb553e8b..9ec5ea49e06e 100644
--- a/drivers/infiniband/core/uverbs_std_types_mr.c
+++ b/drivers/infiniband/core/uverbs_std_types_mr.c
@@ -31,6 +31,7 @@
*/
#include "rdma_core.h"
+#include "restrack.h"
#include "uverbs.h"
#include <rdma/uverbs_std_types.h>
@@ -134,6 +135,15 @@ static int UVERBS_HANDLER(UVERBS_METHOD_DM_MR_REG)(
atomic_inc(&pd->usecnt);
atomic_inc(&dm->usecnt);
+ rdma_restrack_new(&mr->res, RDMA_RESTRACK_MR);
+ rdma_restrack_set_name(&mr->res, NULL);
+ ret = rdma_restrack_add(&mr->res);
+ if (ret) {
+ rdma_restrack_put(&mr->res);
+ pd->device->ops.dereg_mr(mr, &attrs->driver_udata);
+ return ret;
+ }
+
uobj->object = mr;
uverbs_finalize_uobj_create(attrs, UVERBS_ATTR_REG_DM_MR_HANDLE);
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH rdma-next v2 13/14] RDMA/core: Track device memory MRs
2020-09-07 12:21 ` [PATCH rdma-next v2 13/14] RDMA/core: Track device memory MRs Leon Romanovsky
@ 2020-09-24 20:02 ` Jason Gunthorpe
0 siblings, 0 replies; 32+ messages in thread
From: Jason Gunthorpe @ 2020-09-24 20:02 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Leon Romanovsky, linux-rdma
On Mon, Sep 07, 2020 at 03:21:55PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Device memory (DM) are registered to MR during initialization flow,
> these MRs were not tracked by resource tracker and had res->valid set
> as a false. Update the code to manage them too.
>
> Before this change:
> [leonro@vm ~]$ ibv_rc_pingpong -j &
> [leonro@vm ~]$ rdma res show mr <-- shows nothing
>
> After this change:
> [leonro@mtl-leonro-l-vm ~]$ ibv_rc_pingpong -j &
> [leonro@mtl-leonro-l-vm ~]$ rdma res show mr
> dev ibp0s9 mrn 0 mrlen 4096 pdn 3 pid 734 comm ibv_rc_pingpong
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/core/uverbs_std_types_mr.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
Fixes: be934cca9e98 ("IB/uverbs: Add device memory registration ioctl support")
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH rdma-next v2 14/14] RDMA/restrack: Drop valid restrack field as source of ambiguity
2020-09-07 12:21 [PATCH rdma-next v2 00/14] Track memory allocation with restrack DB help Leon Romanovsky
` (12 preceding siblings ...)
2020-09-07 12:21 ` [PATCH rdma-next v2 13/14] RDMA/core: Track device memory MRs Leon Romanovsky
@ 2020-09-07 12:21 ` Leon Romanovsky
13 siblings, 0 replies; 32+ messages in thread
From: Leon Romanovsky @ 2020-09-07 12:21 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma
From: Leon Romanovsky <leonro@mellanox.com>
The valid field was needed to distinguish between supported/not
supported QPs, after the create_qp was changed to support all types,
that field can be dropped and the code simplified a little bit.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/core/restrack.c | 29 ++++++++---------------------
include/rdma/restrack.h | 9 ---------
2 files changed, 8 insertions(+), 30 deletions(-)
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index acf06c29b176..8593e0f082e8 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -143,7 +143,7 @@ static struct ib_device *res_to_dev(struct rdma_restrack_entry *res)
return container_of(res, struct rdma_counter, res)->device;
default:
WARN_ONCE(true, "Wrong resource tracking type %u\n", res->type);
- return NULL;
+ return ERR_PTR(-EINVAL);
}
}
@@ -223,7 +223,7 @@ int __must_check rdma_restrack_add(struct rdma_restrack_entry *res)
struct rdma_restrack_root *rt;
int ret = 0;
- if (!dev)
+ if (IS_ERR_OR_NULL(dev))
return -ENODEV;
if (res->no_track)
@@ -261,10 +261,7 @@ int __must_check rdma_restrack_add(struct rdma_restrack_entry *res)
}
out:
- if (ret)
- return ret;
- res->valid = true;
- return 0;
+ return ret;
}
EXPORT_SYMBOL(rdma_restrack_add);
@@ -323,25 +320,16 @@ EXPORT_SYMBOL(rdma_restrack_put);
*/
void rdma_restrack_del(struct rdma_restrack_entry *res)
{
+ struct ib_device *dev = res_to_dev(res);
struct rdma_restrack_entry *old;
struct rdma_restrack_root *rt;
- struct ib_device *dev;
- if (!res->valid) {
- if (res->task) {
- put_task_struct(res->task);
- res->task = NULL;
- }
- return;
- }
-
- if (res->no_track)
+ WARN_ONCE(!dev && res->type != RDMA_RESTRACK_CM_ID,
+ "IB device should be set for restrack type %s",
+ type2str(res->type));
+ if (res->no_track || IS_ERR_OR_NULL(dev))
goto out;
- dev = res_to_dev(res);
- if (WARN_ON(!dev))
- return;
-
rt = &dev->res[res->type];
old = xa_erase(&rt->xa, res->id);
@@ -350,7 +338,6 @@ void rdma_restrack_del(struct rdma_restrack_entry *res)
WARN_ON(old != res);
out:
- res->valid = false;
rdma_restrack_put(res);
wait_for_completion(&res->comp);
}
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
index d52f7ad6641f..c85db3d6a81e 100644
--- a/include/rdma/restrack.h
+++ b/include/rdma/restrack.h
@@ -59,15 +59,6 @@ enum rdma_restrack_type {
* struct rdma_restrack_entry - metadata per-entry
*/
struct rdma_restrack_entry {
- /**
- * @valid: validity indicator
- *
- * The entries are filled during rdma_restrack_add,
- * can be attempted to be free during rdma_restrack_del.
- *
- * As an example for that, see mlx5 QPs with type MLX5_IB_QPT_HW_GSI
- */
- bool valid;
/**
* @no_track: don't add this entry to restrack DB
*
--
2.26.2
^ permalink raw reply related [flat|nested] 32+ messages in thread