Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count

[Luis R. Rodriguez]

On Fri, May 19, 2017 at 02:58:29PM -0700, Dmitry Torokhov wrote:
> On Fri, May 19, 2017 at 02:45:29PM -0700, Luis R. Rodriguez wrote:
> > On May 19, 2017 1:45 PM, "Dmitry Torokhov" <dmitry.torokhov@xxxxxxxxx>
> > wrote:
> > 
> > On Thu, May 18, 2017 at 08:24:39PM -0700, Luis R. Rodriguez wrote:
> > > We currently statically limit the number of modprobe threads which
> > > we allow to run concurrently to 50. As per Keith Owens, this was a
> > > completely arbitrary value, and it was set in the 2.3.38 days [0]
> > > over 16 years ago in year 2000.
> > >
> > > Although we haven't yet hit our lower limits, experimentation [1]
> > > shows that when and if we hit this limit in the worst case, will be
> > > fatal -- consider get_fs_type() failures upon mount on a system which
> > > has many partitions, some of which might even be with the same
> > > filesystem. Its best to be prudent and increase and set this
> > > value to something more sensible which ensures we're far from hitting
> > > the limit and also allows default build/user run time override.
> > >
> > > The worst case is fatal given that once a module fails to load there
> > > is a period of time during which subsequent request for the same module
> > > will fail, so in the case of partitions its not just one request that
> > > could fail, but whole series of partitions. This later issue of a
> > > module request failure domino effect can be addressed later, but
> > > increasing the limit to something more meaninful should at least give us
> > > enough cushion to avoid this for a while.
> > >
> > > Set this value up with a bit more meaninful modern limits:
> > >
> > > Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> > > Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
> > >
> > > Also allow the default max limit to be further fine tuned at compile
> > > time and at initialization at run time at boot up using the kernel
> > > parameter: max_modprobes.
> > >
> > > [0] https://git.kernel.org/cgit/linux/kernel/git/history/
> > history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
> > > [1] https://github.com/mcgrof/test_request_module
> > 
> > If we actually run into this issue, instead of slamming the system with
> > bazillion concurrent requests, can we wait for the other modprobes to
> > finish and then continue?
> > 
> > 
> > Yes ! That I have a patch that does precisely that ! That is actually still
> > *not enough* to not fail fatally but this would be subject of another
> > series with more debatable approaches.
> > 
> 
> Then please post it.

Will do.

> > This at least pushes us to closer safer limits for now while also making it
> > configurable.
> 
> Making it configurable depending on how big/little box is makes no
> sense,

If we set a hard limit then we need to patch a system if we need to increment
it. This is rather stupid given we have no current heuristics to make kmod
loading deterministic from userspace, and in the worst case this can be fatal.
General system size is a good first guess, but making it configurable is
really key given current limitations. I'll post further patches which reveals
some of these issues more clearly.

> especially if the above is implemented, as depth of modprobe
> invocations depends on configuration and not computing power of the
> hardware the system is running on.

You seem to agree making it configurable is sensible , but not depending on
the system size ?

  Luis