diff mbox series

[rdma-core,10/10] verbs: use the execute_cmd_write/_ex path for the ioctl fallback

Message ID 20181122232416.30132-11-jgg@ziepe.ca (mailing list archive)
State Not Applicable
Headers show
Series Command execution rework | expand

Commit Message

Jason Gunthorpe Nov. 22, 2018, 11:24 p.m. UTC
From: Jason Gunthorpe <jgg@mellanox.com>

This is slightly less efficient than the old path, but it creates only two
places where write() are called.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 libibverbs/cmd_cq.c       |  6 ++-
 libibverbs/cmd_fallback.c | 68 +-------------------------------
 libibverbs/cmd_write.h    | 83 +++++++++++++++++++++------------------
 3 files changed, 51 insertions(+), 106 deletions(-)
diff mbox series

Patch

diff --git a/libibverbs/cmd_cq.c b/libibverbs/cmd_cq.c
index 03e5ca4c5e13bf..542daa7b63936f 100644
--- a/libibverbs/cmd_cq.c
+++ b/libibverbs/cmd_cq.c
@@ -69,7 +69,8 @@  static int ibv_icmd_create_cq(struct ibv_context *context, int cqe,
 			.comp_channel = channel ? channel->fd : -1,
 		};
 
-		ret = execute_write_bufs(cq->context, req, resp);
+		ret = execute_write_bufs(
+			cq->context, IB_USER_VERBS_CMD_CREATE_CQ, req, resp);
 		if (ret)
 			return ret;
 
@@ -90,7 +91,8 @@  static int ibv_icmd_create_cq(struct ibv_context *context, int cqe,
 			.flags = flags,
 		};
 
-		ret = execute_write_bufs_ex(cq->context, req);
+		ret = execute_write_bufs_ex(
+			cq->context, IB_USER_VERBS_EX_CMD_CREATE_CQ, req, resp);
 		if (ret)
 			return ret;
 
diff --git a/libibverbs/cmd_fallback.c b/libibverbs/cmd_fallback.c
index cad8b21756a6a6..0e50503e003484 100644
--- a/libibverbs/cmd_fallback.c
+++ b/libibverbs/cmd_fallback.c
@@ -120,8 +120,7 @@  enum write_fallback _execute_ioctl_fallback(struct ibv_context *ctx,
  * directly before the UHW memory (see _write_set_uhw)
  */
 void *_write_get_req(struct ibv_command_buffer *link,
-		     struct ib_uverbs_cmd_hdr *onstack, size_t size,
-		     uint32_t cmdnum)
+		     struct ib_uverbs_cmd_hdr *onstack, size_t size)
 {
 	struct ib_uverbs_cmd_hdr *hdr;
 
@@ -139,13 +138,11 @@  void *_write_get_req(struct ibv_command_buffer *link,
 		hdr->in_words = __check_divide(size, 4);
 	}
 
-	hdr->command = cmdnum;
-
 	return hdr + 1;
 }
 
 void *_write_get_req_ex(struct ibv_command_buffer *link, struct ex_hdr *onstack,
-			size_t size, uint32_t cmdnum)
+			size_t size)
 {
 	struct ex_hdr *hdr;
 	size_t full_size = size + sizeof(*hdr);
@@ -156,17 +153,12 @@  void *_write_get_req_ex(struct ibv_command_buffer *link, struct ex_hdr *onstack,
 		assert(uhw->attr_id == UVERBS_ATTR_UHW_IN);
 		assert(link->uhw_in_headroom_dwords * 4 >= full_size);
 		hdr = (void *)((uintptr_t)uhw->data - full_size);
-		hdr->hdr.in_words = __check_divide(size, 8);
 		hdr->ex_hdr.provider_in_words = __check_divide(uhw->len, 8);
 	} else {
 		hdr = onstack;
-		hdr->hdr.in_words = __check_divide(size, 8);
 		hdr->ex_hdr.provider_in_words = 0;
 	}
 
-	hdr->hdr.command = IB_USER_VERBS_CMD_FLAG_EXTENDED | cmdnum;
-	hdr->ex_hdr.cmd_hdr_reserved = 0;
-
 	return hdr + 1;
 }
 
@@ -205,46 +197,15 @@  void *_write_get_resp_ex(struct ibv_command_buffer *link,
 		assert(uhw->attr_id == UVERBS_ATTR_UHW_OUT);
 		assert(link->uhw_out_headroom_dwords * 4 >= resp_size);
 		resp_start = (void *)((uintptr_t)uhw->data - resp_size);
-		hdr->hdr.out_words = __check_divide(resp_size, 8);
 		hdr->ex_hdr.provider_out_words = __check_divide(uhw->len, 8);
 	} else {
 		resp_start = onstack;
-		hdr->hdr.out_words = __check_divide(resp_size, 8);
 		hdr->ex_hdr.provider_out_words = 0;
 	}
 
-	hdr->ex_hdr.response = ioctl_ptr_to_u64(resp_start);
-
 	return resp_start;
 }
 
