| Message ID | 20230829123808.325202-1-andrea.righi@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | libkmod: allow to fallback to user-space decompression | expand |
On Tue, Aug 29, 2023 at 02:38:08PM +0200, Andrea Righi wrote: >Fallback to user-space decompression when the kernel cannot allocate >enough memory to perform the decompression. > >This can happen with large modules, such as xfs on linux 6.5 for >example, an ENOMEM would be returned and the module fails to load. > >It seems more reliable to try again with user-space decompression >rather than reporting an error and failing to load the module. > >Fixes: 09c9f8c ("libkmod: Use kernel decompression when available") >Signed-off-by: Andrea Righi <andrea.righi@canonical.com> >--- > libkmod/libkmod-module.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c >index 585da41..d2d4815 100644 >--- a/libkmod/libkmod-module.c >+++ b/libkmod/libkmod-module.c >@@ -978,7 +978,7 @@ KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod, > } > > err = do_finit_module(mod, flags, args); >- if (err == -ENOSYS) >+ if (err == -ENOSYS || err == -ENOMEM) oh... ENOMEM can be returned in several places. I don't think it's a great interface to just retry in userspace. Luis, can we do better on the kernel side? Andrea, did you track the exact location triggering the ENOMEM? Is it the return of kvmalloc_array() or alloc_page() from module_get_next_page()? Or one of the previous kmalloc we use for the different deflate methods? thanks Lucas De Marchi > err = do_init_module(mod, flags, args); > > if (err < 0) >-- >2.40.1 >
On Tue, Aug 29, 2023 at 07:47:05AM -0700, Lucas De Marchi wrote: > On Tue, Aug 29, 2023 at 02:38:08PM +0200, Andrea Righi wrote: > > Fallback to user-space decompression when the kernel cannot allocate > > enough memory to perform the decompression. > > > > This can happen with large modules, such as xfs on linux 6.5 for > > example, an ENOMEM would be returned and the module fails to load. > > > > It seems more reliable to try again with user-space decompression > > rather than reporting an error and failing to load the module. > > > > Fixes: 09c9f8c ("libkmod: Use kernel decompression when available") > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > --- > > libkmod/libkmod-module.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c > > index 585da41..d2d4815 100644 > > --- a/libkmod/libkmod-module.c > > +++ b/libkmod/libkmod-module.c > > @@ -978,7 +978,7 @@ KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod, > > } > > > > err = do_finit_module(mod, flags, args); > > - if (err == -ENOSYS) > > + if (err == -ENOSYS || err == -ENOMEM) > > oh... ENOMEM can be returned in several places. I don't think it's a great > interface to just retry in userspace. > > Luis, can we do better on the kernel side? > > Andrea, did you track the exact location triggering the ENOMEM? Is it > the return of kvmalloc_array() or alloc_page() from > module_get_next_page()? Or one of the previous kmalloc we use for the > different deflate methods? It was this kmalloc() right here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/decompress.c?h=v6.5#n244 For the records, I've also sent this patch to address the issue from a kernel perspective: https://lore.kernel.org/lkml/20230829120508.317611-1-andrea.righi@canonical.com/T/#u That seems to solve the issue in my particular case, but I'm not sure if that's a valid solution across all architectures. Thanks, -Andrea
On Tue, Aug 29, 2023 at 05:23:25PM +0200, Andrea Righi wrote: > For the records, I've also sent this patch to address the issue from a > kernel perspective: > https://lore.kernel.org/lkml/20230829120508.317611-1-andrea.righi@canonical.com/T/#u > > That seems to solve the issue in my particular case, but I'm not sure if > that's a valid solution across all architectures. I can't think of a reason why it would regress on other architectures. We'll give it a spin soon fast as it will be on its way to Linus today. Luis
On Tue, Aug 29, 2023 at 05:23:25PM +0200, Andrea Righi wrote: >On Tue, Aug 29, 2023 at 07:47:05AM -0700, Lucas De Marchi wrote: >> On Tue, Aug 29, 2023 at 02:38:08PM +0200, Andrea Righi wrote: >> > Fallback to user-space decompression when the kernel cannot allocate >> > enough memory to perform the decompression. >> > >> > This can happen with large modules, such as xfs on linux 6.5 for >> > example, an ENOMEM would be returned and the module fails to load. >> > >> > It seems more reliable to try again with user-space decompression >> > rather than reporting an error and failing to load the module. >> > >> > Fixes: 09c9f8c ("libkmod: Use kernel decompression when available") >> > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> >> > --- >> > libkmod/libkmod-module.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c >> > index 585da41..d2d4815 100644 >> > --- a/libkmod/libkmod-module.c >> > +++ b/libkmod/libkmod-module.c >> > @@ -978,7 +978,7 @@ KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod, >> > } >> > >> > err = do_finit_module(mod, flags, args); >> > - if (err == -ENOSYS) >> > + if (err == -ENOSYS || err == -ENOMEM) >> >> oh... ENOMEM can be returned in several places. I don't think it's a great >> interface to just retry in userspace. >> >> Luis, can we do better on the kernel side? >> >> Andrea, did you track the exact location triggering the ENOMEM? Is it >> the return of kvmalloc_array() or alloc_page() from >> module_get_next_page()? Or one of the previous kmalloc we use for the >> different deflate methods? > >It was this kmalloc() right here: >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/decompress.c?h=v6.5#n244 > >For the records, I've also sent this patch to address the issue from a >kernel perspective: >https://lore.kernel.org/lkml/20230829120508.317611-1-andrea.righi@canonical.com/T/#u That patch seems better. We will probably have to move other decompress algorithms to vmalloc too. thanks Lucas De Marchi > >That seems to solve the issue in my particular case, but I'm not sure if >that's a valid solution across all architectures. > >Thanks, >-Andrea
On Tue, Aug 29, 2023 at 09:42:43AM -0700, Luis Chamberlain wrote: > On Tue, Aug 29, 2023 at 05:23:25PM +0200, Andrea Righi wrote: > > For the records, I've also sent this patch to address the issue from a > > kernel perspective: > > https://lore.kernel.org/lkml/20230829120508.317611-1-andrea.righi@canonical.com/T/#u > > > > That seems to solve the issue in my particular case, but I'm not sure if > > that's a valid solution across all architectures. > > I can't think of a reason why it would regress on other architectures. > We'll give it a spin soon fast as it will be on its way to Linus today. Cool! Yes, if this patch can be applied we don't really need to change libkmod and if there are other spots in the decompress code that can use vmalloc() we can adjust them in a similar way. Thanks, -Andrea
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index 585da41..d2d4815 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -978,7 +978,7 @@ KMOD_EXPORT int kmod_module_insert_module(struct kmod_module *mod, } err = do_finit_module(mod, flags, args); - if (err == -ENOSYS) + if (err == -ENOSYS || err == -ENOMEM) err = do_init_module(mod, flags, args); if (err < 0)
Fallback to user-space decompression when the kernel cannot allocate enough memory to perform the decompression. This can happen with large modules, such as xfs on linux 6.5 for example, an ENOMEM would be returned and the module fails to load. It seems more reliable to try again with user-space decompression rather than reporting an error and failing to load the module. Fixes: 09c9f8c ("libkmod: Use kernel decompression when available") Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- libkmod/libkmod-module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)