[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH gnumach] x86_64: avoid iterating over the message twice in copyou
|
From: |
Flavio Cruz |
|
Subject: |
[PATCH gnumach] x86_64: avoid iterating over the message twice in copyoutmsg/copyinmsg for faster RPCs. |
|
Date: |
Mon, 19 Feb 2024 23:58:00 -0500 |
This is a follow up to
https://git.savannah.gnu.org/cgit/hurd/gnumach.git/commit/?id=69620634858b2992e1a362e33c95d9a8ee57bce7
where we made inlined ports 8 bytes long to avoid resizing.
The last thing that copy{in,out}msg were doing was just updating
msgt_size field since that's required for kernel stub code and implicitly
assumed by IPC code. This was moved into ipc_kmsg_copy{in,out}_body.
For a 32 bit userland, the code also stops updating
msgt_size for out of line ports, same as the 64 bit userland.
---
i386/i386/locore.h | 6 +++
ipc/ipc_kmsg.c | 52 +++++++++++++------
x86_64/copy_user.c | 123 +++++++++------------------------------------
3 files changed, 66 insertions(+), 115 deletions(-)
diff --git a/i386/i386/locore.h b/i386/i386/locore.h
index 374c8cf..217e6de 100644
--- a/i386/i386/locore.h
+++ b/i386/i386/locore.h
@@ -50,7 +50,13 @@ extern int discover_x86_cpu_type (void);
extern int copyin (const void *userbuf, void *kernelbuf, size_t cn);
extern int copyinmsg (const void *userbuf, void *kernelbuf, size_t cn, size_t
kn);
extern int copyout (const void *kernelbuf, void *userbuf, size_t cn);
+#ifdef USER32
extern int copyoutmsg (const void *kernelbuf, void *userbuf, size_t cn);
+#else
+static inline int copyoutmsg (const void *kernelbuf, void *userbuf, size_t cn)
{
+ return copyout (kernelbuf, userbuf, cn);
+}
+#endif
extern int inst_fetch (int eip, int cs);
diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
index bd84380..6a7ad3c 100644
--- a/ipc/ipc_kmsg.c
+++ b/ipc/ipc_kmsg.c
@@ -1321,7 +1321,7 @@ ipc_kmsg_copyin_body(
mach_msg_type_number_t number;
boolean_t is_inline, longform, dealloc, is_port;
vm_offset_t data;
- uint64_t length;
+ vm_size_t length;
kern_return_t kr;
type = (mach_msg_type_long_t *) saddr;
@@ -1355,7 +1355,8 @@ ipc_kmsg_copyin_body(
is_port = MACH_MSG_TYPE_PORT_ANY(name);
- if ((is_port && (size != PORT_T_SIZE_IN_BITS)) ||
+ if ((is_port && !is_inline && (size !=
PORT_NAME_T_SIZE_IN_BITS)) ||
+ (is_port && is_inline && (size != PORT_T_SIZE_IN_BITS)) ||
#ifndef __x86_64__
(longform && ((type->msgtl_header.msgt_name != 0) ||
(type->msgtl_header.msgt_size != 0) ||
@@ -1396,11 +1397,22 @@ ipc_kmsg_copyin_body(
if (length == 0)
data = 0;
else if (is_port) {
+ const vm_size_t user_length = length;
+ /*
+ * In 64 bit architectures, out of line port
names are
+ * represented as an array of mach_port_name_t
which are
+ * smaller than mach_port_t.
+ */
+ if (sizeof(mach_port_name_t) !=
sizeof(mach_port_t)) {
+ length = sizeof(mach_port_t) * number;
+ type->msgtl_size = sizeof(mach_port_t)
* 8;
+ }
+
data = kalloc(length);
if (data == 0)
goto invalid_memory;
- if (sizeof(mach_port_name_t) !=
sizeof(mach_port_t))
+ if (user_length != length)
{
mach_port_name_t *src =
(mach_port_name_t*)addr;
mach_port_t *dst = (mach_port_t*)data;
@@ -1416,7 +1428,7 @@ ipc_kmsg_copyin_body(
goto invalid_memory;
}
if (dealloc &&
- (vm_deallocate(map, addr, length) !=
KERN_SUCCESS)) {
+ (vm_deallocate(map, addr, user_length) !=
KERN_SUCCESS)) {
kfree(data, length);
goto invalid_memory;
}
@@ -2372,7 +2384,7 @@ ipc_kmsg_copyout_body(
mach_msg_type_size_t size;
mach_msg_type_number_t number;
boolean_t is_inline, longform, is_port;
- uint64_t length;
+ vm_size_t length;
vm_offset_t addr;
type = (mach_msg_type_long_t *) saddr;
@@ -2406,18 +2418,28 @@ ipc_kmsg_copyout_body(
ipc_object_t *objects;
mach_msg_type_number_t i;
- if (!is_inline && (length != 0)) {
- /* first allocate memory in the map */
- uint64_t allocated = length;
+ if (!is_inline) {
+ if (length != 0) {
+ vm_size_t user_length = length;
- _Static_assert(sizeof(mach_port_name_t) <=
sizeof(mach_port_t),
- "Size of mach_port_t should be
equal or larger than mach_port_name_t.");
- allocated -= (sizeof(mach_port_t) -
sizeof(mach_port_name_t)) * number;
+ if (sizeof(mach_port_name_t) !=
sizeof(mach_port_t)) {
+ user_length =
sizeof(mach_port_name_t) * number;
+ }
- kr = vm_allocate(map, &addr, allocated, TRUE);
- if (kr != KERN_SUCCESS) {
- ipc_kmsg_clean_body(taddr, saddr);
- goto vm_copyout_failure;
+ /* first allocate memory in the map */
+ kr = vm_allocate(map, &addr,
user_length, TRUE);
+ if (kr != KERN_SUCCESS) {
+ ipc_kmsg_clean_body(taddr,
saddr);
+ goto vm_copyout_failure;
+ }
+ }
+
+ if (sizeof(mach_port_name_t) !=
sizeof(mach_port_t)) {
+ /* Out of line ports are always
returned as mach_port_name_t.
+ * Note: we have to do this after
ipc_kmsg_clean_body, otherwise
+ * the cleanup function will not work
correctly.
+ */
+ type->msgtl_size =
sizeof(mach_port_name_t) * 8;
}
}
diff --git a/x86_64/copy_user.c b/x86_64/copy_user.c
index c6e125d..89acd81 100644
--- a/x86_64/copy_user.c
+++ b/x86_64/copy_user.c
@@ -49,10 +49,6 @@ typedef struct {
natural_t msgtl_number;
} mach_msg_user_type_long_t;
_Static_assert(sizeof(mach_msg_user_type_long_t) == 12);
-#else
-typedef mach_msg_type_t mach_msg_user_type_t;
-typedef mach_msg_type_long_t mach_msg_user_type_long_t;
-#endif /* USER32 */
/*
* Helper to unpack the relevant fields of a msg type; the fields are different
@@ -87,7 +83,6 @@ static inline void unpack_msg_type(vm_offset_t addr,
}
}
-#ifdef USER32
static inline void mach_msg_user_type_to_kernel(const mach_msg_user_type_t *u,
mach_msg_type_t* k) {
k->msgt_name = u->msgt_name;
@@ -144,53 +139,33 @@ static inline void
mach_msg_kernel_type_to_user_long(const mach_msg_type_long_t
};
*u = user;
}
-#endif
static inline int copyin_mach_msg_type(const rpc_vm_offset_t *uaddr,
mach_msg_type_t *kaddr) {
-#ifdef USER32
mach_msg_user_type_t user;
- int ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_t));
- if (ret) {
- return ret;
- }
+ if (copyin(uaddr, &user, sizeof(mach_msg_user_type_t)))
+ return 1;
mach_msg_user_type_to_kernel(&user, kaddr);
return 0;
-#else
- return copyin(uaddr, kaddr, sizeof(mach_msg_type_t));
-#endif
}
static inline int copyout_mach_msg_type(const mach_msg_type_t *kaddr,
rpc_vm_offset_t *uaddr) {
-#ifdef USER32
mach_msg_user_type_t user;
mach_msg_kernel_type_to_user(kaddr, &user);
return copyout(&user, uaddr, sizeof(mach_msg_user_type_t));
-#else
- return copyout(kaddr, uaddr, sizeof(mach_msg_type_t));
-#endif
}
static inline int copyin_mach_msg_type_long(const rpc_vm_offset_t *uaddr,
mach_msg_type_long_t *kaddr) {
-#ifdef USER32
mach_msg_user_type_long_t user;
- int ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_long_t));
- if (ret)
- return ret;
+ if (copyin(uaddr, &user, sizeof(mach_msg_user_type_long_t)))
+ return 1;
mach_msg_user_type_to_kernel_long(&user, kaddr);
return 0;
-#else
- return copyin(uaddr, kaddr, sizeof(mach_msg_type_long_t));
-#endif
}
static inline int copyout_mach_msg_type_long(const mach_msg_type_long_t
*kaddr, rpc_vm_offset_t *uaddr) {
-#ifdef USER32
mach_msg_user_type_long_t user;
mach_msg_kernel_type_to_user_long(kaddr, &user);
return copyout(&user, uaddr, sizeof(mach_msg_user_type_long_t));
-#else
- return copyout(kaddr, uaddr, sizeof(mach_msg_type_long_t));
-#endif
}
/* Optimized version of unpack_msg_type(), including proper copyin() */
@@ -265,15 +240,8 @@ static inline int copyout_unpack_msg_type(vm_offset_t
kaddr,
mach_msg_type_size_t orig_size = kmtl->msgtl_size;
int ret;
- if (MACH_MSG_TYPE_PORT_ANY(kmtl->msgtl_name)) {
-#ifdef USER32
- kmtl->msgtl_size = bytes_to_descsize(sizeof(mach_port_name_t));
-#else
- /* 64 bit ABI uses mach_port_name_inlined_t for inlined ports. */
- if (!kmt->msgt_inline)
+ if (MACH_MSG_TYPE_PORT_ANY(kmtl->msgtl_name) && kmt->msgt_inline)
kmtl->msgtl_size = bytes_to_descsize(sizeof(mach_port_name_t));
-#endif
- }
ret = copyout_mach_msg_type_long(kmtl, (void*)uaddr);
kmtl->msgtl_size = orig_size;
if (ret)
@@ -289,21 +257,12 @@ static inline int copyout_unpack_msg_type(vm_offset_t
kaddr,
{
mach_msg_type_size_t orig_size = kmt->msgt_size;
int ret;
-
- if (MACH_MSG_TYPE_PORT_ANY(kmt->msgt_name)) {
-#ifdef USER32
+ if (MACH_MSG_TYPE_PORT_ANY(kmt->msgt_name) && kmt->msgt_inline)
kmt->msgt_size = bytes_to_descsize(sizeof(mach_port_name_t));
-#else
- /* 64 bit ABI uses mach_port_name_inlined_t for inlined ports. */
- if (!kmt->msgt_inline)
- kmt->msgt_size = bytes_to_descsize(sizeof(mach_port_name_t));
-#endif
- }
ret = copyout_mach_msg_type(kmt, (void *)uaddr);
kmt->msgt_size = orig_size;
if (ret)
return 1;
-
*name = kmt->msgt_name;
*size = kmt->msgt_size;
*number = kmt->msgt_number;
@@ -313,7 +272,6 @@ static inline int copyout_unpack_msg_type(vm_offset_t kaddr,
return 0;
}
-#ifdef USER32
/*
* Compute the user-space size of a message still in the kernel when processing
* messages from 32bit userland.
@@ -372,7 +330,7 @@ size_t msg_usize(const mach_msg_header_t *kmsg)
}
return usize;
}
-#endif /* USER32 */
+#endif /* USER32 */
/*
* Expand the msg header and, if required, the msg body (ports, pointers)
@@ -386,6 +344,9 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const
size_t usize, const s
const mach_msg_user_header_t *umsg = userbuf;
mach_msg_header_t *kmsg = kernelbuf;
+ _Static_assert(!mach_msg_user_is_misaligned(sizeof(mach_msg_user_header_t)),
+ "mach_msg_user_header_t needs to be MACH_MSG_USER_ALIGNMENT
aligned.");
+
#ifdef USER32
if (copyin(&umsg->msgh_bits, &kmsg->msgh_bits, sizeof(kmsg->msgh_bits)))
return 1;
@@ -397,23 +358,12 @@ int copyinmsg (const void *userbuf, void *kernelbuf,
const size_t usize, const s
if (copyin(&umsg->msgh_seqno, &kmsg->msgh_seqno,
sizeof(kmsg->msgh_seqno) + sizeof(kmsg->msgh_id)))
return 1;
-#else
- /* The 64 bit interface ensures the header is the same size, so it does not
need any resizing. */
- _Static_assert(sizeof(mach_msg_header_t) == sizeof(mach_msg_user_header_t),
- "mach_msg_header_t and mach_msg_user_header_t expected to be
of the same size");
- if (copyin(umsg, kmsg, sizeof(mach_msg_header_t)))
- return 1;
- kmsg->msgh_remote_port &= 0xFFFFFFFF; // FIXME: still have port names here
- kmsg->msgh_local_port &= 0xFFFFFFFF; // also, this assumes little-endian
-#endif
vm_offset_t usaddr, ueaddr, ksaddr;
ksaddr = (vm_offset_t)(kmsg + 1);
usaddr = (vm_offset_t)(umsg + 1);
ueaddr = (vm_offset_t)umsg + usize;
- _Static_assert(!mach_msg_user_is_misaligned(sizeof(mach_msg_user_header_t)),
- "mach_msg_user_header_t needs to be MACH_MSG_USER_ALIGNMENT
aligned.");
if (usize > sizeof(mach_msg_user_header_t))
{
@@ -441,7 +391,6 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const
size_t usize, const s
{
if (MACH_MSG_TYPE_PORT_ANY(name))
{
-#ifdef USER32
if (size != bytes_to_descsize(sizeof(mach_port_name_t)))
return 1;
if ((usaddr + sizeof(mach_port_name_t)*number) > ueaddr)
@@ -454,17 +403,6 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const
size_t usize, const s
ksaddr += sizeof(mach_port_t);
usaddr += sizeof(mach_port_name_t);
}
-#else
- if (size !=
bytes_to_descsize(sizeof(mach_port_name_inlined_t)))
- return 1;
- const vm_size_t length = number *
sizeof(mach_port_name_inlined_t);
- if ((usaddr + length) > ueaddr)
- return 1;
- if (copyin((void*)usaddr, (void*)ksaddr, length))
- return 1;
- usaddr += length;
- ksaddr += length;
-#endif
}
else
{
@@ -485,11 +423,9 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const
size_t usize, const s
/* out-of-line port arrays are always arrays of mach_port_name_t
(4 bytes)
* and are expanded in ipc_kmsg_copyin_body() */
- if (MACH_MSG_TYPE_PORT_ANY(name)) {
- if (size != bytes_to_descsize(sizeof(mach_port_name_t)))
- return 1;
- adjust_msg_type_size(ktaddr, sizeof(mach_port_t) -
sizeof(mach_port_name_t));
- }
+ if (MACH_MSG_TYPE_PORT_ANY(name) &&
+ (size != bytes_to_descsize(sizeof(mach_port_name_t))))
+ return 1;
if (copyin_address((rpc_vm_offset_t*)usaddr,
(vm_offset_t*)ksaddr))
return 1;
@@ -506,18 +442,24 @@ int copyinmsg (const void *userbuf, void *kernelbuf,
const size_t usize, const s
kmsg->msgh_size = sizeof(mach_msg_header_t) + ksaddr - (vm_offset_t)(kmsg +
1);
assert(kmsg->msgh_size <= ksize);
-#ifndef USER32
- if (kmsg->msgh_size != usize)
+#else
+ /* The 64 bit interface ensures the header is the same size, so it does not
need any resizing. */
+ _Static_assert(sizeof(mach_msg_header_t) == sizeof(mach_msg_user_header_t),
+ "mach_msg_header_t and mach_msg_user_header_t expected to be
of the same size");
+ if (copyin(umsg, kmsg, usize))
return 1;
+ kmsg->msgh_remote_port &= 0xFFFFFFFF; // FIXME: still have port names here
+ kmsg->msgh_local_port &= 0xFFFFFFFF; // also, this assumes little-endian
#endif
return 0;
}
+#ifdef USER32
+/* This is defined simply as copyout for !USER32. */
int copyoutmsg (const void *kernelbuf, void *userbuf, const size_t ksize)
{
const mach_msg_header_t *kmsg = kernelbuf;
mach_msg_user_header_t *umsg = userbuf;
-#ifdef USER32
if (copyout(&kmsg->msgh_bits, &umsg->msgh_bits, sizeof(kmsg->msgh_bits)))
return 1;
/* umsg->msgh_size is filled in later */
@@ -528,10 +470,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf,
const size_t ksize)
if (copyout(&kmsg->msgh_seqno, &umsg->msgh_seqno,
sizeof(kmsg->msgh_seqno) + sizeof(kmsg->msgh_id)))
return 1;
-#else
- if (copyout(kmsg, umsg, sizeof(mach_msg_header_t)))
- return 1;
-#endif /* USER32 */
vm_offset_t ksaddr, keaddr, usaddr;
ksaddr = (vm_offset_t)(kmsg + 1);
@@ -559,7 +497,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, const
size_t ksize)
{
if (MACH_MSG_TYPE_PORT_ANY(name))
{
-#ifdef USER32
for (int i=0; i<number; i++)
{
if (copyout_port((mach_port_t*)ksaddr,
(mach_port_name_t*)usaddr))
@@ -567,15 +504,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf,
const size_t ksize)
ksaddr += sizeof(mach_port_t);
usaddr += sizeof(mach_port_name_t);
}
-#else
- if (size !=
bytes_to_descsize(sizeof(mach_port_name_inlined_t)))
- return 1;
- const vm_size_t length = number *
sizeof(mach_port_name_inlined_t);
- if (copyout((void*)ksaddr, (void*)usaddr, length))
- return 1;
- ksaddr += length;
- usaddr += length;
-#endif
}
else
{
@@ -603,11 +531,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf,
const size_t ksize)
usize = sizeof(mach_msg_user_header_t) + usaddr - (vm_offset_t)(umsg + 1);
if (copyout(&usize, &umsg->msgh_size, sizeof(umsg->msgh_size)))
return 1;
-#ifndef USER32
- if (usize != ksize)
- return 1;
-#endif
-
return 0;
-
}
+#endif /* USER32 */
--
2.39.2
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [PATCH gnumach] x86_64: avoid iterating over the message twice in copyoutmsg/copyinmsg for faster RPCs.,
Flavio Cruz <=