-int _execute_write_raw(struct ibv_context *ctx, struct ib_uverbs_cmd_hdr *hdr,
-		       void *resp)
-{
-	/*
-	 * Users assumes the stack buffer is zeroed before passing to the
-	 * kernel for writing.
-	 */
-	if (resp) {
-		/*
-		 * The helper macros prove the response is at offset 0 of the
-		 * request.
-		 */
-		uint64_t *response = (uint64_t *)(hdr + 1);
-
-		*response = ioctl_ptr_to_u64(resp);
-		memset(resp, 0, hdr->out_words * 4);
-	}
-
-	if (write(ctx->cmd_fd, hdr, hdr->in_words * 4) != hdr->in_words * 4)
-		return errno;
-
-	if (resp)
-		VALGRIND_MAKE_MEM_DEFINED(resp, hdr->out_words * 4);
-
-	return 0;
-}
-
 int _execute_cmd_write(struct ibv_context *ctx, unsigned int write_method,
 		       struct ib_uverbs_cmd_hdr *req, size_t core_req_size,
 		       size_t req_size, void *resp, size_t core_resp_size,
@@ -262,31 +223,6 @@  int _execute_cmd_write(struct ibv_context *ctx, unsigned int write_method,
 	return 0;
 }
 
-int _execute_write_raw_ex(struct ibv_context *ctx, struct ex_hdr *hdr)
-{
-	size_t write_bytes =
-		sizeof(*hdr) +
-		(hdr->hdr.in_words + hdr->ex_hdr.provider_in_words) * 8;
-	size_t resp_bytes =
-		(hdr->hdr.out_words + hdr->ex_hdr.provider_out_words) * 8;
-	void *resp = (void *)(uintptr_t)hdr->ex_hdr.response;
-
-	/*
-	 * Users assumes the stack buffer is zeroed before passing to the
-	 * kernel for writing.
-	 */
-	if (resp)
-		memset(resp, 0, resp_bytes);
-
-	if (write(ctx->cmd_fd, hdr, write_bytes) != write_bytes)
-		return errno;
-
-	if (resp)
-		VALGRIND_MAKE_MEM_DEFINED(resp, resp_bytes);
-
-	return 0;
-}
-
 /*
  * req_size is the total length of the ex_hdr, core payload and driver data.
  * core_req_size is the total length of the ex_hdr and core_payload.
diff --git a/libibverbs/cmd_write.h b/libibverbs/cmd_write.h
index 88ff66600bc118..1d8c462522e2a9 100644
--- a/libibverbs/cmd_write.h
+++ b/libibverbs/cmd_write.h
@@ -40,21 +40,10 @@ 
 
 #include <stdbool.h>
 
-static inline struct ib_uverbs_cmd_hdr *get_req_hdr(void *req)
-{
-	return ((struct ib_uverbs_cmd_hdr *)req) - 1;
-}
-
-static inline struct ex_hdr *get_req_hdr_ex(void *req)
-{
-	return ((struct ex_hdr *)req) - 1;
-}
-
 void *_write_get_req(struct ibv_command_buffer *link,
-		     struct ib_uverbs_cmd_hdr *onstack, size_t size,
-		     uint32_t cmdnum);
+		     struct ib_uverbs_cmd_hdr *onstack, size_t size);
 void *_write_get_req_ex(struct ibv_command_buffer *link, struct ex_hdr *onstack,
-			size_t size, uint32_t cmdnum);
+			size_t size);
 void *_write_get_resp(struct ibv_command_buffer *link,
 		      struct ib_uverbs_cmd_hdr *hdr, void *onstack,
 		      size_t resp_size);
@@ -71,23 +60,26 @@  void *_write_get_resp_ex(struct ibv_command_buffer *link,
 #define DECLARE_LEGACY_UHW_BUFS(_link, _enum)                                  \
 	IBV_ABI_REQ(_enum) __req_onstack;                                      \
 	IBV_KABI_RESP(_enum) __resp_onstack;                                   \
-	static_assert(offsetof(IBV_KABI_REQ(_enum), response) == 0,            \
-		      "Bad response offset");                                  \
-	IBV_KABI_REQ(_enum) *req = _write_get_req(_link, &__req_onstack.hdr,   \
-						  sizeof(*req), _enum);        \
+	IBV_KABI_REQ(_enum) *req =                                             \
+		_write_get_req(_link, &__req_onstack.hdr, sizeof(*req));       \
 	IBV_KABI_RESP(_enum) *resp = ({                                        \
-		void *_resp = _write_get_resp(_link, get_req_hdr(req),         \
-					      &__resp_onstack, sizeof(*resp)); \
+		void *_resp = _write_get_resp(                                 \
+			_link,                                                 \
+			&container_of(req, IBV_ABI_REQ(_enum), core_payload)   \
+				 ->hdr,                                        \
+			&__resp_onstack, sizeof(*resp));                       \
 		_resp;                                                         \
 	})
 
 #define DECLARE_LEGACY_UHW_BUFS_EX(_link, _enum)                               \
 	IBV_ABI_REQ(_enum) __req_onstack;                                      \
 	IBV_KABI_RESP(_enum) __resp_onstack;                                   \
-	IBV_KABI_REQ(_enum) *req = _write_get_req_ex(                          \
-		_link, &__req_onstack.hdr, sizeof(*req), _enum);               \
+	IBV_KABI_REQ(_enum) *req =                                             \
+		_write_get_req_ex(_link, &__req_onstack.hdr, sizeof(*req));    \
 	IBV_KABI_RESP(_enum) *resp = _write_get_resp_ex(                       \
-		_link, get_req_hdr_ex(req), &__resp_onstack, sizeof(*resp))
+		_link,                                                         \
+		&container_of(req, IBV_ABI_REQ(_enum), core_payload)->hdr,     \
+		&__resp_onstack, sizeof(*resp))
 
 /*
  * This macro is used to implement the compatibility command call wrappers.
@@ -143,20 +135,6 @@  enum write_fallback _execute_ioctl_fallback(struct ibv_context *ctx,
 #define execute_ioctl_fallback(ctx, cmd_name, cmdb, ret)                       \
 	_execute_ioctl_fallback(ctx, _CMD_BIT(cmd_name), cmdb, ret)
 
-/* These helpers replace the raw write() and IBV_INIT_CMD macros */
-int _execute_write_raw(struct ibv_context *ctx, struct ib_uverbs_cmd_hdr *req,
-		       void *resp);
-
-/* For users of DECLARE_LEGACY_UHW_BUFS */
-#define execute_write_bufs(ctx, req, resp)                             \
-		_execute_write_raw(ctx, get_req_hdr(req), resp)
-
-int _execute_write_raw_ex(struct ibv_context *ctx, struct ex_hdr *req);
-
-/* For users of DECLARE_LEGACY_UHW_BUFS_EX */
-#define execute_write_bufs_ex(ctx, req)                                        \
-	_execute_write_raw_ex(ctx, get_req_hdr_ex(req))
-
 /*
  * For write() only commands that have fixed core structures and may take uhw
  * driver data. The last arguments are the same ones passed into the typical
@@ -206,6 +184,18 @@  int _execute_cmd_write(struct ibv_context *ctx, unsigned int write_method,
 			resp_size, resp_size);                                 \
 	})
 
+/*
+ * For users of DECLARE_LEGACY_UHW_BUFS, in this case the machinery has
+ * already stored the full req/resp length in the hdr.
+ */
+#define execute_write_bufs(ctx, enum, req, resp)                               \
+	({                                                                     \
+		IBV_ABI_REQ(enum) *_hdr =                                      \
+			container_of(req, IBV_ABI_REQ(enum), core_payload);    \
+		execute_cmd_write(ctx, enum, _hdr, _hdr->hdr.in_words * 4,     \
+				  resp, _hdr->hdr.out_words * 4);              \
+	})
+
 /*
  * For write() commands that use the _ex protocol. _full allows the caller to
  * specify all 4 sizes directly. This version is used when the core structs
@@ -236,6 +226,20 @@  int _execute_cmd_write_ex(struct ibv_context *ctx, unsigned int write_method,
 			sizeof(*(cmd)), cmd_size, NULL, 0, 0);                 \
 	})
 
+/* For users of DECLARE_LEGACY_UHW_BUFS_EX */
+#define execute_write_bufs_ex(ctx, enum, req, resp)                            \
+	({                                                                     \
+		IBV_ABI_REQ(enum) *_hdr =                                      \
+			container_of(req, IBV_ABI_REQ(enum), core_payload);    \
+		execute_cmd_write_ex(                                          \
+			ctx, enum, _hdr,                                       \
+			sizeof(*_hdr) +                                        \
+				_hdr->hdr.ex_hdr.provider_in_words * 8,        \
+			resp,                                                  \
+			sizeof(*(resp)) +                                      \
+				_hdr->hdr.ex_hdr.provider_out_words * 8);      \
+	})
+
 /*
  * These two macros are used only with execute_ioctl_fallback - they allow the
  * IOCTL code to be elided by the compiler when disabled.
@@ -263,14 +267,17 @@  _execute_ioctl_only(struct ibv_context *context, struct ibv_command_buffer *cmd,
 	_execute_ioctl_only(ctx, cmdb, ret)
 
 #undef execute_write_bufs
-static inline int execute_write_bufs(struct ibv_context *ctx, void *req,
+static inline int execute_write_bufs(struct ibv_context *ctx,
+				     unsigned int write_command, void *req,
 				     void *resp)
 {
 	return ENOSYS;
 }
 
 #undef execute_write_bufs_ex
-static inline int execute_write_bufs_ex(struct ibv_context *ctx, void *req)
+static inline int execute_write_bufs_ex(struct ibv_context *ctx,
+					unsigned int write_command, void *req,
+					void *resp)
 {
 	return ENOSYS;
 }