rpms/libmthca/devel libmthca-Fix-race-between-create-QP-and-destroy-QP.patch, NONE, 1.1 libmthca-Remove-empty-stubs-for-detach-attach_mcast.patch, NONE, 1.1 libmthca-Update-function-prototypes-to-match-libibverbs-enum-.patch, NONE, 1.1 libmthca-Use-mmap-MAP_ANONYMOUS-to-allocate-queue-buffers.patch, NONE, 1.1 libmthca.spec, 1.13, 1.14

Wed Dec 2 19:35:12 UTC 2009

Author: dledford

Update of /cvs/extras/rpms/libmthca/devel
In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv1068

Modified Files:
	libmthca.spec 
Added Files:
	libmthca-Fix-race-between-create-QP-and-destroy-QP.patch 
	libmthca-Remove-empty-stubs-for-detach-attach_mcast.patch 
	libmthca-Update-function-prototypes-to-match-libibverbs-enum-.patch 
	libmthca-Use-mmap-MAP_ANONYMOUS-to-allocate-queue-buffers.patch 
Log Message:
* Wed Dec 02 2009 Doug Ledford <dledford at redhat.com> - 1.0.5-5
- Rename devel-static package to just -static, it only has a single static
  lib in it and no actual devel files (like headers, those are part of
  libibverbs-devel instead).  Obsolete the various other named packages we
  want this to supercede
- Enable valgrind annotations on all arches except ia64
- Update to latest code, including update for libibverbs API change
- Bump release to 5 so it is higher than the current rhel5 libmthca release
- Add various patches from the upstream git repo that haven't been rolled into
  a new release tarball yet


libmthca-Fix-race-between-create-QP-and-destroy-QP.patch:
 qp.c    |   18 +++---------------
 verbs.c |   10 ++++++++--
 2 files changed, 11 insertions(+), 17 deletions(-)

--- NEW FILE libmthca-Fix-race-between-create-QP-and-destroy-QP.patch ---
>From be5eef3895eb7864db6395b885a19f770fde7234 Mon Sep 17 00:00:00 2001
From: Jack Morgenstein <jackm at dev.mellanox.co.il>
Date: Sat, 22 Nov 2008 11:54:01 +0200
Subject: [PATCH 3/9] Fix race between create QP and destroy QP

There is a race in libmthca because mthca_create_qp() and
mthca_destroy_qp() are not atomic WRT each other.  If one thread is
destroying a QP while another is creating a QP, the following can
happen: the destroying thread can be scheduled out after it has
deleted the QP from kernel space, but before it has cleared it from
userspace store (mthca_clear_qp()).  If the other thread creates a QP
during this break, it gets the same QP base number and overwrites the
destroyed QP's entry with mthca_store_qp().  When the destroying thread
resumes, it clears the new entry from the userspace store via
mthca_clear_qp.

Fix this by expanding where qp_table_mutex is held to serialize the
full create and destroy operations against each other.

Signed-off-by: Roland Dreier <rolandd at cisco.com>
---
 src/qp.c    |   18 +++---------------
 src/verbs.c |    9 ++++++++-
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/qp.c b/src/qp.c
index 55608be..84dd206 100644
--- a/src/qp.c
+++ b/src/qp.c
@@ -909,39 +909,27 @@ struct mthca_qp *mthca_find_qp(struct mthca_context *ctx, uint32_t qpn)
 int mthca_store_qp(struct mthca_context *ctx, uint32_t qpn, struct mthca_qp *qp)
 {
 	int tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift;
-	int ret = 0;
-
-	pthread_mutex_lock(&ctx->qp_table_mutex);
 
 	if (!ctx->qp_table[tind].refcnt) {
 		ctx->qp_table[tind].table = calloc(ctx->qp_table_mask + 1,
 						   sizeof (struct mthca_qp *));
-		if (!ctx->qp_table[tind].table) {
-			ret = -1;
-			goto out;
-		}
+		if (!ctx->qp_table[tind].table)
+			return -1;
 	}
 
 	++ctx->qp_table[tind].refcnt;
 	ctx->qp_table[tind].table[qpn & ctx->qp_table_mask] = qp;
-
-out:
-	pthread_mutex_unlock(&ctx->qp_table_mutex);
-	return ret;
+	return 0;
 }
 
 void mthca_clear_qp(struct mthca_context *ctx, uint32_t qpn)
 {
 	int tind = (qpn & (ctx->num_qps - 1)) >> ctx->qp_table_shift;
 
-	pthread_mutex_lock(&ctx->qp_table_mutex);
-
 	if (!--ctx->qp_table[tind].refcnt)
 		free(ctx->qp_table[tind].table);
 	else
 		ctx->qp_table[tind].table[qpn & ctx->qp_table_mask] = NULL;
-
-	pthread_mutex_unlock(&ctx->qp_table_mutex);
 }
 
 int mthca_free_err_wqe(struct mthca_qp *qp, int is_send,
diff --git a/src/verbs.c b/src/verbs.c
index def0f30..b2ef3ec 100644
--- a/src/verbs.c
+++ b/src/verbs.c
@@ -566,6 +566,7 @@ struct ibv_qp *mthca_create_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr)
 		cmd.sq_db_index = cmd.rq_db_index = 0;
 	}
 
