[systemd-devel] [RFC 5/8] HACK1: optimize copy operations in kdbus_conn_queue_alloc()

AKASHI Takahiro takahiro.akashi at linaro.org
Wed Jun 25 02:13:34 PDT 2014


This function copies the reply message directly into receiver's buffer
(pool slice). Looking into the implementation closely, however, there are
multiple occurrences of copy operations, kdbus_pool_slice_copy(), which
then executes costly prolog/epilog functions, say set_fs() and
write_begin/write_end(), on each entry/exit.

This patch uses a temporary buffer on the stack to compose a message and
replace a costly function, kdbus_pool_slice_copy(), to memcpy() wherever
possible, although I have to further confirm that this change is actually
safe.

Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
---
 connection.c |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 pool.c       |    2 +-
 pool.h       |    3 ++
 3 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/connection.c b/connection.c
index 40cf133..d122be8 100644
--- a/connection.c
+++ b/connection.c
@@ -39,6 +39,10 @@
 #include "policy.h"
 #include "util.h"
 
+/*
+ * KDBUS_HACK1: optimize copy operations in kdbus_conn_queue_alloc()
+ */
+
 struct kdbus_conn_reply;
 
 /**
@@ -232,7 +236,11 @@ exit_unref:
 static int kdbus_conn_payload_add(struct kdbus_conn *conn,
 				  struct kdbus_conn_queue *queue,
 				  const struct kdbus_kmsg *kmsg,
+#if KDBUS_HACK1
+				  void *buf, size_t items, size_t vec_data)
+#else
 				  size_t items, size_t vec_data)
+#endif
 {
 	const struct kdbus_item *item;
 	int ret;
@@ -253,8 +261,13 @@ static int kdbus_conn_payload_add(struct kdbus_conn *conn,
 			    KDBUS_ITEMS_SIZE(&kmsg->msg, items)) {
 		switch (item->type) {
 		case KDBUS_ITEM_PAYLOAD_VEC: {
+#if KDBUS_HACK1
+			u64 tmp[(KDBUS_ITEM_HEADER_SIZE +
+				 sizeof(struct kdbus_vec) + 7)/8];
+#else
 			char tmp[KDBUS_ITEM_HEADER_SIZE +
 				 sizeof(struct kdbus_vec)];
+#endif
 			struct kdbus_item *it = (struct kdbus_item *)tmp;
 
 			/* add item */
