On Thu, Jun 15, 2023 at 10:36:31AM +0100, Emil Velikov wrote:
Greetings Lucas, list,
I've pulled the email off lore.kernel.org manually (haven't played
with lei yet), so chances are the following will be "slightly"
malformed.
One easy easy if you want to pick single patch series is to use b4
b4 mbox 20230601224001.23397-1-lucas.de.marchi@xxxxxxxxx
neomut -f ./20230601224001.23397-1-lucas.de.marchi@xxxxxxxxxxxxx
or instead of neomutt, import the mbox to your favorite email client.
Above all - hell yeah, thank you for wiring this neat functionality.
Out of curiosity: have you done any measurements - CPU cycles, memory
or other - how well the kernel decompression performs vs the userspace
one?
no, I didn't. I think one important aspect is when paired with the
patch to the kernel that deduplicates the module loading. With
compressed modules, that would be irrelevant as libkmod would go the
init_module() path rather than finit_module().
A reasonable test would be to create a VM with a bunch of CPUs (so we
get many parallel requests or the same modules) and then measure
a) 6.4-rcX
b) 6.4-rcX + that kernel patch
c) 6.4-rcX + that kernel patch + this patch series
That said, I may have spotted a small bug, namely:
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -864,15 +864,24 @@ extern long init_module(const void *mem, unsigned long len, const char *args);
static int do_finit_module(struct kmod_module *mod, unsigned int flags,
const char *args)
{
+ enum kmod_file_compression_type compression, kernel_compression;
unsigned int kernel_flags = 0;
int err;
/*
- * Re-use ENOSYS, returned when there is no such syscall, so the
- * fallback to init_module applies
+ * When module is not compressed or its compression type matches the
+ * one in use by the kernel, there is no need to read the file
+ * in userspace. Otherwise, re-use ENOSYS to trigger the same fallback
+ * as when finit_module() is not supported.
*/
- if (!kmod_file_get_direct(mod->file))
- return -ENOSYS;
+ compression = kmod_file_get_compression(mod->file);
+ kernel_compression = kmod_get_kernel_compression(mod->ctx);
+ if (!(compression == KMOD_FILE_COMPRESSION_NONE ||
+ compression == kernel_compression))
+ return ENOSYS;
+
Old code returns negative -ENOSYS (negative), the new one a positive
ENOSYS. Where the fallback, mentioned in the comment just above,
triggers on the former negative ENOSYS.
Mind you I'm still sipping coffee, so chances are I'm missing something here.
no, apparently I didn't have enough coffee when I wrote this.
I'll fix this up on next version.
thanks
Lucas De Marchi
Thanks again and HTH o/
Emil