+	pthread_mutex_lock(&to_mctx(pd->context)->qp_table_mutex);
 	ret = ibv_cmd_create_qp(pd, &qp->ibv_qp, attr, &cmd.ibv_cmd, sizeof cmd,
 				&resp, sizeof resp);
 	if (ret)
@@ -579,6 +580,7 @@ struct ibv_qp *mthca_create_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr)
 	ret = mthca_store_qp(to_mctx(pd->context), qp->ibv_qp.qp_num, qp);
 	if (ret)
 		goto err_destroy;
+	pthread_mutex_unlock(&to_mctx(pd->context)->qp_table_mutex);
 
 	qp->sq.max 	    = attr->cap.max_send_wr;
 	qp->rq.max 	    = attr->cap.max_recv_wr;
@@ -592,6 +594,7 @@ err_destroy:
 	ibv_cmd_destroy_qp(&qp->ibv_qp);
 
 err_rq_db:
+	pthread_mutex_unlock(&to_mctx(pd->context)->qp_table_mutex);
 	if (mthca_is_memfree(pd->context))
 		mthca_free_db(to_mctx(pd->context)->db_tab, MTHCA_DB_TYPE_RQ,
 			      qp->rq.db_index);
@@ -686,9 +689,12 @@ int mthca_destroy_qp(struct ibv_qp *qp)
 {
 	int ret;
 
+	pthread_mutex_lock(&to_mctx(qp->context)->qp_table_mutex);
 	ret = ibv_cmd_destroy_qp(qp);
-	if (ret)
+	if (ret) {
+		pthread_mutex_unlock(&to_mctx(qp->context)->qp_table_mutex);
 		return ret;
+	}
 
 	mthca_lock_cqs(qp);
 
@@ -700,6 +706,7 @@ int mthca_destroy_qp(struct ibv_qp *qp)
 	mthca_clear_qp(to_mctx(qp->context), qp->qp_num);
 
 	mthca_unlock_cqs(qp);
+	pthread_mutex_unlock(&to_mctx(qp->context)->qp_table_mutex);
 
 	if (mthca_is_memfree(qp->context)) {
 		mthca_free_db(to_mctx(qp->context)->db_tab, MTHCA_DB_TYPE_RQ,
-- 
1.6.5.2


libmthca-Remove-empty-stubs-for-detach-attach_mcast.patch:
 mthca.c |    4 ++--
 mthca.h |    2 --
 verbs.c |   11 -----------
 3 files changed, 2 insertions(+), 15 deletions(-)

--- NEW FILE libmthca-Remove-empty-stubs-for-detach-attach_mcast.patch ---
>From 6dbdcb6984671547ff230163b3dca634eacca790 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe at obsidianresearch.com>
Date: Mon, 20 Jul 2009 16:36:02 -0600
Subject: [PATCH 7/9] Remove empty stubs for detach/attach_mcast

Just use ibv_cmd_* directly.  Simplifies the code and fixes const
correctness warnings due to changes in libibverbs.

Signed-off-by: Jason Gunthorpe <jgunthorpe at obsidianresearch.com>
Signed-off-by: Roland Dreier <rolandd at cisco.com>
---
 src/mthca.c |    4 ++--
 src/mthca.h |    2 --
 src/verbs.c |   10 ----------
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/src/mthca.c b/src/mthca.c
index e00c4ee..e33bf7f 100644
--- a/src/mthca.c
+++ b/src/mthca.c
@@ -127,8 +127,8 @@ static struct ibv_context_ops mthca_ctx_ops = {
 	.destroy_qp    = mthca_destroy_qp,
 	.create_ah     = mthca_create_ah,
 	.destroy_ah    = mthca_destroy_ah,
-	.attach_mcast  = mthca_attach_mcast,
-	.detach_mcast  = mthca_detach_mcast
+	.attach_mcast  = ibv_cmd_attach_mcast,
+	.detach_mcast  = ibv_cmd_detach_mcast
 };
 
 static struct ibv_context *mthca_alloc_context(struct ibv_device *ibdev, int cmd_fd)
diff --git a/src/mthca.h b/src/mthca.h
index 66751f3..9a2e362 100644
--- a/src/mthca.h
+++ b/src/mthca.h
@@ -372,7 +372,5 @@ int mthca_destroy_ah(struct ibv_ah *ah);
 int mthca_alloc_av(struct mthca_pd *pd, struct ibv_ah_attr *attr,
 		   struct mthca_ah *ah);
 void mthca_free_av(struct mthca_ah *ah);
-int mthca_attach_mcast(struct ibv_qp *qp, union ibv_gid *gid, uint16_t lid);
-int mthca_detach_mcast(struct ibv_qp *qp, union ibv_gid *gid, uint16_t lid);
 
 #endif /* MTHCA_H */
diff --git a/src/verbs.c b/src/verbs.c
index b2ef3ec..f6570c6 100644
--- a/src/verbs.c
+++ b/src/verbs.c
@@ -746,13 +746,3 @@ int mthca_destroy_ah(struct ibv_ah *ah)
 
 	return 0;
 }
-
-int mthca_attach_mcast(struct ibv_qp *qp, union ibv_gid *gid, uint16_t lid)
-{
-	return ibv_cmd_attach_mcast(qp, gid, lid);
-}
-
-int mthca_detach_mcast(struct ibv_qp *qp, union ibv_gid *gid, uint16_t lid)
-{
-	return ibv_cmd_detach_mcast(qp, gid, lid);
-}
-- 
1.6.5.2


libmthca-Update-function-prototypes-to-match-libibverbs-enum-.patch:
 mthca.h |    8 ++++----
 verbs.c |   11 +++++------
 2 files changed, 9 insertions(+), 10 deletions(-)

--- NEW FILE libmthca-Update-function-prototypes-to-match-libibverbs-enum-.patch ---
>From 0d1253d0f64a325dac9d3a078a04e573c007f5b6 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe at obsidianresearch.com>
Date: Thu, 23 Jul 2009 10:04:29 -0600
Subject: [PATCH 9/9] Update function prototypes to match libibverbs enum type change

Change enum bit flags to int to match libibverbs prototype changes.

Signed-off-by: Jason Gunthorpe <jgunthorpe at obsidianresearch.com>
Signed-off-by: Roland Dreier <rolandd at cisco.com>
---
 src/mthca.h |    8 ++++----
 src/verbs.c |   10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/mthca.h b/src/mthca.h
index 9a2e362..bd1e7a2 100644
--- a/src/mthca.h
+++ b/src/mthca.h
@@ -309,7 +309,7 @@ struct ibv_pd *mthca_alloc_pd(struct ibv_context *context);
 int mthca_free_pd(struct ibv_pd *pd);
 
 struct ibv_mr *mthca_reg_mr(struct ibv_pd *pd, void *addr,
-			    size_t length, enum ibv_access_flags access);
+			    size_t length, int access);
 int mthca_dereg_mr(struct ibv_mr *mr);
 
 struct ibv_cq *mthca_create_cq(struct ibv_context *context, int cqe,
@@ -330,7 +330,7 @@ struct ibv_srq *mthca_create_srq(struct ibv_pd *pd,
 				 struct ibv_srq_init_attr *attr);
 int mthca_modify_srq(struct ibv_srq *srq,
 		     struct ibv_srq_attr *attr,
-		     enum ibv_srq_attr_mask mask);
+		     int mask);
 int mthca_query_srq(struct ibv_srq *srq,
 			   struct ibv_srq_attr *attr);
 int mthca_destroy_srq(struct ibv_srq *srq);
@@ -346,10 +346,10 @@ int mthca_arbel_post_srq_recv(struct ibv_srq *ibsrq,
 
 struct ibv_qp *mthca_create_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr);
 int mthca_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
-		   enum ibv_qp_attr_mask attr_mask,
+		   int attr_mask,
 		   struct ibv_qp_init_attr *init_attr);
 int mthca_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
