From cc413494678e3049e67cc5c8d79126de30c36d7a Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sat, 16 Nov 2024 12:49:41 +0100 Subject: [PATCH] libkmod: Improve index dump performance Use FILE for output to reduce the amount of system calls. Removes the only use case of write_str_safe, which can be removed as well. Signed-off-by: Tobias Stoeckmann --- libkmod/libkmod-index.c | 32 ++++++++++++++++---------------- libkmod/libkmod-index.h | 4 ++-- libkmod/libkmod.c | 33 ++++++++++++++++++++++++++------- shared/util.c | 26 -------------------------- shared/util.h | 1 - testsuite/test-util.c | 29 ----------------------------- 6 files changed, 44 insertions(+), 81 deletions(-) diff --git a/libkmod/libkmod-index.c b/libkmod/libkmod-index.c index 7909c774..22e14c6e 100644 --- a/libkmod/libkmod-index.c +++ b/libkmod/libkmod-index.c @@ -360,7 +360,7 @@ static struct index_node_f *index_readchild(const struct index_node_f *parent, i return NULL; } -static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int fd) +static void index_dump_node(struct index_node_f *node, struct strbuf *buf, FILE *fp) { struct index_value *v; size_t pushed; @@ -369,10 +369,10 @@ static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int f pushed = strbuf_pushchars(buf, node->prefix); for (v = node->values; v != NULL; v = v->next) { - write_str_safe(fd, buf->bytes, buf->used); - write_str_safe(fd, " ", 1); - write_str_safe(fd, v->value, strlen(v->value)); - write_str_safe(fd, "\n", 1); + fwrite(buf->bytes, 1, buf->used, fp); + fputc(' ', fp); + fputs(v->value, fp); + fputc('\n', fp); } for (ch = node->first; ch <= node->last; ch++) { @@ -382,7 +382,7 @@ static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int f continue; if (strbuf_pushchar(buf, ch)) { - index_dump_node(child, buf, fd); + index_dump_node(child, buf, fp); strbuf_popchar(buf); } } @@ -391,7 +391,7 @@ static void index_dump_node(struct index_node_f *node, struct strbuf *buf, int f index_close(node); } -void index_dump(struct index_file *in, int fd, bool alias_prefix) +void index_dump(struct index_file *in, FILE *fp, bool alias_prefix) { struct index_node_f *root; struct strbuf buf; @@ -402,7 +402,7 @@ void index_dump(struct index_file *in, int fd, bool alias_prefix) strbuf_init(&buf); if (!alias_prefix || strbuf_pushchars(&buf, "alias ")) - index_dump_node(root, &buf, fd); + index_dump_node(root, &buf, fp); strbuf_release(&buf); } @@ -821,7 +821,7 @@ static struct index_mm_node *index_mm_readchild(const struct index_mm_node *pare return NULL; } -static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, int fd) +static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, FILE *fp) { const void *p; size_t i, pushed; @@ -833,10 +833,10 @@ static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, i struct index_mm_value v; read_value_mm(&p, &v); - write_str_safe(fd, buf->bytes, buf->used); - write_str_safe(fd, " ", 1); - write_str_safe(fd, v.value, v.len); - write_str_safe(fd, "\n", 1); + fwrite(buf->bytes, 1, buf->used, fp); + fputc(' ', fp); + fwrite(v.value, 1, v.len, fp); + fputc('\n', fp); } for (ch = node->first; ch <= node->last; ch++) { @@ -847,7 +847,7 @@ static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, i continue; if (strbuf_pushchar(buf, ch)) { - index_mm_dump_node(child, buf, fd); + index_mm_dump_node(child, buf, fp); strbuf_popchar(buf); } } @@ -855,7 +855,7 @@ static void index_mm_dump_node(struct index_mm_node *node, struct strbuf *buf, i strbuf_popchars(buf, pushed); } -void index_mm_dump(const struct index_mm *idx, int fd, bool alias_prefix) +void index_mm_dump(const struct index_mm *idx, FILE *fp, bool alias_prefix) { struct index_mm_node nbuf, *root; struct strbuf buf; @@ -866,7 +866,7 @@ void index_mm_dump(const struct index_mm *idx, int fd, bool alias_prefix) strbuf_init(&buf); if (!alias_prefix || strbuf_pushchars(&buf, "alias ")) - index_mm_dump_node(root, &buf, fd); + index_mm_dump_node(root, &buf, fp); strbuf_release(&buf); } diff --git a/libkmod/libkmod-index.h b/libkmod/libkmod-index.h index 47c50d38..9a98a8cf 100644 --- a/libkmod/libkmod-index.h +++ b/libkmod/libkmod-index.h @@ -19,7 +19,7 @@ struct index_file; struct index_file *index_file_open(const char *filename); void index_file_close(struct index_file *idx); char *index_search(struct index_file *idx, const char *key); -void index_dump(struct index_file *in, int fd, bool alias_prefix); +void index_dump(struct index_file *in, FILE *fp, bool alias_prefix); struct index_value *index_searchwild(struct index_file *idx, const char *key); void index_values_free(struct index_value *values); @@ -31,4 +31,4 @@ int index_mm_open(const struct kmod_ctx *ctx, const char *filename, void index_mm_close(struct index_mm *index); char *index_mm_search(const struct index_mm *idx, const char *key); struct index_value *index_mm_searchwild(const struct index_mm *idx, const char *key); -void index_mm_dump(const struct index_mm *idx, int fd, bool alias_prefix); +void index_mm_dump(const struct index_mm *idx, FILE *fp, bool alias_prefix); diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c index f9d63a94..f7cd8f23 100644 --- a/libkmod/libkmod.c +++ b/libkmod/libkmod.c @@ -810,22 +810,37 @@ KMOD_EXPORT void kmod_unload_resources(struct kmod_ctx *ctx) KMOD_EXPORT int kmod_dump_index(struct kmod_ctx *ctx, enum kmod_index type, int fd) { + int err = 0, fd2; + FILE *fp; + if (ctx == NULL) return -ENOSYS; + fd2 = dup(fd); + if (fd2 == -1) + return -errno; + fp = fdopen(fd2, "ae"); + if (fp == NULL) { + err = -errno; + close(fd2); + return err; + } + #if defined(__clang__) #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wtautological-unsigned-enum-zero-compare" #endif - if (type < 0 || type >= _KMOD_INDEX_MODULES_SIZE) - return -ENOENT; + if (type < 0 || type >= _KMOD_INDEX_MODULES_SIZE) { + err = -ENOENT; + goto done; + } #if defined(__clang__) #pragma clang diagnostic pop #endif if (ctx->indexes[type] != NULL) { DBG(ctx, "use mmapped index '%s'\n", index_files[type].fn); - index_mm_dump(ctx->indexes[type], fd, index_files[type].alias_prefix); + index_mm_dump(ctx->indexes[type], fp, index_files[type].alias_prefix); } else { char fn[PATH_MAX]; struct index_file *idx; @@ -835,14 +850,18 @@ KMOD_EXPORT int kmod_dump_index(struct kmod_ctx *ctx, enum kmod_index type, int DBG(ctx, "file=%s\n", fn); idx = index_file_open(fn); - if (idx == NULL) - return -ENOSYS; + if (idx == NULL) { + err = -ENOSYS; + goto done; + } - index_dump(idx, fd, index_files[type].alias_prefix); + index_dump(idx, fp, index_files[type].alias_prefix); index_file_close(idx); } - return 0; +done: + fclose(fp); + return err; } const struct kmod_config *kmod_get_config(const struct kmod_ctx *ctx) diff --git a/shared/util.c b/shared/util.c index 7a1f3ee8..f688c6ae 100644 --- a/shared/util.c +++ b/shared/util.c @@ -237,32 +237,6 @@ ssize_t read_str_safe(int fd, char *buf, size_t buflen) return done; } -ssize_t write_str_safe(int fd, const char *buf, size_t buflen) -{ - size_t todo = buflen; - size_t done = 0; - - assert_cc(EAGAIN == EWOULDBLOCK); - - do { - ssize_t r = write(fd, buf + done, todo); - - if (r == 0) - break; - else if (r > 0) { - todo -= r; - done += r; - } else { - if (errno == EAGAIN || errno == EINTR) - continue; - else - return -errno; - } - } while (todo > 0); - - return done; -} - int read_str_long(int fd, long *value, int base) { char buf[32], *end; diff --git a/shared/util.h b/shared/util.h index 65d90dba..7d329016 100644 --- a/shared/util.h +++ b/shared/util.h @@ -37,7 +37,6 @@ _nonnull_all_ bool path_ends_with_kmod_ext(const char *path, size_t len); _must_check_ _nonnull_(2) ssize_t pread_str_safe(int fd, char *buf, size_t buflen, off_t off); _must_check_ _nonnull_(2) ssize_t read_str_safe(int fd, char *buf, size_t buflen); -_nonnull_(2) ssize_t write_str_safe(int fd, const char *buf, size_t buflen); _must_check_ _nonnull_(2) int read_str_long(int fd, long *value, int base); _must_check_ _nonnull_(2) int read_str_ulong(int fd, unsigned long *value, int base); _nonnull_(1) char *freadline_wrapped(FILE *fp, unsigned int *linenum); diff --git a/testsuite/test-util.c b/testsuite/test-util.c index e38b9f83..2cd99251 100644 --- a/testsuite/test-util.c +++ b/testsuite/test-util.c @@ -162,35 +162,6 @@ static int test_path_ends_with_kmod_ext(const struct test *t) DEFINE_TEST(test_path_ends_with_kmod_ext, .description = "check implementation of path_ends_with_kmod_ext()") -#define TEST_WRITE_STR_SAFE_FILE "/write-str-safe" -#define TEST_WRITE_STR_SAFE_PATH TESTSUITE_ROOTFS "test-util2/" TEST_WRITE_STR_SAFE_FILE -static int test_write_str_safe(const struct test *t) -{ - const char *s = "test"; - int fd; - - fd = open(TEST_WRITE_STR_SAFE_FILE ".txt", O_CREAT | O_TRUNC | O_WRONLY, 0644); - assert_return(fd >= 0, EXIT_FAILURE); - - write_str_safe(fd, s, strlen(s)); - close(fd); - - return EXIT_SUCCESS; -} -DEFINE_TEST(test_write_str_safe, - .description = "check implementation of write_str_safe()", - .config = { - [TC_ROOTFS] = TESTSUITE_ROOTFS "test-util2/", - }, - .need_spawn = true, - .output = { - .files = (const struct keyval[]) { - { TEST_WRITE_STR_SAFE_PATH ".txt", - TEST_WRITE_STR_SAFE_PATH "-correct.txt" }, - { }, - }, - }); - static int test_uadd32_overflow(const struct test *t) { uint32_t res;