libkmod: cache open file for later access

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.

diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
index 8beb7e34021b644b463acb43717bb86cb8f57897..679edef3f2e2e3c0febd80b8b80b2ab294cbe43a 100644 (file)
--- 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 9756d57d82db45081a52242042ca2661e586809d..0d87ce1f9b9f76ee7b548985185b6b1889c4b7be 100644 (file)
--- 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 90618540a9b5c842730117aaad8b9b763c6f9331..476073303e390dc96af4c5d4e762df0909cc2614 100644 (file)
--- 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)));