-		    enum ibv_qp_attr_mask attr_mask);
+		    int attr_mask);
 int mthca_destroy_qp(struct ibv_qp *qp);
 void mthca_init_qp_indices(struct mthca_qp *qp);
 int mthca_tavor_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
diff --git a/src/verbs.c b/src/verbs.c
index f6570c6..b6782c9 100644
--- a/src/verbs.c
+++ b/src/verbs.c
@@ -117,7 +117,7 @@ int mthca_free_pd(struct ibv_pd *pd)
 
 static struct ibv_mr *__mthca_reg_mr(struct ibv_pd *pd, void *addr,
 				     size_t length, uint64_t hca_va,
-				     enum ibv_access_flags access,
+				     int access,
 				     int dma_sync)
 {
 	struct ibv_mr *mr;
@@ -157,7 +157,7 @@ static struct ibv_mr *__mthca_reg_mr(struct ibv_pd *pd, void *addr,
 }
 
 struct ibv_mr *mthca_reg_mr(struct ibv_pd *pd, void *addr,
-			    size_t length, enum ibv_access_flags access)
+			    size_t length, int access)
 {
 	return __mthca_reg_mr(pd, addr, length, (uintptr_t) addr, access, 0);
 }
@@ -468,7 +468,7 @@ err:
 
 int mthca_modify_srq(struct ibv_srq *srq,
 		     struct ibv_srq_attr *attr,
-		     enum ibv_srq_attr_mask attr_mask)
+		     int attr_mask)
 {
 	struct ibv_modify_srq cmd;
 
@@ -618,7 +618,7 @@ err:
 }
 
 int mthca_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
