[ofa-general] Re: [RFC 1/1] libmthca: CQ/DMA race on Altix

akepner at sgi.com akepner at sgi.com
Wed Jul 18 16:22:32 PDT 2007


On Mon, Jul 16, 2007 at 09:57:52AM -0700, Roland Dreier wrote:

> Looks reasonable but I would prefer to see explicit tests of the abi
> version so that we use the old register MR ABI for old kernels rather
> than unconditionally passing the extra parameter.

How about the following?

This is somewhat untidy, in that the abi_version is exposed to 
verbs.c, but it seemed the best way to go.

 mthca-abi.h |   11 ++++++++++-
 mthca.c     |   19 +++++++++++++------
 verbs.c     |   29 ++++++++++++++++++++---------
 3 files changed, 43 insertions(+), 16 deletions(-)
-- 

diff -rup ofa_1_2_user-20070623-0200.orig/src/userspace/libmthca/src/mthca-abi.h ofa_1_2_user-20070623-0200/src/userspace/libmthca/src/mthca-abi.h
--- ofa_1_2_user-20070623-0200.orig/src/userspace/libmthca/src/mthca-abi.h	2007-06-23 02:00:34.000000000 -0700
+++ ofa_1_2_user-20070623-0200/src/userspace/libmthca/src/mthca-abi.h	2007-07-18 10:58:07.903823741 -0700
@@ -36,7 +36,8 @@
 
 #include <infiniband/kern-abi.h>
 
