From: Lennart Poettering Date: Thu, 16 May 2013 14:26:35 +0000 (+0200) Subject: bus: send memfds as payload only on directed messages and for large parts X-Git-Tag: v205~229 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=66b26c5c9b02e787bc46db24daff04ad41e05ec5 bus: send memfds as payload only on directed messages and for large parts --- diff --git a/src/libsystemd-bus/bus-kernel.c b/src/libsystemd-bus/bus-kernel.c index f7759b6fb..8ef5752b3 100644 --- a/src/libsystemd-bus/bus-kernel.c +++ b/src/libsystemd-bus/bus-kernel.c @@ -69,6 +69,10 @@ static void append_payload_vec(struct kdbus_item **d, const void *p, size_t sz) *d = ALIGN8_PTR(*d); + /* Note that p can be NULL, which encodes a region full of + * zeroes, which is useful to optimize certain padding + * conditions */ + (*d)->size = offsetof(struct kdbus_item, vec) + sizeof(struct kdbus_vec); (*d)->type = KDBUS_MSG_PAYLOAD_VEC; (*d)->vec.address = PTR_TO_UINT64(p); @@ -244,9 +248,12 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) { sz += ALIGN8(offsetof(struct kdbus_item, fds) + sizeof(int)*m->n_fds); m->kdbus = memalign(8, sz); - if (!m->kdbus) - return -ENOMEM; + if (!m->kdbus) { + r = -ENOMEM; + goto fail; + } + m->free_kdbus = true; memset(m->kdbus, 0, sz); m->kdbus->flags = @@ -269,24 +276,28 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) { MESSAGE_FOREACH_PART(part, i, m) { if (part->is_zero) { + /* If this is padding then simply send a + * vector with a NULL data pointer which the + * kernel will just pass through. This is the + * most efficient way to encode zeroes */ + append_payload_vec(&d, NULL, part->size); continue; } - if (part->memfd >= 0 && part->sealed) { - bus_body_part_unmap(part); + if (part->memfd >= 0 && part->sealed && m->destination) { + /* Try to send a memfd, if the part is + * sealed and this is not a broadcast. Since we can only */ - if (!part->data) { - append_payload_memfd(&d, part->memfd, part->size); - continue; - } + append_payload_memfd(&d, part->memfd, part->size); + continue; } - if (part->memfd >= 0) { - r = bus_body_part_map(part); - if (r < 0) - goto fail; - } + /* Otherwise let's send a vector to the actual data, + * for that we need to map it first. */ + r = bus_body_part_map(part); + if (r < 0) + goto fail; append_payload_vec(&d, part->data, part->size); } @@ -306,13 +317,10 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) { m->kdbus->size = (uint8_t*) d - (uint8_t*) m->kdbus; assert(m->kdbus->size <= sz); - m->free_kdbus = true; - return 0; fail: - free(m->kdbus); - m->kdbus = NULL; + m->poisoned = true; return r; } diff --git a/src/libsystemd-bus/bus-kernel.h b/src/libsystemd-bus/bus-kernel.h index ed3f987cc..1651c1e41 100644 --- a/src/libsystemd-bus/bus-kernel.h +++ b/src/libsystemd-bus/bus-kernel.h @@ -24,7 +24,14 @@ #include "sd-bus.h" #define MEMFD_CACHE_MAX 32 -#define MEMFD_CACHE_ITEM_SIZE_MAX (128*1024) + +/* When we cache a memfd block for reuse, we will truncate blocks + * longer than this in order not to keep too much data around. */ +#define MEMFD_CACHE_ITEM_SIZE_MAX (32*1024) + +/* This determines at which minimum size we prefer sending memfds over + * sending vectors */ +#define MEMFD_MIN_SIZE (32*1024) struct memfd_cache { int fd; diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c index 209fd71c1..ceac15601 100644 --- a/src/libsystemd-bus/bus-message.c +++ b/src/libsystemd-bus/bus-message.c @@ -59,6 +59,8 @@ static void message_free_part(sd_bus_message *m, struct bus_body_part *part) { assert(part); if (part->memfd >= 0) { + /* If we can reuse the memfd, try that. For that it + * can't be sealed yet. */ if (!part->sealed) bus_kernel_push_memfd(m->bus, part->memfd, part->data, part->mapped); @@ -3239,7 +3241,7 @@ int sd_bus_message_read_array(sd_bus_message *m, char type, const void **ptr, si return align; r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, CHAR_TO_STR(type)); - if (r < 0) + if (r <= 0) return r; c = message_get_container(m); @@ -3730,21 +3732,27 @@ int bus_message_seal(sd_bus_message *m, uint64_t serial) { return r; } - /* Add padding at the end, since we know the body - * needs to start at an 8 byte alignment. */ - + /* Add padding at the end of the fields part, since we know + * the body needs to start at an 8 byte alignment. We made + * sure we allocated enough space for this, so all we need to + * do here is to zero it out. */ l = BUS_MESSAGE_FIELDS_SIZE(m); a = ALIGN8(l) - l; if (a > 0) memset((uint8_t*) BUS_MESSAGE_FIELDS(m) + l, 0, a); - MESSAGE_FOREACH_PART(part, i, m) - if (part->memfd >= 0 && !part->sealed) { - bus_body_part_unmap(part); + /* If this is something we can send as memfd, then let's seal + the memfd now. Note that we can send memfds as payload only + for directed messages, and not for broadcasts. */ + if (m->destination) { + MESSAGE_FOREACH_PART(part, i, m) + if (part->memfd >= 0 && !part->sealed && part->size > MEMFD_MIN_SIZE) { + bus_body_part_unmap(part); - if (ioctl(part->memfd, KDBUS_CMD_MEMFD_SEAL_SET, 1) >= 0) - part->sealed = true; - } + if (ioctl(part->memfd, KDBUS_CMD_MEMFD_SEAL_SET, 1) >= 0) + part->sealed = true; + } + } m->header->serial = serial; m->sealed = true; diff --git a/src/libsystemd-bus/bus-socket.c b/src/libsystemd-bus/bus-socket.c index 4635da4b9..befded707 100644 --- a/src/libsystemd-bus/bus-socket.c +++ b/src/libsystemd-bus/bus-socket.c @@ -88,23 +88,33 @@ static int bus_message_setup_iovec(sd_bus_message *m) { m->iovec = m->iovec_fixed; else { m->iovec = new(struct iovec, n); - if (!m->iovec) - return -ENOMEM; + if (!m->iovec) { + r = -ENOMEM; + goto fail; + } } r = append_iovec(m, m->header, BUS_MESSAGE_BODY_BEGIN(m)); if (r < 0) - return r; + goto fail; MESSAGE_FOREACH_PART(part, i, m) { + r = bus_body_part_map(part); + if (r < 0) + goto fail; + r = append_iovec(m, part->data, part->size); if (r < 0) - return r; + goto fail; } assert(n == m->n_iovec); return 0; + +fail: + m->poisoned = true; + return r; } bool bus_socket_auth_needs_write(sd_bus *b) { diff --git a/src/libsystemd-bus/test-bus-zero-copy.c b/src/libsystemd-bus/test-bus-zero-copy.c index 5a9f45489..63bb92145 100644 --- a/src/libsystemd-bus/test-bus-zero-copy.c +++ b/src/libsystemd-bus/test-bus-zero-copy.c @@ -31,14 +31,19 @@ #include "bus-error.h" #include "bus-kernel.h" +#define FIRST_ARRAY 17 +#define SECOND_ARRAY 33 + int main(int argc, char *argv[]) { _cleanup_free_ char *bus_name = NULL, *address = NULL; - void *p; + uint8_t *p; sd_bus *a, *b; int r, bus_ref; sd_bus_message *m; sd_memfd *f; uint64_t sz; + uint32_t u32; + size_t i, l; log_set_max_level(LOG_DEBUG); @@ -75,20 +80,20 @@ int main(int argc, char *argv[]) { r = sd_bus_message_open_container(m, 'r', "ayay"); assert_se(r >= 0); - r = sd_bus_message_append_array_space(m, 'y', 32, &p); + r = sd_bus_message_append_array_space(m, 'y', FIRST_ARRAY, (void**) &p); assert_se(r >= 0); - memset(p, 'L', 32); + memset(p, 'L', FIRST_ARRAY); - r = sd_memfd_new_and_map(&f, 17, &p); + r = sd_memfd_new_and_map(&f, SECOND_ARRAY, (void**) &p); assert_se(r >= 0); - memset(p, 'P', 17); - munmap(p, 17); + memset(p, 'P', SECOND_ARRAY); + munmap(p, SECOND_ARRAY); r = sd_memfd_get_size(f, &sz); assert_se(r >= 0); - assert_se(sz == 17); + assert_se(sz == SECOND_ARRAY); r = sd_bus_message_append_array_memfd(m, 'y', f); assert_se(r >= 0); @@ -115,6 +120,32 @@ int main(int argc, char *argv[]) { assert_se(r > 0); bus_message_dump(m); + sd_bus_message_rewind(m, true); + + r = sd_bus_message_enter_container(m, 'r', "ayay"); + assert_se(r > 0); + + r = sd_bus_message_read_array(m, 'y', (const void**) &p, &l); + assert_se(r > 0); + assert_se(l == FIRST_ARRAY); + + for (i = 0; i < l; i++) + assert_se(p[i] == 'L'); + + r = sd_bus_message_read_array(m, 'y', (const void**) &p, &l); + assert_se(r > 0); + assert_se(l == SECOND_ARRAY); + + for (i = 0; i < l; i++) + assert_se(p[i] == 'P'); + + r = sd_bus_message_exit_container(m); + assert_se(r > 0); + + r = sd_bus_message_read(m, "u", &u32); + assert_se(r > 0); + assert_se(u32 == 4711); + sd_bus_message_unref(m); sd_bus_unref(a);