-		   enum ibv_qp_attr_mask attr_mask,
+		   int attr_mask,
 		   struct ibv_qp_init_attr *init_attr)
 {
 	struct ibv_query_qp cmd;
@@ -627,7 +627,7 @@ int mthca_query_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
 }
 
 int mthca_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
-		    enum ibv_qp_attr_mask attr_mask)
+		    int attr_mask)
 {
 	struct ibv_modify_qp cmd;
 	int ret;
-- 
1.6.5.2


libmthca-Use-mmap-MAP_ANONYMOUS-to-allocate-queue-buffers.patch:
 buf.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

--- NEW FILE libmthca-Use-mmap-MAP_ANONYMOUS-to-allocate-queue-buffers.patch ---
>From 14fb976c9999e1c6dc2a5c7a5e0f790d35631658 Mon Sep 17 00:00:00 2001
From: Sebastien Dugue <sebastien.dugue at bull.net>
Date: Wed, 29 Jul 2009 11:56:53 -0700
Subject: [PATCH 8/9] Use mmap(MAP_ANONYMOUS) to allocate queue buffers

Internal buffers for QPs, CQs, SRQs etc. are allocated with
mthca_alloc_buf(), which rounds the buffer's size to the page size and
then allocates page aligned memory using posix_memalign().

However, this allocation is quite wasteful on architectures using 64K
pages (ia64 for example) because we then hit glibc's MMAP_THRESHOLD
malloc parameter and chunks are allocated using mmap.  Thus we end up
allocating:

  (requested size rounded to the page size) + (page size) + (malloc overhead)

rounded internally to the page size.

So for example, if we request a buffer of page_size bytes, we end up
consuming 3 pages.  In short, for each buffer we allocate, there is an
overhead of 2 pages.  This is quite visible on large clusters where
the number of QPs can reach several thousands.

This patch replaces the call to posix_memalign() in mthca_alloc_buf()
with a direct call to mmap().

Signed-off-by: Sebastien Dugue <sebastien.dugue at bull.net>
Signed-off-by: Roland Dreier <rolandd at cisco.com>
---
 src/buf.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/buf.c b/src/buf.c
index 6c1be4f..074a5f8 100644
--- a/src/buf.c
+++ b/src/buf.c
@@ -35,6 +35,8 @@
 #endif /* HAVE_CONFIG_H */
 
 #include <stdlib.h>
+#include <sys/mman.h>
+#include <errno.h>
 
 #include "mthca.h"
 
@@ -61,16 +63,15 @@ int mthca_alloc_buf(struct mthca_buf *buf, size_t size, int page_size)
 {
 	int ret;
 
-	ret = posix_memalign(&buf->buf, page_size, align(size, page_size));
-	if (ret)
-		return ret;
+	buf->length = align(size, page_size);
+	buf->buf = mmap(NULL, buf->length, PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (buf->buf == MAP_FAILED)
+		return errno;
 
 	ret = ibv_dontfork_range(buf->buf, size);
 	if (ret)
-		free(buf->buf);
-
-	if (!ret)
-		buf->length = size;
+		munmap(buf->buf, buf->length);
 
 	return ret;
 }
@@ -78,5 +79,5 @@ int mthca_alloc_buf(struct mthca_buf *buf, size_t size, int page_size)
 void mthca_free_buf(struct mthca_buf *buf)
 {
 	ibv_dofork_range(buf->buf, buf->length);
-	free(buf->buf);
+	munmap(buf->buf, buf->length);
 }
-- 
1.6.5.2



Index: libmthca.spec
===================================================================
RCS file: /cvs/extras/rpms/libmthca/devel/libmthca.spec,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -p -r1.13 -r1.14
--- libmthca.spec	25 Jul 2009 06:09:15 -0000	1.13
+++ libmthca.spec	2 Dec 2009 19:35:12 -0000	1.14
@@ -1,35 +1,53 @@
 Name: libmthca
 Version: 1.0.5
-Release: 3%{?dist}
+Release: 5%{?dist}
 Summary: Mellanox InfiniBand HCA Userspace Driver
-
+Provides: libibverbs-driver
 Group: System Environment/Libraries
 License: GPLv2 or BSD
 Url: http://openfabrics.org/
-Source: http://openfabrics.org/downloads/mthca/libmthca-1.0.5.tar.gz
+Source: http://openfabrics.org/downloads/mthca/%{name}-%{version}.tar.gz
+Patch0: libmthca-Fix-race-between-create-QP-and-destroy-QP.patch
+Patch1: libmthca-Remove-empty-stubs-for-detach-attach_mcast.patch
+Patch2: libmthca-Use-mmap-MAP_ANONYMOUS-to-allocate-queue-buffers.patch
+Patch3: libmthca-Update-function-prototypes-to-match-libibverbs-enum-.patch
 BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
-
-BuildRequires: libibverbs-devel >= 1.1-0.1.rc2
+BuildRequires: libibverbs-devel >= 1.1.3
+%ifnarch ia64
+BuildRequires: valgrind-devel
+%endif
 
 %description
 libmthca provides a device-specific userspace driver for Mellanox HCAs
 (MT23108 InfiniHost and MT25208 InfiniHost III Ex) for use with the
 libibverbs library.
 
-%package devel-static
+%package static
 Summary: Development files for the libmthca driver
 Group: System Environment/Libraries
+Provides: %{name}-devel-static = %{version}-%{release}
+Obsoletes: %{name}-devel-static <= 1.0.5-3
+Provides: %{name}-devel = %{version}-%{release}
+Obsoletes: %{name}-devel <= 1.0.5-3
 Requires: %{name} = %{version}-%{release}
 
-%description devel-static
+%description static
 Static version of libmthca that may be linked directly to an
 application, which may be useful for debugging.
 
 %prep
-%setup -q -n %{name}-1.0.5
+%setup -q
+%patch0 -p1
+%patch1 -p1
+%patch2 -p1
+%patch3 -p1
 
 %build
+%ifnarch ia64
+%configure --with-valgrind
+%else
 %configure
+%endif
 make %{?_smp_mflags}
 
 %install
@@ -47,11 +65,22 @@ rm -rf $RPM_BUILD_ROOT
 %{_sysconfdir}/libibverbs.d/mthca.driver
 %doc AUTHORS COPYING ChangeLog README
 
-%files devel-static
+%files static
 %defattr(-,root,root,-)
 %{_libdir}/libmthca.a
 
 %changelog
+* Wed Dec 02 2009 Doug Ledford <dledford at redhat.com> - 1.0.5-5
+- Rename devel-static package to just -static, it only has a single static
+  lib in it and no actual devel files (like headers, those are part of
+  libibverbs-devel instead).  Obsolete the various other named packages we
+  want this to supercede
+- Enable valgrind annotations on all arches except ia64
+- Update to latest code, including update for libibverbs API change
+- Bump release to 5 so it is higher than the current rhel5 libmthca release
+- Add various patches from the upstream git repo that haven't been rolled into
+  a new release tarball yet
+
 * Fri Jul 24 2009 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 1.0.5-3
 - Rebuilt for https://fedoraproject.org/wiki/Fedora_12_Mass_Rebuild
 


