On Mon, Jan 29, 2018 at 03:11:53PM -0500, Doug Ledford wrote: > On Sun, 2018-01-28 at 14:05 -0700, Jason Gunthorpe wrote: > > On Sun, Jan 28, 2018 at 11:17:17AM +0200, Leon Romanovsky wrote: > > > > > 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. > > > > Well, this cover letter isn't quite right anymore.. but no matter. > > > > My small comments aside it looks OK to me. > > Likewise. I'm happy with it at this point. Okay, I fixed up the small things and applied the patches to for-next Leon: Please validate I didn't screw it up. Here is the diff against what you sent: - Success path on the main execution flow, not under an if - constify static structure - Remove confusing comment about locking, ib_enum_all_devs obtains locks to iterate its list and rdma_restrack_count holds res->rwsem so everything is accounted for directly without trickyness - Speeling - Remove extra lock in rdma_restrack_del - Restore pd = NULL in ib_create_xrc_qp. This scraed me a bit, xrc is wonky. But ib_create_xrc_q is only called in cases where rdma_restrack_add is not added, so keeping things as-they-are should not impact restrack. If restrack needs the pd for a XRC someday it should get it from qp->real_qp - Remove SET/NEW/DEL cargo cult, please send a patch for rest? Branch is here: https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/jgg-for-next Still unhappy with the kref-as-not-a-kref. diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h index 3dcacf220e5e7e..c4560d84dfaebd 100644 --- a/drivers/infiniband/core/core_priv.h +++ b/drivers/infiniband/core/core_priv.h @@ -310,21 +310,21 @@ static inline struct ib_qp *_ib_create_qp(struct ib_device *dev, struct ib_qp *qp; qp = dev->create_qp(pd, attr, udata); - if (!IS_ERR(qp)) { - qp->device = dev; - qp->pd = pd; - /* - * 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. - */ - if (attr->qp_type < IB_QPT_XRC_INI) { - qp->res.type = RDMA_RESTRACK_QP; - rdma_restrack_add(&qp->res); - } else { - qp->res.valid = false; - } - } + if (IS_ERR(qp)) + return qp; + + qp->device = dev; + qp->pd = pd; + /* + * 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. + */ + if (attr->qp_type < IB_QPT_XRC_INI) { + qp->res.type = RDMA_RESTRACK_QP; + rdma_restrack_add(&qp->res); + } else + qp->res.valid = false; return qp; } diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c index 421d5fa6a81731..fa8655e3b3edfe 100644 --- a/drivers/infiniband/core/nldev.c +++ b/drivers/infiniband/core/nldev.c @@ -178,7 +178,7 @@ static int fill_res_info_entry(struct sk_buff *msg, static int fill_res_info(struct sk_buff *msg, struct ib_device *device) { - static const char *names[RDMA_RESTRACK_MAX] = { + static const char * const names[RDMA_RESTRACK_MAX] = { [RDMA_RESTRACK_PD] = "pd", [RDMA_RESTRACK_CQ] = "cq", [RDMA_RESTRACK_QP] = "qp", @@ -553,10 +553,6 @@ static int _nldev_res_get_dumpit(struct ib_device *device, 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); } @@ -627,8 +623,8 @@ static int nldev_res_get_qp_dumpit(struct sk_buff *skb, (!rdma_is_kernel_res(res) && task_active_pid_ns(current) != task_active_pid_ns(res->task))) /* - * 1. Kernel QPs should be visible in init namsapce only - * 2. Preent only QPs visible in the current namespace + * 1. Kernel QPs should be visible in init namspace only + * 2. Present only QPs visible in the current namespace */ goto next; diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c index 351b6940f6dc17..857637bf46da27 100644 --- a/drivers/infiniband/core/restrack.c +++ b/drivers/infiniband/core/restrack.c @@ -150,9 +150,7 @@ void rdma_restrack_del(struct rdma_restrack_entry *res) if (!dev) return; - down_read(&dev->res.rwsem); rdma_restrack_put(res); - up_read(&dev->res.rwsem); wait_for_completion(&res->comp); diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index a98a3e8412f810..16ebc6372c31ab 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -849,6 +849,7 @@ static struct ib_qp *ib_create_xrc_qp(struct ib_qp *qp, qp->event_handler = __ib_shared_qp_event_handler; qp->qp_context = qp; + qp->pd = NULL; qp->send_cq = qp->recv_cq = NULL; qp->srq = NULL; qp->xrcd = qp_init_attr->xrcd; diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h index 23bef401598208..17e59bec169ec0 100644 --- a/include/uapi/rdma/rdma_netlink.h +++ b/include/uapi/rdma/rdma_netlink.h @@ -237,14 +237,8 @@ enum rdma_nldev_command { 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_CMD_RES_QP_GET, /* can dump */ - RDMA_NLDEV_CMD_RES_QP_SET, - RDMA_NLDEV_CMD_RES_QP_NEW, - RDMA_NLDEV_CMD_RES_QP_DEL, RDMA_NLDEV_NUM_OPS }; -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
