diff mbox

[rdma-next,v4,2/7] RDMA/core: Add helper function to create named QPs

Message ID 20180115151255.30167-3-leon@kernel.org (mailing list archive)
State Changes Requested
Headers show

Commit Message

Leon Romanovsky Jan. 15, 2018, 3:12 p.m. UTC 
From: Leon Romanovsky <leonro@mellanox.com>

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.

Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
---
 drivers/infiniband/core/core_priv.h | 19 +++++++++++++++++++
 include/rdma/ib_verbs.h             |  6 ++++++
 2 files changed, 25 insertions(+)

Comments

Bart Van Assche Jan. 16, 2018, 9:35 p.m. UTC | #1 
On Mon, 2018-01-15 at 17:12 +0200, Leon Romanovsky wrote:
> --- a/include/rdma/ib_verbs.h

> +++ b/include/rdma/ib_verbs.h

> @@ -1141,6 +1141,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];

>  };


Why is comm[] an array? For queue pairs created from user space, are there any
queue pairs that can live longer than the task that created them? If not, does
that mean that it is safe to change comm into a const char pointer?

Additionally, please use the kernel-doc syntax to document structure members.

Thanks,

Bart.
Leon Romanovsky Jan. 17, 2018, 5:59 a.m. UTC | #2 
On Tue, Jan 16, 2018 at 09:35:42PM +0000, Bart Van Assche wrote:
> On Mon, 2018-01-15 at 17:12 +0200, Leon Romanovsky wrote:
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -1141,6 +1141,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];
> >  };
>
> Why is comm[] an array? For queue pairs created from user space, are there any
> queue pairs that can live longer than the task that created them? If not, does
> that mean that it is safe to change comm into a const char pointer?

As I wrote in cover letter, this code will be refactored and moved to PD once
Steve's implementation of CMA resource tracking will be accepted in upstream.

>
> Additionally, please use the kernel-doc syntax to document structure members.

Kernel-doc checker doesn't work with patches and ib_verbs.h full of
kernel-doc warning.

>
> Thanks,
>
> Bart.
Bart Van Assche Jan. 18, 2018, 3:22 a.m. UTC | #3 
On Wed, 2018-01-17 at 07:59 +0200, Leon Romanovsky wrote:
> On Tue, Jan 16, 2018 at 09:35:42PM +0000, Bart Van Assche wrote:

> > Additionally, please use the kernel-doc syntax to document structure members.

> 

> Kernel-doc checker doesn't work with patches and ib_verbs.h full of

> kernel-doc warning.


I can review the kernel-doc syntax for you if the kernel-doc checking script
triggers too many warnings.

Bart.
diff mbox

Patch

diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
index 2b1372da708a..3f1442bd5fdc 100644
--- a/drivers/infiniband/core/core_priv.h
+++ b/drivers/infiniband/core/core_priv.h
@@ -301,4 +301,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 */
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 271265cd7c1a..41d4f59cac5e 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1141,6 +1141,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 {