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

["Luis R. Rodriguez" <mcgrof@kernel.org>]

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