From 1eff942e37adda55bd8c506d4e2a7970fdbf7a74 Mon Sep 17 00:00:00 2001 From: Lucas De Marchi Date: Thu, 18 Oct 2012 01:36:33 -0300 Subject: [PATCH] libkmod: cache open file for later access MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If we are accessing several times the modules and reading some sections by sucessive calls to the functions below, we are incurring in a penalty of having to open, parse the header and close the file. For each function. - kmod_module_get_info() - kmod_module_get_versions() - kmod_module_get_symbols() - kmod_module_get_dependency_symbols() These functions are particularly important to depmod. It calls all of them, for each module. Moreover there's a huge bottleneck in the open operation if we are using compression. Every time we open the module we need to uncompress the file and after getting the information we need we discard the result. This is clearly shown by profiling depmod with perf (record + report), using compressed modules: 64.07% depmod libz.so.1.2.7 [.] 0x00000000000074b8 ◆ 18.18% depmod libz.so.1.2.7 [.] crc32 ▒ 2.42% depmod libz.so.1.2.7 [.] inflate ▒ 1.17% depmod libc-2.16.so [.] __memcpy_ssse3_back ▒ 0.96% depmod [kernel.kallsyms] [k] copy_user_generic_string ▒ 0.89% depmod libc-2.16.so [.] __strcmp_sse42 ▒ 0.82% depmod [kernel.kallsyms] [k] hrtimer_interrupt ▒ 0.77% depmod libc-2.16.so [.] _int_malloc ▒ 0.44% depmod kmod-nolib [.] kmod_elf_get_strings ▒ 0.41% depmod kmod-nolib [.] kmod_elf_get_dependency_symbols ▒ 0.37% depmod kmod-nolib [.] kmod_elf_get_section ▒ 0.36% depmod kmod-nolib [.] kmod_elf_get_symbols ... Average of running depmod 5 times, dropping caches between them, in a slow spinning disk: Before: 12.25 +- 0.20 After: 8.20 +- 0.21 m-i-t: 9.62 +- 0.27 So this patch leads to an improvement of ~33% over unpatched version, ending up with 15% speedup over module-init-tools. --- libkmod/libkmod-file.c | 13 ++++ libkmod/libkmod-module.c | 152 ++++++++++++---------------------------------- libkmod/libkmod-private.h | 1 + 3 files changed, 54 insertions(+), 112 deletions(-) diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c index 8beb7e3..679edef 100644 --- a/libkmod/libkmod-file.c +++ b/libkmod/libkmod-file.c @@ -56,6 +56,7 @@ struct kmod_file { void *memory; const struct file_ops *ops; const struct kmod_ctx *ctx; + struct kmod_elf *elf; }; #ifdef ENABLE_XZ @@ -268,6 +269,15 @@ static const struct file_ops reg_ops = { load_reg, unload_reg }; +struct kmod_elf *kmod_file_get_elf(struct kmod_file *file) +{ + if (file->elf) + return file->elf; + + file->elf = kmod_elf_new(file->memory, file->size); + return file->elf; +} + struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx, const char *filename) { @@ -345,6 +355,9 @@ off_t kmod_file_get_size(const struct kmod_file *file) void kmod_file_unref(struct kmod_file *file) { + if (file->elf) + kmod_elf_unref(file->elf); + file->ops->unload(file); if (file->fd >= 0) close(file->fd); diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index 9756d57..0d87ce1 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -60,6 +60,7 @@ struct kmod_module { const char *install_commands; /* owned by kmod_config */ const char *remove_commands; /* owned by kmod_config */ char *alias; /* only set if this module was created from an alias */ + struct kmod_file *file; int n_dep; int refcount; struct { @@ -436,6 +437,10 @@ KMOD_EXPORT struct kmod_module *kmod_module_unref(struct kmod_module *mod) kmod_pool_del_module(mod->ctx, mod, mod->hashkey); kmod_module_unref_list(mod->dep); + + if (mod->file) + kmod_file_unref(mod->file); + kmod_unref(mod->ctx); free(mod->options); free(mod->path); @@ -2043,6 +2048,25 @@ KMOD_EXPORT void kmod_module_section_free_list(struct kmod_list *list) } } +static struct kmod_elf *kmod_module_get_elf(const struct kmod_module *mod) +{ + if (mod->file == NULL) { + const char *path = kmod_module_get_path(mod); + + if (path == NULL) { + errno = ENOENT; + return NULL; + } + + ((struct kmod_module *)mod)->file = kmod_file_open(mod->ctx, + path); + if (mod->file == NULL) + return NULL; + } + + return kmod_file_get_elf(mod->file); +} + struct kmod_module_info { char *key; char value[]; @@ -2088,12 +2112,8 @@ static void kmod_module_info_free(struct kmod_module_info *info) */ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_list **list) { - struct kmod_file *file; struct kmod_elf *elf; - const char *path; - const void *mem; char **strings; - size_t size; int i, count, ret = 0; if (mod == NULL || list == NULL) @@ -2101,28 +2121,13 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ assert(*list == NULL); - path = kmod_module_get_path(mod); - if (path == NULL) - return -ENOENT; - - file = kmod_file_open(mod->ctx, path); - if (file == NULL) + elf = kmod_module_get_elf(mod); + if (elf == NULL) return -errno; - size = kmod_file_get_size(file); - mem = kmod_file_get_contents(file); - - elf = kmod_elf_new(mem, size); - if (elf == NULL) { - ret = -errno; - goto elf_open_error; - } - count = kmod_elf_get_strings(elf, ".modinfo", &strings); - if (count < 0) { - ret = count; - goto get_strings_error; - } + if (count < 0) + return count; for (i = 0; i < count; i++) { struct kmod_module_info *info; @@ -2164,11 +2169,6 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ list_error: free(strings); -get_strings_error: - kmod_elf_unref(elf); -elf_open_error: - kmod_file_unref(file); - return ret; } @@ -2266,12 +2266,8 @@ static void kmod_module_version_free(struct kmod_module_version *version) */ KMOD_EXPORT int kmod_module_get_versions(const struct kmod_module *mod, struct kmod_list **list) { - struct kmod_file *file; struct kmod_elf *elf; - const char *path; - const void *mem; struct kmod_modversion *versions; - size_t size; int i, count, ret = 0; if (mod == NULL || list == NULL) @@ -2279,28 +2275,13 @@ KMOD_EXPORT int kmod_module_get_versions(const struct kmod_module *mod, struct k assert(*list == NULL); - path = kmod_module_get_path(mod); - if (path == NULL) - return -ENOENT; - - file = kmod_file_open(mod->ctx, path); - if (file == NULL) + elf = kmod_module_get_elf(mod); + if (elf == NULL) return -errno; - size = kmod_file_get_size(file); - mem = kmod_file_get_contents(file); - - elf = kmod_elf_new(mem, size); - if (elf == NULL) { - ret = -errno; - goto elf_open_error; - } - count = kmod_elf_get_modversions(elf, &versions); - if (count < 0) { - ret = count; - goto get_strings_error; - } + if (count < 0) + return count; for (i = 0; i < count; i++) { struct kmod_module_version *mv; @@ -2329,11 +2310,6 @@ KMOD_EXPORT int kmod_module_get_versions(const struct kmod_module *mod, struct k list_error: free(versions); -get_strings_error: - kmod_elf_unref(elf); -elf_open_error: - kmod_file_unref(file); - return ret; } @@ -2431,12 +2407,8 @@ static void kmod_module_symbol_free(struct kmod_module_symbol *symbol) */ KMOD_EXPORT int kmod_module_get_symbols(const struct kmod_module *mod, struct kmod_list **list) { - struct kmod_file *file; struct kmod_elf *elf; - const char *path; - const void *mem; struct kmod_modversion *symbols; - size_t size; int i, count, ret = 0; if (mod == NULL || list == NULL) @@ -2444,28 +2416,13 @@ KMOD_EXPORT int kmod_module_get_symbols(const struct kmod_module *mod, struct km assert(*list == NULL); - path = kmod_module_get_path(mod); - if (path == NULL) - return -ENOENT; - - file = kmod_file_open(mod->ctx, path); - if (file == NULL) + elf = kmod_module_get_elf(mod); + if (elf == NULL) return -errno; - size = kmod_file_get_size(file); - mem = kmod_file_get_contents(file); - - elf = kmod_elf_new(mem, size); - if (elf == NULL) { - ret = -errno; - goto elf_open_error; - } - count = kmod_elf_get_symbols(elf, &symbols); - if (count < 0) { - ret = count; - goto get_strings_error; - } + if (count < 0) + return count; for (i = 0; i < count; i++) { struct kmod_module_symbol *mv; @@ -2494,11 +2451,6 @@ KMOD_EXPORT int kmod_module_get_symbols(const struct kmod_module *mod, struct km list_error: free(symbols); -get_strings_error: - kmod_elf_unref(elf); -elf_open_error: - kmod_file_unref(file); - return ret; } @@ -2599,12 +2551,8 @@ static void kmod_module_dependency_symbol_free(struct kmod_module_dependency_sym */ KMOD_EXPORT int kmod_module_get_dependency_symbols(const struct kmod_module *mod, struct kmod_list **list) { - struct kmod_file *file; struct kmod_elf *elf; - const char *path; - const void *mem; struct kmod_modversion *symbols; - size_t size; int i, count, ret = 0; if (mod == NULL || list == NULL) @@ -2612,28 +2560,13 @@ KMOD_EXPORT int kmod_module_get_dependency_symbols(const struct kmod_module *mod assert(*list == NULL); - path = kmod_module_get_path(mod); - if (path == NULL) - return -ENOENT; - - file = kmod_file_open(mod->ctx, path); - if (file == NULL) + elf = kmod_module_get_elf(mod); + if (elf == NULL) return -errno; - size = kmod_file_get_size(file); - mem = kmod_file_get_contents(file); - - elf = kmod_elf_new(mem, size); - if (elf == NULL) { - ret = -errno; - goto elf_open_error; - } - count = kmod_elf_get_dependency_symbols(elf, &symbols); - if (count < 0) { - ret = count; - goto get_strings_error; - } + if (count < 0) + return count; for (i = 0; i < count; i++) { struct kmod_module_dependency_symbol *mv; @@ -2664,11 +2597,6 @@ KMOD_EXPORT int kmod_module_get_dependency_symbols(const struct kmod_module *mod list_error: free(symbols); -get_strings_error: - kmod_elf_unref(elf); -elf_open_error: - kmod_file_unref(file); - return ret; } diff --git a/libkmod/libkmod-private.h b/libkmod/libkmod-private.h index 9061854..4760733 100644 --- a/libkmod/libkmod-private.h +++ b/libkmod/libkmod-private.h @@ -139,6 +139,7 @@ void kmod_module_set_builtin(struct kmod_module *mod, bool builtin) __attribute_ /* libkmod-file.c */ struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx, const char *filename) _must_check_ __attribute__((nonnull(1,2))); +struct kmod_elf *kmod_file_get_elf(struct kmod_file *file) __attribute__((nonnull(1))); void *kmod_file_get_contents(const struct kmod_file *file) _must_check_ __attribute__((nonnull(1))); off_t kmod_file_get_size(const struct kmod_file *file) _must_check_ __attribute__((nonnull(1))); void kmod_file_unref(struct kmod_file *file) __attribute__((nonnull(1))); -- 2.7.4