| Message ID | 1524803775-24118-3-git-send-email-rzambre@uci.edu (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Fri, Apr 27, 2018 at 04:36:15AM +0000, Rohit Zambre wrote: > By creating a QP within a thread domain the application is guaranteeing > that the QP will not be accessed concurrently from multiple user threads. > Hence, a lock is not needed for QP that is assigned to a thread domain. > > This patch disables locking on the QP if a thread domain is passed during > QP-creation. > > Signed-off-by: Rohit Zambre <rzambre@uci.edu> > providers/mlx5/verbs.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c > index 9fd765e..f7132e4 100644 > +++ b/providers/mlx5/verbs.c > @@ -1607,6 +1607,7 @@ static struct ibv_qp *create_qp(struct ibv_context *context, > struct mlx5_create_qp_ex_resp resp_ex; > struct mlx5_qp *qp; > int ret; > + int thread_safe; > struct mlx5_context *ctx = to_mctx(context); > struct ibv_qp *ibqp; > int32_t usr_idx = 0; > @@ -1751,8 +1752,14 @@ static struct ibv_qp *create_qp(struct ibv_context *context, > > mlx5_init_qp_indices(qp); > > - if (mlx5_spinlock_init(&qp->sq.lock, !mlx5_single_threaded) || > - mlx5_spinlock_init(&qp->rq.lock, !mlx5_single_threaded)) > + mparent_domain = to_mparent_domain(attr->pd); > + if (mparent_domain && mparent_domain->mtd) > + thread_safe = 1; > + else > + thread_safe = mlx5_single_threaded; > + > + if (mlx5_spinlock_init(&qp->sq.lock, !thread_safe) || > + mlx5_spinlock_init(&qp->rq.lock, !thread_safe)) > goto err_free_qp_buf; This would be nicer to have a 'mlx5_spinlock_init_pd' that takes the object's pd and does the above calcuations. All the remaining uses should be revised at once to use a pd if it makes sense during the init.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 30, 2018 at 1:38 PM, Jason Gunthorpe <jgg@mellanox.com> wrote: > > On Fri, Apr 27, 2018 at 04:36:15AM +0000, Rohit Zambre wrote: > > By creating a QP within a thread domain the application is guaranteeing > > that the QP will not be accessed concurrently from multiple user threads. > > Hence, a lock is not needed for QP that is assigned to a thread domain. > > > > This patch disables locking on the QP if a thread domain is passed during > > QP-creation. > > > > Signed-off-by: Rohit Zambre <rzambre@uci.edu> > > providers/mlx5/verbs.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c > > index 9fd765e..f7132e4 100644 > > +++ b/providers/mlx5/verbs.c > > @@ -1607,6 +1607,7 @@ static struct ibv_qp *create_qp(struct ibv_context *context, > > struct mlx5_create_qp_ex_resp resp_ex; > > struct mlx5_qp *qp; > > int ret; > > + int thread_safe; > > struct mlx5_context *ctx = to_mctx(context); > > struct ibv_qp *ibqp; > > int32_t usr_idx = 0; > > @@ -1751,8 +1752,14 @@ static struct ibv_qp *create_qp(struct ibv_context *context, > > > > mlx5_init_qp_indices(qp); > > > > - if (mlx5_spinlock_init(&qp->sq.lock, !mlx5_single_threaded) || > > - mlx5_spinlock_init(&qp->rq.lock, !mlx5_single_threaded)) > > + mparent_domain = to_mparent_domain(attr->pd); > > + if (mparent_domain && mparent_domain->mtd) > > + thread_safe = 1; > > + else > > + thread_safe = mlx5_single_threaded; > > + > > + if (mlx5_spinlock_init(&qp->sq.lock, !thread_safe) || > > + mlx5_spinlock_init(&qp->rq.lock, !thread_safe)) > > goto err_free_qp_buf; > > This would be nicer to have a 'mlx5_spinlock_init_pd' that takes the > object's pd and does the above calcuations. > > All the remaining uses should be revised at once to use a pd if it > makes sense during the init.. Makes sense. Are you referring to the Parent Domain or the Protection Domain by PD here? -Rohit -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 01, 2018 at 04:55:05PM -0500, Rohit Zambre wrote: > On Mon, Apr 30, 2018 at 1:38 PM, Jason Gunthorpe <jgg@mellanox.com> wrote: > > > > On Fri, Apr 27, 2018 at 04:36:15AM +0000, Rohit Zambre wrote: > > > By creating a QP within a thread domain the application is guaranteeing > > > that the QP will not be accessed concurrently from multiple user threads. > > > Hence, a lock is not needed for QP that is assigned to a thread domain. > > > > > > This patch disables locking on the QP if a thread domain is passed during > > > QP-creation. > > > > > > Signed-off-by: Rohit Zambre <rzambre@uci.edu> > > > providers/mlx5/verbs.c | 12 +++++++++--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c > > > index 9fd765e..f7132e4 100644 > > > +++ b/providers/mlx5/verbs.c > > > @@ -1607,6 +1607,7 @@ static struct ibv_qp *create_qp(struct ibv_context *context, > > > struct mlx5_create_qp_ex_resp resp_ex; > > > struct mlx5_qp *qp; > > > int ret; > > > + int thread_safe; > > > struct mlx5_context *ctx = to_mctx(context); > > > struct ibv_qp *ibqp; > > > int32_t usr_idx = 0; > > > @@ -1751,8 +1752,14 @@ static struct ibv_qp *create_qp(struct ibv_context *context, > > > > > > mlx5_init_qp_indices(qp); > > > > > > - if (mlx5_spinlock_init(&qp->sq.lock, !mlx5_single_threaded) || > > > - mlx5_spinlock_init(&qp->rq.lock, !mlx5_single_threaded)) > > > + mparent_domain = to_mparent_domain(attr->pd); > > > + if (mparent_domain && mparent_domain->mtd) > > > + thread_safe = 1; > > > + else > > > + thread_safe = mlx5_single_threaded; > > > + > > > + if (mlx5_spinlock_init(&qp->sq.lock, !thread_safe) || > > > + mlx5_spinlock_init(&qp->rq.lock, !thread_safe)) > > > goto err_free_qp_buf; > > > > This would be nicer to have a 'mlx5_spinlock_init_pd' that takes the > > object's pd and does the above calcuations. > > > > All the remaining uses should be revised at once to use a pd if it > > makes sense during the init.. > > Makes sense. Are you referring to the Parent Domain or the Protection > Domain by PD here? If I recall, at this layer they are still interchangable. The 'mlx5_spinlock_init_pd' should determine if the ibv_pd is a parent domain, then determine if it specifies single threaded operation. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/providers/mlx5/verbs.c b/providers/mlx5/verbs.c index 9fd765e..f7132e4 100644 --- a/providers/mlx5/verbs.c +++ b/providers/mlx5/verbs.c @@ -1607,6 +1607,7 @@ static struct ibv_qp *create_qp(struct ibv_context *context, struct mlx5_create_qp_ex_resp resp_ex; struct mlx5_qp *qp; int ret; + int thread_safe; struct mlx5_context *ctx = to_mctx(context); struct ibv_qp *ibqp; int32_t usr_idx = 0; @@ -1751,8 +1752,14 @@ static struct ibv_qp *create_qp(struct ibv_context *context, mlx5_init_qp_indices(qp); - if (mlx5_spinlock_init(&qp->sq.lock, !mlx5_single_threaded) || - mlx5_spinlock_init(&qp->rq.lock, !mlx5_single_threaded)) + mparent_domain = to_mparent_domain(attr->pd); + if (mparent_domain && mparent_domain->mtd) + thread_safe = 1; + else + thread_safe = mlx5_single_threaded; + + if (mlx5_spinlock_init(&qp->sq.lock, !thread_safe) || + mlx5_spinlock_init(&qp->rq.lock, !thread_safe)) goto err_free_qp_buf; qp->db = mlx5_alloc_dbrec(ctx); @@ -1789,7 +1796,6 @@ static struct ibv_qp *create_qp(struct ibv_context *context, cmd.uidx = usr_idx; } - mparent_domain = to_mparent_domain(attr->pd); if (mparent_domain && mparent_domain->mtd) bf = mparent_domain->mtd->bf;
By creating a QP within a thread domain the application is guaranteeing that the QP will not be accessed concurrently from multiple user threads. Hence, a lock is not needed for QP that is assigned to a thread domain. This patch disables locking on the QP if a thread domain is passed during QP-creation. Signed-off-by: Rohit Zambre <rzambre@uci.edu> --- providers/mlx5/verbs.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)