Re: [RFC 03/10] kmod: add dynamic max concurrent thread count

From: Luis R. Rodriguez
Date: Fri Dec 16 2016 - 03:40:11 EST


On Wed, Dec 14, 2016 at 04:38:27PM +0100, Petr Mladek wrote:
> On Thu 2016-12-08 11:48:14, Luis R. Rodriguez wrote:
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 271692a352f1..da2c25746937 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -2111,6 +2111,29 @@ config TRIM_UNUSED_KSYMS
> >
> > If unsure, or if you need to build out-of-tree modules, say N.
> >
> > +config MAX_KMOD_CONCURRENT
> > + int "Max allowed concurrent request_module() calls (6=>64, 10=>1024)"
> > + range 0 14
>
> Would not too small range break loading module dependencies?

No, dependencies are resolved by depmod, so userspace looks at the list and
just finit_module() the depenencies, skipping kmod. So the limit is
really only for kernel acting like a boss.

> I am not sure how it is implemented but it might require having
> some more module loads in progress.

Dependencies should be OK, a more serious concern with dependencies is
the aggregate memory it takes to load all dep modules for one required
module since finit_module() ends up allocating the struct module to copy
over data from userspace.

> I would give 6 as minimum. Nobody has troubles with the current limit.

Fair enough! Although disabling modprobe calls all together seemed like
a fun test, that should we allow that via the module parameter at least?

> > + default 6 if !BASE_SMALL
> > + default 7 if BASE_SMALL
>
> Aren't the conditions inversed?

Whoops yes, sorry.

> > +void __init init_kmod_umh(void)
> > +{
> > + if (!max_modprobes)
> > + max_modprobes = min(max_threads/2,
> > + 2 << CONFIG_MAX_KMOD_CONCURRENT);
>
> This should be
>
> 1 << CONFIG_MAX_KMOD_CONCURRENT);
>
> 1 << 1 = 2;
>
> Note that this calculation is mentioned also some comments and
> documentation.

Heh sorry, yes fixed! Good thing I had still tested all along with the
value I intended though :P

Luis