From 8e959fbf3862172b53d200cda659795c3744fa78 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 18 Dec 2013 18:46:23 +0100 Subject: [PATCH 1/1] bus: reduce calls to KDBUS_CMD_MEMFD_SIZE_SET ioctl Instead of calling it for each buffer append, increase allocation exponentially and set the real value only at the end, when sealing off the memfd. This should drastically reduce the number of times we invoke the ioctl(). --- src/libsystemd-bus/bus-kernel.c | 39 +++++++++++++---------- src/libsystemd-bus/bus-kernel.h | 7 +++-- src/libsystemd-bus/bus-message.c | 54 +++++++++++++++++++++++--------- src/libsystemd-bus/bus-message.h | 1 + 4 files changed, 66 insertions(+), 35 deletions(-) diff --git a/src/libsystemd-bus/bus-kernel.c b/src/libsystemd-bus/bus-kernel.c index bd6b84a7b..7d514616a 100644 --- a/src/libsystemd-bus/bus-kernel.c +++ b/src/libsystemd-bus/bus-kernel.c @@ -919,12 +919,13 @@ int bus_kernel_read_message(sd_bus *bus) { return r < 0 ? r : 1; } -int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *size) { +int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *mapped, size_t *allocated) { struct memfd_cache *c; int fd; assert(address); - assert(size); + assert(mapped); + assert(allocated); if (!bus || !bus->is_kernel) return -ENOTSUP; @@ -941,17 +942,19 @@ int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *size) { return -errno; *address = NULL; - *size = 0; + *mapped = 0; + *allocated = 0; return fd; } c = &bus->memfd_cache[--bus->n_memfd_cache]; assert(c->fd >= 0); - assert(c->size == 0 || c->address); + assert(c->mapped == 0 || c->address); *address = c->address; - *size = c->size; + *mapped = c->mapped; + *allocated = c->allocated; fd = c->fd; assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0); @@ -966,15 +969,15 @@ static void close_and_munmap(int fd, void *address, size_t size) { close_nointr_nofail(fd); } -void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t size) { +void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t mapped, size_t allocated) { struct memfd_cache *c; - uint64_t max_sz = PAGE_ALIGN(MEMFD_CACHE_ITEM_SIZE_MAX); + uint64_t max_mapped = PAGE_ALIGN(MEMFD_CACHE_ITEM_SIZE_MAX); assert(fd >= 0); - assert(size == 0 || address); + assert(mapped == 0 || address); if (!bus || !bus->is_kernel) { - close_and_munmap(fd, address, size); + close_and_munmap(fd, address, mapped); return; } @@ -983,7 +986,7 @@ void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t size) { if (bus->n_memfd_cache >= ELEMENTSOF(bus->memfd_cache)) { assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0); - close_and_munmap(fd, address, size); + close_and_munmap(fd, address, mapped); return; } @@ -992,12 +995,14 @@ void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t size) { c->address = address; /* If overly long, let's return a bit to the OS */ - if (size > max_sz) { - assert_se(ioctl(fd, KDBUS_CMD_MEMFD_SIZE_SET, &max_sz) >= 0); - assert_se(munmap((uint8_t*) address + max_sz, PAGE_ALIGN(size - max_sz)) >= 0); - c->size = max_sz; - } else - c->size = size; + if (mapped > max_mapped) { + assert_se(ioctl(fd, KDBUS_CMD_MEMFD_SIZE_SET, &max_mapped) >= 0); + assert_se(munmap((uint8_t*) address + max_mapped, PAGE_ALIGN(mapped - max_mapped)) >= 0); + c->mapped = c->allocated = max_mapped; + } else { + c->mapped = mapped; + c->allocated = allocated; + } assert_se(pthread_mutex_unlock(&bus->memfd_cache_mutex) >= 0); } @@ -1008,7 +1013,7 @@ void bus_kernel_flush_memfd(sd_bus *b) { assert(b); for (i = 0; i < b->n_memfd_cache; i++) - close_and_munmap(b->memfd_cache[i].fd, b->memfd_cache[i].address, b->memfd_cache[i].size); + close_and_munmap(b->memfd_cache[i].fd, b->memfd_cache[i].address, b->memfd_cache[i].mapped); } int kdbus_translate_request_name_flags(uint64_t flags, uint64_t *kdbus_flags) { diff --git a/src/libsystemd-bus/bus-kernel.h b/src/libsystemd-bus/bus-kernel.h index 8c7eacc6d..0a825d7b8 100644 --- a/src/libsystemd-bus/bus-kernel.h +++ b/src/libsystemd-bus/bus-kernel.h @@ -51,7 +51,8 @@ struct memfd_cache { int fd; void *address; - size_t size; + size_t mapped; + size_t allocated; }; int bus_kernel_connect(sd_bus *b); @@ -65,8 +66,8 @@ int bus_kernel_create_namespace(const char *name, char **s); int bus_kernel_create_starter(const char *bus, const char *name); int bus_kernel_create_monitor(const char *bus); -int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *size); -void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t size); +int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *mapped, size_t *allocated); +void bus_kernel_push_memfd(sd_bus *bus, int fd, void *address, size_t mapped, size_t allocated); void bus_kernel_flush_memfd(sd_bus *bus); diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c index 1a461e62e..d6888738e 100644 --- a/src/libsystemd-bus/bus-message.c +++ b/src/libsystemd-bus/bus-message.c @@ -65,7 +65,7 @@ static void message_free_part(sd_bus_message *m, struct bus_body_part *part) { * can't be sealed yet. */ if (!part->sealed) - bus_kernel_push_memfd(m->bus, part->memfd, part->data, part->mapped); + bus_kernel_push_memfd(m->bus, part->memfd, part->data, part->mapped, part->allocated); else { if (part->mapped > 0) assert_se(munmap(part->data, part->mapped) == 0); @@ -1049,20 +1049,27 @@ static int part_make_space( return -ENOMEM; if (!part->data && part->memfd < 0) - part->memfd = bus_kernel_pop_memfd(m->bus, &part->data, &part->mapped); + part->memfd = bus_kernel_pop_memfd(m->bus, &part->data, &part->mapped, &part->allocated); if (part->memfd >= 0) { - uint64_t u = sz; - r = ioctl(part->memfd, KDBUS_CMD_MEMFD_SIZE_SET, &u); - if (r < 0) { - m->poisoned = true; - return -errno; + if (part->allocated == 0 || sz > part->allocated) { + uint64_t new_allocated; + + new_allocated = PAGE_ALIGN(sz > 0 ? 2 * sz : 1); + r = ioctl(part->memfd, KDBUS_CMD_MEMFD_SIZE_SET, &new_allocated); + if (r < 0) { + m->poisoned = true; + return -errno; + } + + part->allocated = new_allocated; } if (!part->data || sz > part->mapped) { - size_t psz = PAGE_ALIGN(sz > 0 ? sz : 1); + size_t psz; + psz = PAGE_ALIGN(sz > 0 ? sz : 1); if (part->mapped <= 0) n = mmap(NULL, psz, PROT_READ|PROT_WRITE, MAP_SHARED, part->memfd, 0); else @@ -1079,14 +1086,20 @@ static int part_make_space( part->munmap_this = true; } else { - n = realloc(part->data, MAX(sz, 1u)); - if (!n) { - m->poisoned = true; - return -ENOMEM; - } + if (part->allocated == 0 || sz > part->allocated) { + size_t new_allocated; + + new_allocated = sz > 0 ? 2 * sz : 64; + n = realloc(part->data, new_allocated); + if (!n) { + m->poisoned = true; + return -ENOMEM; + } - part->data = n; - part->free_this = true; + part->data = n; + part->allocated = new_allocated; + part->free_this = true; + } } if (q) @@ -2757,8 +2770,19 @@ int bus_message_seal(sd_bus_message *m, uint64_t serial, usec_t timeout) { if (m->destination && m->bus && m->bus->use_memfd) { MESSAGE_FOREACH_PART(part, i, m) if (part->memfd >= 0 && !part->sealed && (part->size > MEMFD_MIN_SIZE || m->bus->use_memfd < 0)) { + uint64_t sz; + + /* Try to seal it if that makes + * sense. First, unmap our own map to + * make sure we don't keep it busy. */ bus_body_part_unmap(part); + /* Then, sync up real memfd size */ + sz = part->size; + if (ioctl(part->memfd, KDBUS_CMD_MEMFD_SIZE_SET, &sz) < 0) + return -errno; + + /* Finally, try to seal */ if (ioctl(part->memfd, KDBUS_CMD_MEMFD_SEAL_SET, 1) >= 0) part->sealed = true; } diff --git a/src/libsystemd-bus/bus-message.h b/src/libsystemd-bus/bus-message.h index a9d42c345..cd1fb0e54 100644 --- a/src/libsystemd-bus/bus-message.h +++ b/src/libsystemd-bus/bus-message.h @@ -65,6 +65,7 @@ struct bus_body_part { void *data; size_t size; size_t mapped; + size_t allocated; int memfd; bool free_this:1; bool munmap_this:1; -- 2.30.2