@@ -267,10 +280,14 @@ static int kdbus_conn_payload_add(struct kdbus_conn *conn,
 			else
 				it->vec.offset = ~0ULL;
 			it->vec.size = item->vec.size;
+#if KDBUS_HACK1
+			memcpy(buf + items, it, (size_t)it->size);
+#else
 			ret = kdbus_pool_slice_copy(queue->slice, items,
 						    it, it->size);
 			if (ret < 0)
 				return ret;
+#endif
 			items += KDBUS_ALIGN8(it->size);
 
 			/* \0-bytes record */
@@ -286,17 +303,28 @@ static int kdbus_conn_payload_add(struct kdbus_conn *conn,
 				 * null-bytes to the buffer which the \0-bytes
 				 * record would have shifted the alignment.
 				 */
+#if KDBUS_HACK1
+				memset(buf + vec_data, 0, pad);
+#else
 				kdbus_pool_slice_copy(queue->slice, vec_data,
 						      "\0\0\0\0\0\0\0", pad);
+#endif
 				vec_data += pad;
 				break;
 			}
 
 			/* copy kdbus_vec data from sender to receiver */
+#if KDBUS_HACK1
+			if (copy_from_user(buf + vec_data,
+				KDBUS_PTR(item->vec.address),
+				(size_t)item->vec.size))
+				return -EFAULT;
+#else
 			ret = kdbus_pool_slice_copy_user(queue->slice, vec_data,
 				KDBUS_PTR(item->vec.address), item->vec.size);
 			if (ret < 0)
 				return ret;
+#endif
 
 			vec_data += item->vec.size;
 			break;
@@ -314,10 +342,14 @@ static int kdbus_conn_payload_add(struct kdbus_conn *conn,
 			it->size = sizeof(tmp);
 			it->memfd.size = item->memfd.size;
 			it->memfd.fd = -1;
+#if KDBUS_HACK1
+			memcpy(buf + items, it, (size_t)it->size);
+#else
 			ret = kdbus_pool_slice_copy(queue->slice, items,
 						    it, it->size);
 			if (ret < 0)
 				return ret;
+#endif
 
 			/* grab reference of incoming file */
 			ret = kdbus_conn_memfd_ref(item, &fp);
@@ -459,7 +491,13 @@ static int kdbus_conn_queue_alloc(struct kdbus_conn *conn,
 	size_t fds = 0;
 	size_t meta = 0;
 	size_t vec_data;
+#if KDBUS_HACK1
+	size_t want;
+	char *tmp;
+	struct kdbus_item *it;
+#else
 	size_t want, have;
+#endif
 	int ret = 0;
 
 	BUG_ON(!mutex_is_locked(&conn->lock));
@@ -518,6 +556,13 @@ static int kdbus_conn_queue_alloc(struct kdbus_conn *conn,
 
 	/* do not give out more than half of the remaining space */
 	want = vec_data + kmsg->vecs_size;
+#if KDBUS_HACK1
+	ret = kdbus_pool_slice_alloc(conn->pool, &queue->slice, want);
+	if (ret < 0) {
+		ret = -EXFULL;
+		goto exit;
+	}
+#else
 	have = kdbus_pool_remain(conn->pool);
 	if (want < have && want > have / 2) {
 		ret = -EXFULL;
@@ -528,7 +573,59 @@ static int kdbus_conn_queue_alloc(struct kdbus_conn *conn,
 	ret = kdbus_pool_slice_alloc(conn->pool, &queue->slice, want);
 	if (ret < 0)
 		goto exit;
+#endif
+
+#if KDBUS_HACK1
+	tmp = kmalloc(vec_data, GFP_KERNEL);
+
+	/* copy the message header */
+	memcpy(tmp, &kmsg->msg, size);
+	/* update the size */
+	*(size_t *)tmp = msg_size;
 
+	if (dst_name_len  > 0) {
+		it = (struct kdbus_item *)&tmp[size];
+		it->size = KDBUS_ITEM_HEADER_SIZE + dst_name_len;
+		it->type = KDBUS_ITEM_DST_NAME;
+		memcpy(it->str, kmsg->dst_name, dst_name_len);
+	}
+
+	/* add PAYLOAD items */
+	if (payloads > 0) {
+		ret = kdbus_conn_payload_add(conn, queue, kmsg,
+					     tmp, payloads, vec_data);
+		if (ret < 0)
+			goto exit_pool_free;
+	}
+
+	/* add a FDS item; the array content will be updated at RECV time */
+	if (kmsg->fds_count > 0) {
+		it = (struct kdbus_item *)&tmp[fds];
+		it->type = KDBUS_ITEM_FDS;
+		it->size = KDBUS_ITEM_HEADER_SIZE +
+			   (kmsg->fds_count * sizeof(int));
+		memcpy(tmp + fds, it, KDBUS_ITEM_HEADER_SIZE);
+
+		ret = kdbus_conn_fds_ref(queue, kmsg->fds, kmsg->fds_count);
+		if (ret < 0)
+			goto exit_pool_free;
+
+		/* remember the array to update at RECV */
+		queue->fds = fds + offsetof(struct kdbus_item, fds);
+		queue->fds_count = kmsg->fds_count;
+	}
+
+	/* append message metadata/credential items */
+	if (meta > 0) {
+		memcpy(tmp + meta, kmsg->meta->data, kmsg->meta->size);
+	}
+
+	/* finally */
+	ret = kdbus_pool_copy(queue->slice, 0, tmp, NULL, 0, want);
+	kfree(tmp);
+	if (ret < 0)
+		goto exit_pool_free;
+#else
 	/* copy the message header */
 	ret = kdbus_pool_slice_copy(queue->slice, 0, &kmsg->msg, size);
 	if (ret < 0)
@@ -591,6 +688,7 @@ static int kdbus_conn_queue_alloc(struct kdbus_conn *conn,
 		if (ret < 0)
 			goto exit_pool_free;
 	}
+#endif
 
 	queue->priority = kmsg->msg.priority;
 	*q = queue;
diff --git a/pool.c b/pool.c
index 4267c54..fcc54af 100644
--- a/pool.c
+++ b/pool.c
@@ -462,7 +462,7 @@ static int kdbus_pool_copy_data(struct page *p, size_t start,
 }
 
 /* copy data to the receiver's pool */
-static size_t kdbus_pool_copy(const struct kdbus_pool_slice *slice, size_t off,
+size_t kdbus_pool_copy(const struct kdbus_pool_slice *slice, size_t off,
 			      const void __user *data, struct file *f_src,
 			      size_t off_src, size_t len)
 {
diff --git a/pool.h b/pool.h
index d1718eb..5b89d31 100644
--- a/pool.h
+++ b/pool.h
@@ -30,6 +30,9 @@ void kdbus_pool_slice_free(struct kdbus_pool_slice *slice);
 struct kdbus_pool_slice *kdbus_pool_slice_find(struct kdbus_pool *pool,
 					       size_t off);
 size_t kdbus_pool_slice_offset(const struct kdbus_pool_slice *slice);
+size_t kdbus_pool_copy(const struct kdbus_pool_slice *slice, size_t off,
+			      const void __user *data, struct file *f_src,
+			      size_t off_src, size_t len);
 ssize_t kdbus_pool_slice_copy(const struct kdbus_pool_slice *slice, size_t off,
 			       const void *data, size_t len);
 ssize_t
-- 
1.7.9.5



More information about the systemd-devel mailing list