-#define MTHCA_UVERBS_ABI_VERSION	1
+#define MTHCA_UVERBS_MIN_ABI_VERSION	1
+#define MTHCA_UVERBS_MAX_ABI_VERSION	2
 
 struct mthca_alloc_ucontext_resp {
 	struct ibv_get_context_resp	ibv_resp;
@@ -50,6 +51,14 @@ struct mthca_alloc_pd_resp {
 	__u32				reserved;
 };
 
+struct mthca_reg_mr_abi_ver_2 {
+	struct ibv_reg_mr		ibv_cmd;
+	__u32				mr_attrs;
+#define MTHCA_MR_DMAFLUSH		0x1 
+/* flush in-flight DMA on a write to memory region (IA64_SGI_SN2 only) */
+	__u32				reserved;
+};
+
 struct mthca_create_cq {
 	struct ibv_create_cq		ibv_cmd;
 	__u32				lkey;
diff -rup ofa_1_2_user-20070623-0200.orig/src/userspace/libmthca/src/mthca.c ofa_1_2_user-20070623-0200/src/userspace/libmthca/src/mthca.c
--- ofa_1_2_user-20070623-0200.orig/src/userspace/libmthca/src/mthca.c	2007-06-23 02:00:34.000000000 -0700
+++ ofa_1_2_user-20070623-0200/src/userspace/libmthca/src/mthca.c	2007-07-18 15:50:07.174842760 -0700
@@ -56,6 +56,8 @@
 #include "mthca.h"
 #include "mthca-abi.h"
 
+int abi_ver = 0;
+
 #ifndef PCI_VENDOR_ID_MELLANOX
 #define PCI_VENDOR_ID_MELLANOX			0x15b3
 #endif
@@ -282,11 +284,16 @@ static struct ibv_device *mthca_driver_i
 	return NULL;
 
 found:
-	if (abi_version > MTHCA_UVERBS_ABI_VERSION) {
-		fprintf(stderr, PFX "Fatal: ABI version %d of %s is too new (expected %d)\n",
-			abi_version, uverbs_sys_path, MTHCA_UVERBS_ABI_VERSION);
+	if (abi_version < MTHCA_UVERBS_MIN_ABI_VERSION ||
+	    abi_version > MTHCA_UVERBS_MAX_ABI_VERSION) {
+		fprintf(stderr, PFX "Fatal: ABI version %d of %s is not supported "
+			"(min supported %d, max supported %d)\n",
+			abi_version, uverbs_sys_path, 
+			MTHCA_UVERBS_MIN_ABI_VERSION, 
+			MTHCA_UVERBS_MAX_ABI_VERSION);
 		return NULL;
 	}
+	abi_ver = abi_version;
 
 	dev = malloc(sizeof *dev);
 	if (!dev) {
@@ -314,13 +321,13 @@ static __attribute__((constructor)) void
  */
 struct ibv_device *openib_driver_init(struct sysfs_class_device *sysdev)
 {
-	int abi_ver = 0;
+	int abi_version = 0;
 	char value[8];
 
 	if (ibv_read_sysfs_file(sysdev->path, "abi_version",
 				value, sizeof value) > 0)
-		abi_ver = strtol(value, NULL, 10);
+		abi_version = strtol(value, NULL, 10);
 
-	return mthca_driver_init(sysdev->path, abi_ver);
+	return mthca_driver_init(sysdev->path, abi_version);
 }
 #endif /* HAVE_IBV_REGISTER_DRIVER */
diff -rup ofa_1_2_user-20070623-0200.orig/src/userspace/libmthca/src/verbs.c ofa_1_2_user-20070623-0200/src/userspace/libmthca/src/verbs.c
--- ofa_1_2_user-20070623-0200.orig/src/userspace/libmthca/src/verbs.c	2007-06-23 02:00:34.000000000 -0700
+++ ofa_1_2_user-20070623-0200/src/userspace/libmthca/src/verbs.c	2007-07-18 15:43:13.230506881 -0700
@@ -45,6 +45,8 @@
 #include "mthca.h"
 #include "mthca-abi.h"
 
+extern int abi_ver;
+
 int mthca_query_device(struct ibv_context *context, struct ibv_device_attr *attr)
 {
 	struct ibv_query_device cmd;
@@ -117,26 +119,35 @@ 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)
+				     enum ibv_access_flags access, 
+				     int dmaflush)
 {
 	struct ibv_mr *mr;
-	struct ibv_reg_mr cmd;
+	struct mthca_reg_mr_abi_ver_2 cmd;
+	size_t cmd_size;
 	int ret;
 
 	mr = malloc(sizeof *mr);
 	if (!mr)
 		return NULL;
 
+	if (abi_ver > 1) {
+		cmd.mr_attrs |= (__u32) dmaflush ? MTHCA_MR_DMAFLUSH : 0;
+		cmd_size = sizeof(struct mthca_reg_mr_abi_ver_2);
+	} else 
+		cmd_size = sizeof(struct ibv_reg_mr);
+
 #ifdef IBV_CMD_REG_MR_HAS_RESP_PARAMS
 	{
 		struct ibv_reg_mr_resp resp;
 
 		ret = ibv_cmd_reg_mr(pd, addr, length, hca_va, access, mr,
-				     &cmd, sizeof cmd, &resp, sizeof resp);
+				     &cmd.ibv_cmd, cmd_size, &resp, 
+				     sizeof resp);
 	}
 #else
 	ret = ibv_cmd_reg_mr(pd, addr, length, hca_va, access, mr,
-			     &cmd, sizeof cmd);
+			     &cmd.ibv_cmd, cmd_size);
 #endif
 	if (ret) {
 		free(mr);
@@ -149,7 +160,7 @@ static struct ibv_mr *__mthca_reg_mr(str
 struct ibv_mr *mthca_reg_mr(struct ibv_pd *pd, void *addr,
 			    size_t length, enum ibv_access_flags access)
 {
-	return __mthca_reg_mr(pd, addr, length, (uintptr_t) addr, access);
+	return __mthca_reg_mr(pd, addr, length, (uintptr_t) addr, access, 0);
 }
 
 int mthca_dereg_mr(struct ibv_mr *mr)
@@ -202,7 +213,7 @@ struct ibv_cq *mthca_create_cq(struct ib
 
 	cq->mr = __mthca_reg_mr(to_mctx(context)->pd, cq->buf.buf,
 				cqe * MTHCA_CQ_ENTRY_SIZE,
-				0, IBV_ACCESS_LOCAL_WRITE);
+				0, IBV_ACCESS_LOCAL_WRITE, 1);
 	if (!cq->mr)
 		goto err_buf;
 
@@ -294,7 +305,7 @@ int mthca_resize_cq(struct ibv_cq *ibcq,
 
 	mr = __mthca_reg_mr(to_mctx(ibcq->context)->pd, buf.buf,
 			    cqe * MTHCA_CQ_ENTRY_SIZE,
-			    0, IBV_ACCESS_LOCAL_WRITE);
+			    0, IBV_ACCESS_LOCAL_WRITE, 1);
 	if (!mr) {
 		mthca_free_buf(&buf);
 		ret = ENOMEM;
@@ -402,7 +413,7 @@ struct ibv_srq *mthca_create_srq(struct 
 	if (mthca_alloc_srq_buf(pd, &attr->attr, srq))
 		goto err;
 
-	srq->mr = __mthca_reg_mr(pd, srq->buf.buf, srq->buf_size, 0, 0);
+	srq->mr = __mthca_reg_mr(pd, srq->buf.buf, srq->buf_size, 0, 0, 0);
 	if (!srq->mr)
 		goto err_free;
 
@@ -520,7 +531,7 @@ struct ibv_qp *mthca_create_qp(struct ib
 	    pthread_spin_init(&qp->rq.lock, PTHREAD_PROCESS_PRIVATE))
 		goto err_free;
 
-	qp->mr = __mthca_reg_mr(pd, qp->buf.buf, qp->buf_size, 0, 0);
+	qp->mr = __mthca_reg_mr(pd, qp->buf.buf, qp->buf_size, 0, 0, 0);
 	if (!qp->mr)
 		goto err_free;
 
-- 
Arthur




More information about the general mailing list