From: Emil Velikov <emil.l.velikov@gmail.com>
To: Lucas De Marchi <lucas.de.marchi@gmail.com>
Cc: linux-modules@vger.kernel.org
Subject: Re: [PATCH 5/5] libkmod: Use kernel decompression when available
Date: Thu, 15 Jun 2023 10:36:31 +0100 [thread overview]
Message-ID: <CACvgo53yHOd964PoQkM_oX3LGcr+qCagz64T+sxcP-eAveS85g@mail.gmail.com> (raw)
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.
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?
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.
Thanks again and HTH o/
Emil
next reply other threads:[~2023-06-15 9:38 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-15 9:36 Emil Velikov [this message]
2023-06-15 10:09 ` [PATCH 5/5] libkmod: Use kernel decompression when available Emil Velikov
2023-06-27 14:05 ` Lucas De Marchi
2023-06-16 5:12 ` Lucas De Marchi
-- strict thread matches above, loose matches on Subject: below --
2023-06-01 22:39 [PATCH 0/5] libkmod: Use kernel decompression support Lucas De Marchi
2023-06-01 22:40 ` [PATCH 5/5] libkmod: Use kernel decompression when available Lucas De Marchi
2023-06-06 18:38 ` Luis Chamberlain
2023-06-06 19:01 ` Lucas De Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACvgo53yHOd964PoQkM_oX3LGcr+qCagz64T+sxcP-eAveS85g@mail.gmail.com \
--to=emil.l.velikov@gmail.com \
--cc=linux-modules@vger.kernel.org \
--cc=lucas.de.marchi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).