RDMA resource tracking
Subsystems (7)
v2 cover RDMA resource tracking
Changelog:v1->v2:
* Rebased on latest rdma/for-next
* Replaced mutex lock which protect linked lists to be RW semaphore. It has no impact on current implementation, because there is only one reader (nldev) and it is serialized. However better to be prepared for multiple readers from the beginning.
* Added reinitialization next QP entry logic to ensure that it exists and didn't vanish during fill_req_qp() work.v0->v1:
* Dropped RFC
* Separated to thre series, one for for-rc, and two for-next.
-------
The original goal of this series was to allow ability to view connection (QP) information about running processes, however I used this opportunity and created common infrastructure to track and report various resources. The report part is implemented in netlink (nldev), but smart ULPs can now create advanced usage models based on device utilization.
The current implementation relies on one lock per-object per-device, so creation/destroying of various objects (CQ, PD, e.t.c) on various or the same devices doesn't interfere each with another.
The data protection is performed with SRCU and its reader-writer model ensures that resource won't be destroyed till readers will finish their work.
Possible future work will include:
* Reducing number of locks in RDMA, because of SRCU.
* Converting CMA to be based completely on resource tracking.
* Addition of other objects and extending current to give full and detailed state of the RDMA kernel stack.
* Replacing synchronize_srcu with call_srcu to make destroy flow non-blocking.
* Provide reliable device reset flow, preserving resource creation ordering.
The patches are available in the git repository at:
git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-next-2018-01-02
Thanks
---------------------------------------
Leon Romanovsky (7):
RDMA/restrack: Add general infrastructure to track RDMA resources RDMA/core: Add helper function to create named QPs
RDMA: Annotate create QP callers
RDMA/core: Add resource tracking for create and destroy CQs RDMA/core: Add resource tracking for create and destroy PDs RDMA/nldev: Provide global resource utilization RDMA/nldev: Provide detailed QP information
drivers/infiniband/core/Makefile | 2 +- drivers/infiniband/core/cma.c | 1 + drivers/infiniband/core/core_priv.h | 20 ++ drivers/infiniband/core/cq.c | 3 + drivers/infiniband/core/device.c | 7 + drivers/infiniband/core/mad.c | 1 + drivers/infiniband/core/nldev.c | 386 +++++++++++++++++++++++++++++ drivers/infiniband/core/restrack.c | 178 +++++++++++++ drivers/infiniband/core/uverbs_cmd.c | 5 +- drivers/infiniband/core/uverbs_std_types.c | 2 + drivers/infiniband/core/verbs.c | 8 +- drivers/infiniband/hw/mlx4/mad.c | 1 + drivers/infiniband/hw/mlx4/qp.c | 1 + drivers/infiniband/hw/mlx5/gsi.c | 2 + drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 +- drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 1 + drivers/infiniband/ulp/srp/ib_srp.c | 1 + drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + include/rdma/ib_verbs.h | 23 +- include/rdma/restrack.h | 149 +++++++++++ include/uapi/rdma/rdma_netlink.h | 54 ++++ net/smc/smc_ib.c | 1 + 22 files changed, 844 insertions(+), 7 deletions(-) create mode 100644 drivers/infiniband/core/restrack.c create mode 100644 include/rdma/restrack.h
--
2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20180102081832.5264-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
PATCH v2 1/7 RDMA/restrack: Add general infrastructure to track RDMA resources
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The RDMA subsystem has very strict set of objects to work on it, but it completely lacks tracking facilities and no visibility of resource utilization.
The following patch adds such infrastructure to keep track of RDMA resources to help with debugging of user space applications. The primary user of this infrastructure is RDMA nldev netlink (following patches), but it is not limited too.
At this stage, the main three objects (PD, CQ and QP) are added, and more will be added later.
There are four new functions in use by RDMA/core:
* rdma_restrack_init(...) - initializes restrack database * rdma_restrack_clean(...) - cleans restrack database * rdma_restrack_add(...) - adds object to be tracked * rdma_restrack_del(...) - removes object from tracking
3 functions and one iterator visible to kernel users:
* rdma_restrack_count(...) - returns number of allocated objects of specific type
* rdma_restrack_lock(...) - Lock primitive to protect access to list of resources
* rdma_restrack_unlock(...)- Unlock primitive to protect access to list of resources
* for_each_res_safe(...) - iterates over all relevant objects in
the restrack database.
--- drivers/infiniband/core/Makefile | 2 +- drivers/infiniband/core/core_priv.h | 1 + drivers/infiniband/core/device.c | 7 ++ drivers/infiniband/core/restrack.c | 178 ++++++++++++++++++++++++++++++++++++ include/rdma/ib_verbs.h | 17 +++- include/rdma/restrack.h | 149 ++++++++++++++++++++++++++++++ 6 files changed, 352 insertions(+), 2 deletions(-) create mode 100644 drivers/infiniband/core/restrack.c create mode 100644 include/rdma/restrack.h
drivers/infiniband/core/Makefile +1 -1
@@ -12,7 +12,7 @@ ib_core-y := packer.o ud_header.o verbs.o cq.o rw.o sysfs.o \ device.o fmr_pool.o cache.o netlink.o \ roce_gid_mgmt.o mr_pool.o addr.o sa_query.o \ multicast.o mad.o smi.o agent.o mad_rmpp.o \ - security.o nldev.o + security.o nldev.o restrack.o ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o
drivers/infiniband/core/core_priv.h +1 -0
@@ -40,6 +40,7 @@#include <rdma/ib_verbs.h> #include <rdma/opa_addr.h> #include <rdma/ib_mad.h> +#include <rdma/restrack.h> #include "mad_priv.h" struct pkey_index_qp_list {
drivers/infiniband/core/device.c +7 -0
@@ -266,6 +266,11 @@ struct ib_device *ib_alloc_device(size_t size)if (!device)return NULL ; +if (rdma_restrack_init (&device->res)) { +kfree (device); +return NULL ; + } + device->dev.class = &ib_class;device_initialize (&device->dev);
@@ -591,6 +596,8 @@ void ib_unregister_device(struct ib_device *device) }up_read (&lists_rwsem); +rdma_restrack_clean (&device->res); +ib_device_unregister_rdmacg (device);ib_device_unregister_sysfs (device);
drivers/infiniband/core/restrack.c new file +178 -0
178 lines not on this page · raw message
include/rdma/ib_verbs.h +16 -1
@@ -63,6 +63,7 @@#include <linux/uaccess.h> #include <linux/cgroup_rdma.h> #include <uapi/rdma/ib_user_verbs.h> +#include <rdma/restrack.h> #define IB_FW_VERSION_NAME_MAX ETHTOOL_FWVERS_LEN
@@ -1526,9 +1527,10 @@ struct ib_pd {u32 unsafe_global_rkey;/* -* Implementation details of the RDMA core, don't use in drivers: +* Implementation details of the RDMA core, don't use in the drivers */ struct ib_mr *__internal_mr; +struct rdma_restrack_entry res; };struct ib_xrcd {
@@ -1569,6 +1571,10 @@ struct ib_cq {struct irq_poll iop;struct work_struct work; }; +/* +* Internal to RDMA/core, don't use in the drivers +*/ +struct rdma_restrack_entry res; };struct ib_srq {
@@ -1745,6 +1751,11 @@ struct ib_qp {struct ib_rwq_ind_table *rwq_ind_tbl;struct ib_qp_security *qp_sec;u8 port; + +/* +* Internal to RDMA/core, don't use in the drivers +*/ +struct rdma_restrack_entry res; };struct ib_mr {
@@ -2351,6 +2362,10 @@ struct ib_device {#endif u32 index; +/* +* Implementation details of the RDMA core, don't use in the drivers +*/ +struct rdma_restrack_root res;/** * The following mandatory functions are used only at device
include/rdma/restrack.h new file +149 -0
149 lines not on this page · raw message
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 2/7 RDMA/core: Add helper function to create named QPs
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The QPs in the RDMA stack can be created by kernel or users space, but the owner is not visible to users after that.
The added helper keeps track of newly created QP together with the name of the owner. In case of kernel, the caller to create_qp is supposed to update QP's attribute with the name. For user space callers no change is needed and the name will be taken from the process name.
This helper sets qp->device field for all QP types including XRC_TGT, which RDMA/core didn't do before.
--- drivers/infiniband/core/core_priv.h | 19 +++++++++++++++++++ include/rdma/ib_verbs.h | 6 ++++++ 2 files changed, 25 insertions(+)
drivers/infiniband/core/core_priv.h +19 -0
@@ -299,4 +299,23 @@ struct ib_device *ib_device_get_by_index(u32 ifindex);/* RDMA device netlink */ 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_qp *qp; + + qp = dev->create_qp (pd, attr, udata); +if (!IS_ERR (qp)) { + qp->device = dev; +if (attr->qp_type <IB_QPT_MAX ) +rdma_restrack_add (&qp->res, +RDMA_RESTRACK_QP , + attr->comm); + } + +return qp; +}#endif /* _CORE_PRIV_H */
include/rdma/ib_verbs.h +6 -0
@@ -1140,6 +1140,12 @@ struct ib_qp_init_attr {u8 port_num;struct ib_rwq_ind_table *rwq_ind_tbl;u32 source_qpn; + +/* +* Name of entity which created this QP, empty string means that +* it will be taken automatically from task_struct. +*/ +char comm[TASK_COMM_LEN ]; };struct ib_qp_open_attr {
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 3/7 RDMA: Annotate create QP callers
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Update all callers to provide owner name through QP attribute structure and connect create_qp with helper which supports resource tracking.
--- drivers/infiniband/core/cma.c | 1 + drivers/infiniband/core/mad.c | 1 + drivers/infiniband/core/uverbs_cmd.c | 3 +-- drivers/infiniband/core/verbs.c | 4 ++-- drivers/infiniband/hw/mlx4/mad.c | 1 + drivers/infiniband/hw/mlx4/qp.c | 1 + drivers/infiniband/hw/mlx5/gsi.c | 2 ++ drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 +++- drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 1 + drivers/infiniband/ulp/srp/ib_srp.c | 1 + drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + net/smc/smc_ib.c | 1 + 12 files changed, 16 insertions(+), 5 deletions(-)
drivers/infiniband/core/cma.c +1 -0
@@ -858,6 +858,7 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd,return -EINVAL ; qp_init_attr->port_num = id->port_num; +strncpy (qp_init_attr->comm,"rdma-cm" ,TASK_COMM_LEN ); qp =ib_create_qp (pd, qp_init_attr);if (IS_ERR (qp))return PTR_ERR (qp);
drivers/infiniband/core/mad.c +1 -0
@@ -3103,6 +3103,7 @@ static int create_mad_qp(struct ib_mad_qp_info *qp_info, qp_init_attr.port_num = qp_info->port_priv->port_num; qp_init_attr.qp_context = qp_info; qp_init_attr.event_handler = qp_event_handler; +strncpy (qp_init_attr.comm,"rdma-mad" ,TASK_COMM_LEN ); qp_info->qp =ib_create_qp (qp_info->port_priv->pd, &qp_init_attr);if (IS_ERR (qp_info->qp)) { dev_err(&qp_info->port_priv->device->dev,
drivers/infiniband/core/uverbs_cmd.c +1 -2
@@ -1514,7 +1514,7 @@ static int create_qp(struct ib_uverbs_file *file,if (cmd->qp_type ==IB_QPT_XRC_TGT ) qp =ib_create_qp (pd, &attr);else - qp = device->create_qp (pd, &attr, uhw); + qp =_ib_create_qp (device, pd, &attr, uhw);if (IS_ERR (qp)) { ret =PTR_ERR (qp);
@@ -1527,7 +1527,6 @@ static int create_qp(struct ib_uverbs_file *file, goto err_cb; qp->real_qp = qp; - qp->device = device; qp->pd = pd; qp->send_cq = attr.send_cq; qp->recv_cq = attr.recv_cq;
drivers/infiniband/core/verbs.c +2 -2
@@ -866,7 +866,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 = device->create_qp (pd, qp_init_attr,NULL ); + qp =_ib_create_qp (device, pd, qp_init_attr,NULL );if (IS_ERR (qp))return qp;
@@ -876,7 +876,6 @@ struct ib_qp *ib_create_qp(struct ib_pd *pd,return ERR_PTR (ret); } - qp->device = device; qp->real_qp = qp; qp->uobject =NULL ; qp->qp_type = qp_init_attr->qp_type;
@@ -1504,6 +1503,7 @@ int ib_destroy_qp(struct ib_qp *qp)if (!qp->uobject)rdma_rw_cleanup_mrs (qp); +rdma_restrack_del (&qp->res,RDMA_RESTRACK_QP ); ret = qp->device->destroy_qp (qp);if (!ret) {if (pd)
drivers/infiniband/hw/mlx4/mad.c +1 -0
@@ -1834,6 +1834,7 @@ static int create_pv_sqp(struct mlx4_ib_demux_pv_ctx *ctx, 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; +strncpy (qp_init_attr.init_attr.comm,"mlx4-sriov" ,TASK_COMM_LEN ); tun_qp->qp =ib_create_qp (ctx->pd, &qp_init_attr.init_attr);if (IS_ERR (tun_qp->qp)) { ret =PTR_ERR (tun_qp->qp);
drivers/infiniband/hw/mlx4/qp.c +1 -0
@@ -1689,6 +1689,7 @@ struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,if (is_eth && dev->dev->caps.flags2 &MLX4_DEV_CAP_FLAG2_ROCE_V1_V2 ) { init_attr->create_flags |=MLX4_IB_QP_CREATE_ROCE_V2_GSI ; +strncpy (init_attr->comm,"mlx4-gsi" ,TASK_COMM_LEN ); sqp->roce_v2_gsi =ib_create_qp (pd, init_attr);if (IS_ERR (sqp->roce_v2_gsi)) {
drivers/infiniband/hw/mlx5/gsi.c +2 -0
@@ -184,6 +184,7 @@ 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 ; } +strncpy (hw_init_attr.comm,"mlx5-gsi" ,TASK_COMM_LEN ); gsi->rx_qp =ib_create_qp (pd, &hw_init_attr);if (IS_ERR (gsi->rx_qp)) { mlx5_ib_warn(dev, "unable to create hardware GSI QP. error %ld\n",
@@ -264,6 +265,7 @@ static struct ib_qp *create_gsi_ud_qp(struct mlx5_ib_gsi_qp *gsi) .sq_sig_type = gsi->sq_sig_type, .qp_type = IB_QPT_UD, .create_flags = mlx5_ib_create_qp_sqpn_qp1(), + .comm = "mlx5-gsi", };return ib_create_qp (pd, &init_attr);
drivers/infiniband/ulp/ipoib/ipoib_cm.c +3 -1
@@ -263,6 +263,7 @@ static struct ib_qp *ipoib_cm_create_rx_qp(struct net_device *dev, .sq_sig_type = IB_SIGNAL_ALL_WR, .qp_type = IB_QPT_RC, .qp_context = p, + .comm = "ipoib-cm", };if (!ipoib_cm_has_srq (dev)) {
@@ -1060,7 +1061,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_ .sq_sig_type = IB_SIGNAL_ALL_WR, .qp_type = IB_QPT_RC, .qp_context = tx, - .create_flags = 0 + .create_flags = 0, + .comm = "ipoib-cm", };struct ib_qp *tx_qp;
drivers/infiniband/ulp/ipoib/ipoib_verbs.c +1 -0
@@ -206,6 +206,7 @@ int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca)if (priv->hca_caps &IB_DEVICE_MANAGED_FLOW_STEERING ) init_attr.create_flags |=IB_QP_CREATE_NETIF_QP ; +strncpy (init_attr.comm,"ipoib-verbs" ,TASK_COMM_LEN ); priv->qp =ib_create_qp (priv->pd, &init_attr);if (IS_ERR (priv->qp)) {pr_warn ("%s: failed to create QP\n" , ca->name);
drivers/infiniband/ulp/srp/ib_srp.c +1 -0
@@ -521,6 +521,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch) init_attr->send_cq = send_cq; init_attr->recv_cq = recv_cq; +strncpy (init_attr->comm,"srp" ,TASK_COMM_LEN ); qp =ib_create_qp (dev->pd, init_attr);if (IS_ERR (qp)) { ret =PTR_ERR (qp);
drivers/infiniband/ulp/srpt/ib_srpt.c +1 -0
@@ -1673,6 +1673,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch) qp_init->cap.max_recv_sge = qp_init->cap.max_send_sge; } +strncpy (qp_init->comm,"srpt" ,TASK_COMM_LEN ); ch->qp =ib_create_qp (sdev->pd, qp_init);if (IS_ERR (ch->qp)) { ret =PTR_ERR (ch->qp);
net/smc/smc_ib.c +1 -0
@@ -243,6 +243,7 @@ int smc_ib_create_queue_pair(struct smc_link *lnk) }, .sq_sig_type = IB_SIGNAL_REQ_WR, .qp_type = IB_QPT_RC, + .comm = "sec-ib", };int rc;
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 4/7 RDMA/core: Add resource tracking for create and destroy CQs
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Track all creation and destroy of CQ objects.
--- drivers/infiniband/core/cq.c | 3 +++ drivers/infiniband/core/uverbs_cmd.c | 1 + drivers/infiniband/core/uverbs_std_types.c | 2 ++ drivers/infiniband/core/verbs.c | 2 ++ 4 files changed, 8 insertions(+)
drivers/infiniband/core/cq.c +3 -0
@@ -154,6 +154,8 @@ struct ib_cq *ib_alloc_cq(struct ib_device *dev, void *private,if (!cq->wc) goto out_destroy_cq; +rdma_restrack_add (&cq->res,RDMA_RESTRACK_CQ ,NULL ); +switch (cq->poll_ctx) {case IB_POLL_DIRECT : cq->comp_handler = ib_cq_completion_direct;
@@ -208,6 +210,7 @@ void ib_free_cq(struct ib_cq *cq)WARN_ON_ONCE (1 ); } +rdma_restrack_del (&cq->res,RDMA_RESTRACK_CQ );kfree (cq->wc); ret = cq->device->destroy_cq (cq);WARN_ON_ONCE (ret);
drivers/infiniband/core/uverbs_cmd.c +1 -0
@@ -1033,6 +1033,7 @@ static struct ib_ucq_object *create_cq(struct ib_uverbs_file *file, goto err_cb;uobj_alloc_commit (&obj->uobject); +rdma_restrack_add (&cq->res,RDMA_RESTRACK_CQ ,NULL );return obj;
drivers/infiniband/core/uverbs_std_types.c +2 -0
@@ -35,6 +35,7 @@#include <rdma/ib_verbs.h> #include <linux/bug.h> #include <linux/file.h> +#include <rdma/restrack.h> #include "rdma_core.h" #include "uverbs.h"
@@ -319,6 +320,7 @@ static int uverbs_create_cq_handler(struct ib_device *ib_dev, obj->uobject.object = cq; obj->uobject.user_handle = user_handle;atomic_set (&cq->usecnt,0 ); +rdma_restrack_add (&cq->res,RDMA_RESTRACK_CQ ,NULL ); ret =uverbs_copy_to (attrs,CREATE_CQ_RESP_CQE , &cq->cqe);if (ret)
drivers/infiniband/core/verbs.c +2 -0
@@ -1546,6 +1546,7 @@ 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 ); +rdma_restrack_add (&cq->res,RDMA_RESTRACK_CQ ,NULL ); }return cq;
@@ -1564,6 +1565,7 @@ int ib_destroy_cq(struct ib_cq *cq)if (atomic_read (&cq->usecnt))return -EBUSY ; +rdma_restrack_del (&cq->res,RDMA_RESTRACK_CQ );return cq->device->destroy_cq (cq); }EXPORT_SYMBOL (ib_destroy_cq);
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 5/7 RDMA/core: Add resource tracking for create and destroy PDs
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Track all creation and destroy of PD objects.
--- drivers/infiniband/core/uverbs_cmd.c | 1 + drivers/infiniband/core/verbs.c | 2 ++ 2 files changed, 3 insertions(+)
drivers/infiniband/core/uverbs_cmd.c +1 -0
@@ -340,6 +340,7 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file, uobj->object = pd;memset (&resp,0 ,sizeof resp); resp.pd_handle = uobj->id; +rdma_restrack_add (&pd->res,RDMA_RESTRACK_PD ,NULL );if (copy_to_user (u64_to_user_ptr (cmd.response), &resp,sizeof resp)) { ret = -EFAULT ;
drivers/infiniband/core/verbs.c +2 -0
@@ -246,6 +246,7 @@ struct ib_pd *__ib_alloc_pd(struct ib_device *device, unsigned int flags,pr_warn ("%s: enabling unsafe global rkey\n" , caller); mr_access_flags |=IB_ACCESS_REMOTE_READ |IB_ACCESS_REMOTE_WRITE ; } +rdma_restrack_add (&pd->res,RDMA_RESTRACK_PD ,NULL );if (mr_access_flags) {struct ib_mr *mr;
@@ -296,6 +297,7 @@ void ib_dealloc_pd(struct ib_pd *pd)requires the caller to guarantee we can't race here. */ WARN_ON (atomic_read (&pd->usecnt)); +rdma_restrack_del (&pd->res,RDMA_RESTRACK_PD );/* Making delalloc_pd a void return is a WIP, no driver should return an error here. */ ret = pd->device->dealloc_pd (pd);
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 6/7 RDMA/nldev: Provide global resource utilization
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Export through netlink interface, the global device utilization for the rdmatool as the main user of RDMA nldev interface.
Provide both dumpit and doit callbacks.
As an example of possible output from rdmatool for system with 5 Mellanox's card
$ rdma res 1: mlx5_0: curr/max: qp 4/262144 cq 5/16777216 pd 3/16777216 2: mlx5_1: curr/max: qp 4/262144 cq 5/16777216 pd 3/16777216 3: mlx5_2: curr/max: qp 4/262144 cq 5/16777216 pd 3/16777216 4: mlx5_3: curr/max: qp 2/262144 cq 3/16777216 pd 2/16777216 5: mlx5_4: curr/max: qp 4/262144 cq 5/16777216 pd 3/16777216
--- drivers/infiniband/core/nldev.c | 165 +++++++++++++++++++++++++++++++++++++++ include/uapi/rdma/rdma_netlink.h | 11 +++ 2 files changed, 176 insertions(+)
drivers/infiniband/core/nldev.c +165 -0
@@ -52,6 +52,12 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { [RDMA_NLDEV_ATTR_PORT_STATE] = { .type = NLA_U8 }, [RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = { .type = NLA_U8 }, [RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_RES_SUMMARY] = { .type = NLA_NESTED }, + [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY] = { .type = NLA_NESTED }, + [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME ] = { .type =NLA_NUL_STRING , + .len = 16 }, + [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR] = { .type = NLA_U64 }, + [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX] = { .type = NLA_U64 }, };static int fill_nldev_handle (struct sk_buff *msg,struct ib_device *device)
@@ -134,6 +140,79 @@ static int fill_port_info(struct sk_buff *msg,return 0 ; } +static int fill_res_info_entry (struct sk_buff *msg, +const char *name,u64 curr,u64 max) +{ +struct nlattr *entry_attr; + + entry_attr =nla_nest_start (msg,RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY ); +if (!entry_attr) +return -EMSGSIZE ; + +if (nla_put_string (msg,RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME , name)) + goto err; +if (nla_put_u64_64bit (msg, +RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR , curr,0 )) + goto err; +if (nla_put_u64_64bit (msg, +RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX , max,0 )) + goto err; + +nla_nest_end (msg, entry_attr); +return 0 ; + +err: +nla_nest_cancel (msg, entry_attr); +return -EMSGSIZE ; +} + +static u32 get_res_max (struct ib_device *dev,int idx) +{ +switch (idx) { +case RDMA_RESTRACK_PD :return dev->attrs.max_pd; +case RDMA_RESTRACK_CQ :return dev->attrs.max_cq; +case RDMA_RESTRACK_QP :return dev->attrs.max_qp; +default :return 0 ;/* unreachable */ + } +} + +static int fill_res_info (struct sk_buff *msg,struct ib_device *device) +{ +static const char *names[_RDMA_RESTRACK_MAX] = { + [RDMA_RESTRACK_PD ] ="pd" , + [RDMA_RESTRACK_CQ ] ="cq" , + [RDMA_RESTRACK_QP ] ="qp" , + }; + +struct rdma_restrack_root *res = &device->res; +struct nlattr *table_attr; +int ret, i; + +if (fill_nldev_handle (msg, device)) +return -EMSGSIZE ; + + table_attr =nla_nest_start (msg,RDMA_NLDEV_ATTR_RES_SUMMARY ); +if (!table_attr) +return -EMSGSIZE ; + +for (i =0 ; i < _RDMA_RESTRACK_MAX; i++) { +if (!names[i]) +continue ; + ret =fill_res_info_entry (msg, names[i], +rdma_restrack_count (res, i), +get_res_max (device, i)); +if (ret) + goto err; + } + +nla_nest_end (msg, table_attr); +return 0 ; + +err: +nla_nest_cancel (msg, table_attr); +return ret; +} +static int nldev_get_doit (struct sk_buff *skb,struct nlmsghdr *nlh,struct netlink_ext_ack *extack) {
@@ -325,6 +404,88 @@ static int nldev_port_get_dumpit(struct sk_buff *skb,return skb->len; } +static int nldev_res_get_doit (struct sk_buff *skb,struct nlmsghdr *nlh, +struct netlink_ext_ack *extack) +{ +struct nlattr *tb[RDMA_NLDEV_ATTR_MAX ]; +struct ib_device *device; +struct sk_buff *msg; +u32 index; +int ret = -ENOMEM ; + + ret =nlmsg_parse (nlh,0 , tb,RDMA_NLDEV_ATTR_MAX -1 , + nldev_policy, extack); +if (ret || !tb[RDMA_NLDEV_ATTR_DEV_INDEX ]) +return -EINVAL ; + + index =nla_get_u32 (tb[RDMA_NLDEV_ATTR_DEV_INDEX ]); + device =ib_device_get_by_index (index); +if (!device) +return -EINVAL ; + + msg =nlmsg_new (NLMSG_DEFAULT_SIZE ,GFP_KERNEL ); +if (!msg) + goto err; + + nlh =nlmsg_put (msg,NETLINK_CB (skb).portid, nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV ,RDMA_NLDEV_CMD_RES_GET ), +0 ,0 ); + + ret =fill_res_info (msg, device); +if (ret) + goto err_free; + +nlmsg_end (msg, nlh); +put_device (&device->dev); + +return rdma_nl_unicast (msg,NETLINK_CB (skb).portid); + +err_free: +nlmsg_free (msg); +err: +put_device (&device->dev); +return ret; +} + +static int _nldev_res_get_dumpit (struct ib_device *device, +struct sk_buff *skb, +struct netlink_callback *cb, +unsigned int idx) +{ +int start = cb->args[0 ]; +struct nlmsghdr *nlh; + +if (idx < start) +return 0 ; + + nlh =nlmsg_put (skb,NETLINK_CB (cb->skb).portid, cb->nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV ,RDMA_NLDEV_CMD_RES_GET ), +0 ,NLM_F_MULTI ); + +if (fill_res_info (skb, device)) { +nlmsg_cancel (skb, nlh); + goto out; + } + +nlmsg_end (skb, nlh); + + idx++; + +out: + cb->args[0 ] = idx; +return skb->len; +} + +static int nldev_res_get_dumpit (struct sk_buff *skb, +struct netlink_callback *cb) +{ +/* +* There is no need to take lock, because +* we are relying on ib_core's lists_rwsem +*/ +return ib_enum_all_devs (_nldev_res_get_dumpit, skb, cb); +} +static const struct rdma_nl_cbs nldev_cb_table[RDMA_NLDEV_NUM_OPS ] = { [RDMA_NLDEV_CMD_GET ] = { .doit = nldev_get_doit,
@@ -334,6 +495,10 @@ static const struct rdma_nl_cbs nldev_cb_table[RDMA_NLDEV_NUM_OPS] = { .doit = nldev_port_get_doit, .dump = nldev_port_get_dumpit, }, + [RDMA_NLDEV_CMD_RES_GET] = { + .doit = nldev_res_get_doit, + .dump = nldev_res_get_dumpit, + }, };void __init nldev_init (void )
include/uapi/rdma/rdma_netlink.h +11 -0
@@ -236,6 +236,11 @@ enum rdma_nldev_command {RDMA_NLDEV_CMD_PORT_NEW ,RDMA_NLDEV_CMD_PORT_DEL , +RDMA_NLDEV_CMD_RES_GET ,/* can dump */ +RDMA_NLDEV_CMD_RES_SET , +RDMA_NLDEV_CMD_RES_NEW , +RDMA_NLDEV_CMD_RES_DEL , + RDMA_NLDEV_NUM_OPS };
@@ -303,6 +308,12 @@ enum rdma_nldev_attr {RDMA_NLDEV_ATTR_DEV_NODE_TYPE ,/* u8 */ +RDMA_NLDEV_ATTR_RES_SUMMARY ,/* nested table */ +RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY ,/* nested table */ +RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME ,/* string */ +RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR ,/* u64 */ +RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX ,/* u64 */ +RDMA_NLDEV_ATTR_MAX };#endif /* _UAPI_RDMA_NETLINK_H */
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 7/7 RDMA/nldev: Provide detailed QP information
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Implement RDMA nldev netlink interface to get detailed QP information.
Currently only dumpit variant is implemented.
--- drivers/infiniband/core/nldev.c | 221 +++++++++++++++++++++++++++++++++++++++ include/uapi/rdma/rdma_netlink.h | 43 ++++++++ 2 files changed, 264 insertions(+)
drivers/infiniband/core/nldev.c +221 -0
@@ -58,6 +58,18 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = { .len = 16 }, [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR] = { .type = NLA_U64 }, [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX] = { .type = NLA_U64 }, + [RDMA_NLDEV_ATTR_RES_QP] = { .type = NLA_NESTED }, + [RDMA_NLDEV_ATTR_RES_QP_ENTRY] = { .type = NLA_NESTED }, + [RDMA_NLDEV_ATTR_RES_LQPN] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_RES_RQPN] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_RES_RQ_PSN] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_RES_SQ_PSN] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_RES_TYPE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_RES_STATE] = { .type = NLA_U8 }, + [RDMA_NLDEV_ATTR_RES_PID] = { .type = NLA_U32 }, + [RDMA_NLDEV_ATTR_RES_PID_COMM] = { .type = NLA_NUL_STRING, + .len = TASK_COMM_LEN }, };static int fill_nldev_handle (struct sk_buff *msg,struct ib_device *device)
@@ -213,6 +225,74 @@ static int fill_res_info(struct sk_buff *msg, struct ib_device *device)return ret; } +static int fill_res_qp_entry (struct sk_buff *msg, +struct ib_qp *qp,uint32_t port) +{ +struct ib_qp_init_attr qp_init_attr; +struct nlattr *entry_attr; +struct ib_qp_attr qp_attr; +int ret; + + ret =ib_query_qp (qp, &qp_attr,0 , &qp_init_attr); +if (ret) +return ret; + +if (port && port != qp_attr.port_num) +return 0 ; + + entry_attr =nla_nest_start (msg,RDMA_NLDEV_ATTR_RES_QP_ENTRY ); +if (!entry_attr) + goto out; + +/* In create_qp() port is not set yet */ +if (qp_attr.port_num && +nla_put_u32 (msg,RDMA_NLDEV_ATTR_PORT_INDEX , qp_attr.port_num)) + goto err; + +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_LQPN , qp->qp_num)) + goto err; +if (qp->qp_type ==IB_QPT_RC || qp->qp_type ==IB_QPT_UC ) { +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_RQPN , + qp_attr.dest_qp_num)) + goto err; +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_RQ_PSN , + qp_attr.rq_psn)) + goto err; + } + +if (nla_put_u32 (msg,RDMA_NLDEV_ATTR_RES_SQ_PSN , qp_attr.sq_psn)) + goto err; + +if (qp->qp_type ==IB_QPT_RC || qp->qp_type ==IB_QPT_UC || + qp->qp_type ==IB_QPT_XRC_INI || qp->qp_type ==IB_QPT_XRC_TGT ) { +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_RES_PATH_MIG_STATE , + qp_attr.path_mig_state)) + goto err; + } +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_RES_TYPE , qp->qp_type)) + goto err; +if (nla_put_u8 (msg,RDMA_NLDEV_ATTR_RES_STATE , qp_attr.qp_state)) + goto err; + +/* PID == 0 means that this QP was created by kernel */ +if (qp->res.pid &&nla_put_u32 (msg, +RDMA_NLDEV_ATTR_RES_PID , qp->res.pid)) + goto err; + +if (nla_put_string (msg, +RDMA_NLDEV_ATTR_RES_PID_COMM , qp->res.task_comm)) + goto err; + +nla_nest_end (msg, entry_attr); + +return 0 ; + +err: +nla_nest_cancel (msg, entry_attr); +out: +return -EMSGSIZE ; +} +static int nldev_get_doit (struct sk_buff *skb,struct nlmsghdr *nlh,struct netlink_ext_ack *extack) {
@@ -486,6 +566,134 @@ static int nldev_res_get_dumpit(struct sk_buff *skb,return ib_enum_all_devs (_nldev_res_get_dumpit, skb, cb); } +static int nldev_res_get_qp_dumpit (struct sk_buff *skb, +struct netlink_callback *cb) +{ +struct nlattr *tb[RDMA_NLDEV_ATTR_MAX ]; +struct rdma_restrack_entry *res, *nxt; +int err, ret, key, idx =0 ; +struct nlattr *table_attr; +struct ib_device *device; +int start = cb->args[0 ]; +struct nlmsghdr *nlh; +u32 index, port =0 ; +struct ib_qp *qp; + + err =nlmsg_parse (cb->nlh,0 , tb,RDMA_NLDEV_ATTR_MAX -1 , + nldev_policy,NULL ); +/* +* Right now, we are expecting the device index to get QP information, +* but it is possible to extend this code to return all devices in +* one shot by checking the existence of RDMA_NLDEV_ATTR_DEV_INDEX. +* if it doesn't exist, we will iterate over all devices. +* +* But it is not needed for now. +*/ +if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX ]) +return -EINVAL ; + + index =nla_get_u32 (tb[RDMA_NLDEV_ATTR_DEV_INDEX ]); + device =ib_device_get_by_index (index); +if (!device) +return -EINVAL ; + +/* +* If no PORT_INDEX is supplied, we will return QPs from whole device +*/ +if (tb[RDMA_NLDEV_ATTR_PORT_INDEX ]) { + port =nla_get_u32 (tb[RDMA_NLDEV_ATTR_PORT_INDEX ]); +if (!rdma_is_port_valid (device, port)) { + ret = -EINVAL ; + goto err_index; + } + } + + nlh =nlmsg_put (skb,NETLINK_CB (cb->skb).portid, cb->nlh->nlmsg_seq, +RDMA_NL_GET_TYPE (RDMA_NL_NLDEV ,RDMA_NLDEV_CMD_RES_QP_GET ), +0 ,NLM_F_MULTI ); + +if (fill_nldev_handle (skb, device)) { + ret = -EMSGSIZE ; + goto err; + } + + table_attr =nla_nest_start (skb,RDMA_NLDEV_ATTR_RES_QP ); +if (!table_attr) { + ret = -EMSGSIZE ; + goto err; + } + +rdma_restrack_lock (&device->res,RDMA_RESTRACK_QP ); +for_each_res_safe (res, nxt,RDMA_RESTRACK_QP , device) { +if (idx < start) { + idx++; +continue ; + } + +if (!res->valid) +/* +* It can be if resource failed to initialize srcu, +* in other cases internal to restrack lock will esnure +* that this list has only valid entries. +*/ +continue ; + + key =srcu_read_lock (&res->srcu); + + qp = container_of(res, struct ib_qp, res); +/* +* We are releasing the object list lock to ensure that +* object creation/destroy doesn't can progress while +* we are getting information about QP. +* +* It is needed to prevent writer starvation, because of +* large number of reources in object list. +*/ +rdma_restrack_unlock (&device->res,RDMA_RESTRACK_QP ); + ret =fill_res_qp_entry (skb, qp, port); +rdma_restrack_lock (&device->res,RDMA_RESTRACK_QP ); + +/* +* There is a need to ensure that next QP is valid, +* because we dropped the lock protecting our linked list. +* +* The RCU protection of "res" ensures that it is safe. +*/ +list_safe_reset_next (res, nxt, list); + +srcu_read_unlock (&res->srcu, key); + +if (ret == -EMSGSIZE ) +/* +* There is a chance to optimize here. +* It can be done by using list_prepare_entry +* and list_for_each_entry_continue afterwards. +*/ +break ; +if (ret) + goto res_err; + idx++; + } +rdma_restrack_unlock (&device->res,RDMA_RESTRACK_QP ); + +nla_nest_end (skb, table_attr); +nlmsg_end (skb, nlh); + cb->args[0 ] = idx; +put_device (&device->dev); +return 0 ; + +res_err: +nla_nest_cancel (skb, table_attr); +rdma_restrack_unlock (&device->res,RDMA_RESTRACK_QP ); + +err: +nlmsg_cancel (skb, nlh); + +err_index: +put_device (&device->dev); +return ret; +} +static const struct rdma_nl_cbs nldev_cb_table[RDMA_NLDEV_NUM_OPS ] = { [RDMA_NLDEV_CMD_GET ] = { .doit = nldev_get_doit,
@@ -499,6 +707,19 @@ static const struct rdma_nl_cbs nldev_cb_table[RDMA_NLDEV_NUM_OPS] = { .doit = nldev_res_get_doit, .dump = nldev_res_get_dumpit, }, + [RDMA_NLDEV_CMD_RES_QP_GET] = { + .dump = nldev_res_get_qp_dumpit, + /* + * .doit is not implemented yet for two reasons: + * 1. It is not needed yet. + * 2. There is a need to provide identifier, while it is easy + * for the QPs (device index + port index + LQPN), it is not + * the case for the rest of resources (PD and CQ). Because it + * is better to provide similar interface for all resources, + * let's wait till we will have other resources implemented + * too. + */ + }, };void __init nldev_init (void )
include/uapi/rdma/rdma_netlink.h +43 -0
55 lines not on this page · raw message
Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20180102081832.5264-8-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
drivers/infiniband/core/nldev.c static int fill_res_info(struct sk_buff *msg, struct ib_device *device) · line 279
Quoted 3 lines · Leon Romanovsky
drivers/infiniband/core/nldev.c +3
-0+/* PID == 0 means that this QP was created by kernel */ +if (qp->res.pid &&nla_put_u32 (msg, +RDMA_NLDEV_ATTR_RES_PID , qp->res.pid))
This is returning a pid in the init name space, obtained here:
+ res->pid = task_pid_nr(current);
And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
This API also needs to filter the results and only return pids visible, and translate the pids as well..
I also suspsect this needs to be a netlink array of pids for future, as we have hope someday to have RDMA uobjects shared between multiple processes?
drivers/infiniband/core/nldev.c static int fill_res_info(struct sk_buff *msg, struct ib_device *device) · line 284
Quoted 3 lines
drivers/infiniband/core/nldev.c +3
-0+if (nla_put_string (msg, +RDMA_NLDEV_ATTR_RES_PID_COMM , qp->res.task_comm)) + goto err;
Feels odd to return the content of /proc/XX/comm in netlink?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20180109200917.GF4518-uk2M96/[email protected]
RE: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
Quoted 24 lines
-----Original Message-----
From: [email protected] [mailto:linux-rdma- [email protected]] On Behalf Of Jason Gunthorpe
Sent: Tuesday, January 09, 2018 2:09 PM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; RDMA mailing list <linux- [email protected]>; Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/[email protected]>; Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Subject: Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP
informationQuoted 4 lines · Leon Romanovsky
drivers/infiniband/core/nldev.c +3
-0+/* PID == 0 means that this QP was created by kernel */ + if (qp->res.pid && nla_put_u32(msg, + RDMA_NLDEV_ATTR_RES_PID, qp-res.pid))
This is returning a pid in the init name space, obtained here:
(quoted diff) +1
-0+ res->pid = task_pid_nr(current);
Quoted 6 lines
And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
This API also needs to filter the results and only return pids visible, and translate the pids as well..
Same fix needed for rdma_accept() and rdma_create_id() in near future. --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
In reply to a message not in the archiveQuoted 10 lines · Jason Gunthorpe
drivers/infiniband/core/nldev.c static int fill_res_info(struct sk_buff *msg, struct ib_device *device) · line 279
Quoted 3 lines · Leon Romanovsky
drivers/infiniband/core/nldev.c +3
-0+/* PID == 0 means that this QP was created by kernel */ +if (qp->res.pid &&nla_put_u32 (msg, +RDMA_NLDEV_ATTR_RES_PID , qp->res.pid))This is returning a pid in the init name space, obtained here:
(quoted diff) +1
-0+ res->pid = task_pid_nr(current);
Quoted 6 lines
And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
This API also needs to filter the results and only return pids visible, and translate the pids as well..
Correct, PID namespace wasn't taken into account, exactly as it wasn't taken in CMA. So, right now, CMA netlink statistics is returning wrong and unfiltered PIDs. How do you want to progress with that part of the code?
I personally have no plans to fix CMA netlink code and for my opinion it should be removed, instead of beating that dead horse.
Quoted 4 lines
I also suspsect this needs to be a netlink array of pids for future, as we have hope someday to have RDMA uobjects shared between multiple processes?
Not really, there is no such feature yet and once it will be introduced the authors will need to update netlink interface and rdmatool to support multiple PID option. They will add new netlink attribute and fill it for their shared objects. For most users of RDMA stack, this shared object thing is not needed and better to avoid from over-engineering.
Quoted 7 lines
drivers/infiniband/core/nldev.c static int fill_res_info(struct sk_buff *msg, struct ib_device *device) · line 284
Quoted 4 lines
drivers/infiniband/core/nldev.c +3
-0+if (nla_put_string (msg, +RDMA_NLDEV_ATTR_RES_PID_COMM , qp->res.task_comm)) + goto err;Feels odd to return the content of
/proc/XX/commin netlink?
It was just to save extra syscalls for every object. I'll remove it for the user created objects to simplify PID namespaces support.
Quoted 1 line
Jason
A message that is not in the archive (sent off-list or not yet received): 20180110071722.GF7368-U/[email protected]
Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
Quoted 15 lines · Leon Romanovsky
Quoted 14 lines · Jason Gunthorpe
drivers/infiniband/core/nldev.c static int fill_res_info(struct sk_buff *msg, struct ib_device *device) · line 279
Quoted 3 lines · Leon Romanovsky
drivers/infiniband/core/nldev.c +3
-0+/* PID == 0 means that this QP was created by kernel */ +if (qp->res.pid &&nla_put_u32 (msg, +RDMA_NLDEV_ATTR_RES_PID , qp->res.pid))This is returning a pid in the init name space, obtained here:
(quoted diff) +1
-0+ res->pid = task_pid_nr(current);And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
Quoted 6 lines
Quoted 2 lines
This API also needs to filter the results and only return pids visible, and translate the pids as well..
Correct, PID namespace wasn't taken into account, exactly as it wasn't taken in CMA. So, right now, CMA netlink statistics is returning wrong and unfiltered PIDs. How do you want to progress with that part of the code?
It should be fixed or deleted.
Quoted 2 lines
I personally have no plans to fix CMA netlink code and for my opinion it should be removed, instead of beating that dead horse.
What is so wrong with it you can't use it anyhow?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20180110225622.GQ4518-uk2M96/[email protected]
RE: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
Quoted 18 lines · Leon Romanovsky
Quoted 16 lines · Jason Gunthorpe
Quoted 5 lines · Leon Romanovsky
Review
Quoted 4 lines
(quoted diff) +3
-0+ /* PID == 0 means that this QP was created by kernel */ + if (qp->res.pid && nla_put_u32(msg, + RDMA_NLDEV_ATTR_RES_PID, qp-res.pid))
Quoted 9 lines
This is returning a pid in the init name space, obtained here:
(quoted diff) +1
-0+ res->pid = task_pid_nr(current);And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
This API also needs to filter the results and only return pids visible, and translate the pids as well..
Correct, PID namespace wasn't taken into account, exactly as it wasn't taken in CMA. So, right now, CMA netlink statistics is returning wrong and unfiltered PIDs. How do you want to progress with that part of the
code?
Quoted 8 lines
It should be fixed or deleted.
Quoted 2 lines
I personally have no plans to fix CMA netlink code and for my opinion it should be removed, instead of beating that dead horse.
What is so wrong with it you can't use it anyhow?
We certainly need to be able to get detailed cm_id information. At least cm_id bound device, cm_id state, port num, qp-id associated with this cm_id, and src/dst ipaddrs/ports. However, it could be fetched and added to qp attributes. Ala iw_cxgb4's debugfs "qps" file:
[root@stevo3 stevo]# cat /sys/kernel/debug/iw_cxgb4/0000\:04\:00.4/qps rc qp sq id 1136 rq id 1137 state 1 onchip 0 ep tid 12028 state 7 172.16.4.3:58725/58725->172.16.4.4:7174/7174
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
Quoted 39 lines · Steve Wise
Quoted 18 lines · Leon Romanovsky
Quoted 16 lines · Jason Gunthorpe
Quoted 5 lines · Leon Romanovsky
Review
Quoted 4 lines
(quoted diff) +3
-0+ /* PID == 0 means that this QP was created by kernel */ + if (qp->res.pid && nla_put_u32(msg, + RDMA_NLDEV_ATTR_RES_PID, qp-res.pid))
Quoted 9 lines
This is returning a pid in the init name space, obtained here:
(quoted diff) +1
-0+ res->pid = task_pid_nr(current);And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
This API also needs to filter the results and only return pids visible, and translate the pids as well..
Correct, PID namespace wasn't taken into account, exactly as it wasn't taken in CMA. So, right now, CMA netlink statistics is returning wrong and unfiltered PIDs. How do you want to progress with that part of the
code?
Quoted 8 lines
It should be fixed or deleted.
Quoted 2 lines
I personally have no plans to fix CMA netlink code and for my opinion it should be removed, instead of beating that dead horse.
What is so wrong with it you can't use it anyhow?
We certainly need to be able to get detailed
cm_idinformation. At leastcm_idbound device,cm_idstate, port num, qp-id associated with thiscm_id, and src/dst ipaddrs/ports. However, it could be fetched and added to qp attributes. Ala iw_cxgb4's debugfs "qps" file:
Quoted 5 lines
[root@stevo3 stevo]# cat /sys/kernel/debug/iw_cxgb4/0000\:04\:00.4/qps rc qp sq id 1136 rq id 1137 state 1 onchip 0 ep tid 12028 state 7 172.16.4.3:58725/58725->172.16.4.4:7174/7174
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It can be one option, another is to add new RDMA_RESTRACK_CMA object and expose all relevant information with rdmatool, e.g. "rdma resoruce cma link cxgb4/1".
Thanks
Quoted 7 lines
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20180111054141.GM7368-U/[email protected]
RE: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
I personally have no plans to fix CMA netlink code and for my
opinion it
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
should be removed, instead of beating that dead horse.
What is so wrong with it you can't use it anyhow?
We certainly need to be able to get detailed
cm_idinformation. At
least
Quoted 1 line
Quoted 1 line
cm_idbound device,cm_idstate, port num, qp-id associated with this
cm_id,
Quoted 11 lines
Quoted 8 lines
and src/dst ipaddrs/ports. However, it could be fetched and added to qp attributes. Ala iw_cxgb4's debugfs "qps" file:
[root@stevo3 stevo]# cat /sys/kernel/debug/iw_cxgb4/0000\:04\:00.4/qps rc qp sq id 1136 rq id 1137 state 1 onchip 0 ep tid 12028 state 7 172.16.4.3:58725/58725->172.16.4.4:7174/7174
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It can be one option, another is to add new
RDMA_RESTRACK_CMAobject and expose all relevant information with rdmatool, e.g. "rdma resoruce cma
link
Quoted 2 lines
cxgb4/1".
I like the idea of RDMA_RESTRACK_CMA. I think cm_id's are important objects like qps, cqs, pds, and mrs.
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
Quoted 21 lines · Steve Wise
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
I personally have no plans to fix CMA netlink code and for my
opinion it
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
should be removed, instead of beating that dead horse.
What is so wrong with it you can't use it anyhow?
We certainly need to be able to get detailed
cm_idinformation. Atleast
Quoted 1 line
Quoted 1 line
cm_idbound device,cm_idstate, port num, qp-id associated with this
cm_id,Quoted 9 lines
Quoted 8 lines
and src/dst ipaddrs/ports. However, it could be fetched and added to qp attributes. Ala iw_cxgb4's debugfs "qps" file:
[root@stevo3 stevo]# cat /sys/kernel/debug/iw_cxgb4/0000\:04\:00.4/qps rc qp sq id 1136 rq id 1137 state 1 onchip 0 ep tid 12028 state 7 172.16.4.3:58725/58725->172.16.4.4:7174/7174
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 8 lines
Quoted 2 lines
It can be one option, another is to add new
RDMA_RESTRACK_CMAobject and expose all relevant information with rdmatool, e.g. "rdma resoruce cmalink
Quoted 2 lines
cxgb4/1".
I like the idea of
RDMA_RESTRACK_CMA. I think cm_id's are important objects like qps, cqs, pds, and mrs.
Now, it is clear why I want to kill current CMA netlink code :)
Quoted 3 lines
Steve.
Re: [PATCH rdma-next v2 7/7] RDMA/nldev: Provide detailed QP information
In reply to a message not in the archiveQuoted 16 lines · Jason Gunthorpe
Quoted 15 lines · Leon Romanovsky
Quoted 14 lines · Jason Gunthorpe
Review
Quoted 3 lines · Leon Romanovsky
(quoted diff) +3
-0+ /* PID == 0 means that this QP was created by kernel */ + if (qp->res.pid && nla_put_u32(msg, + RDMA_NLDEV_ATTR_RES_PID, qp->res.pid))This is returning a pid in the init name space, obtained here:
(quoted diff) +1
-0+ res->pid = task_pid_nr(current);And since the netlink user is not running in the init name space this will return the wrong pid #, and worse potentially pids the current name space should not see.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
This API also needs to filter the results and only return pids visible, and translate the pids as well..
Correct, PID namespace wasn't taken into account, exactly as it wasn't taken in CMA. So, right now, CMA netlink statistics is returning wrong and unfiltered PIDs. How do you want to progress with that part of the code?
It should be fixed or deleted.
So, should I proceed with this patch? Remove CMA statistics?
https://patchwork.kernel.org/patch/10056129/
Quoted 5 lines
Quoted 3 lines
I personally have no plans to fix CMA netlink code and for my opinion it should be removed, instead of beating that dead horse.
What is so wrong with it you can't use it anyhow?
In addition to posted in the CMA statistics removal RFC thread, it is now very clearly seen that CMA statistics returns wrong PID in not initial PID namespace.
Thanks
Quoted 2 lines
Jason
Steve Wise
Reviewed-byQuoted 13 lines
Changelog:
v1->v2:
* Rebased on latest rdma/for-next
* Replaced mutex lock which protect linked lists to be RW semaphore. It has no impact on current implementation, because there is only one reader (nldev) and it is serialized. However better to be prepared for multiple readers from the beginning.
* Added reinitialization next QP entry logic to ensure that it exists and didn't vanish duringfill_req_qp()work.v0->v1:
* Dropped RFC
* Separated to thre series, one for for-rc, and two for-next.
Hey Leon,
This series looks good to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 14 lines · Steve Wise
Quoted 13 lines
Changelog:
v1->v2:
* Rebased on latest rdma/for-next
* Replaced mutex lock which protect linked lists to be RW semaphore. It has no impact on current implementation, because there is only one reader (nldev) and it is serialized. However better to be prepared for multiple readers from the beginning.
* Added reinitialization next QP entry logic to ensure that it exists and didn't vanish duringfill_req_qp()work.v0->v1:
* Dropped RFC
* Separated to thre series, one for for-rc, and two for-next.
Quoted 6 lines
Hey Leon,
This series looks good to me.
Reviewed-by Steve Wise <[email protected]>
Thanks a lot.>
A message that is not in the archive (sent off-list or not yet received): 20180109075344.GD6823-U/[email protected]
Quoted 7 lines · Steve Wise
Quoted 6 lines
Quoted 6 lines
Changelog:
v1->v2:
* Rebased on latest rdma/for-next
* Replaced mutex lock which protect linked lists to be RW semaphore. It has no impact on current implementation, because there is only
one
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
reader (nldev) and it is serialized. However better to be prepared for multiple readers from the beginning.
* Added reinitialization next QP entry logic to ensure that it exists and didn't vanish duringfill_req_qp()work.v0->v1:
* Dropped RFC
* Separated to thre series, one for for-rc, and two for-next.
Quoted 8 lines
Quoted 6 lines
Hey Leon,
This series looks good to me.
Reviewed-by Steve Wise <[email protected]>Thanks a lot.>
You're welcome! I'm planning on using this as a starting point for qp/cq/connection state dumps for iw_cxgb4 objects. This allows gathering key state when debugging stalls/hangs/etc. So I'm hoping this can get integrated soon, so we can add to it going forward.
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html