Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650631 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 18:30 +0200
    Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-25 18:40 +0200
      Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 19:00 +0200
        Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-25 19:40 +0200
          Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 19:40 +0200
            Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 20:10 +0200
              Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-25 20:30 +0200
                Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 21:10 +0200
                  Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 23:40 +0200

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

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@gmail.com>
> > 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

[toc] | [next] | [standalone]

#1650640

On Thu, May 25, 2017 at 06:22:01PM +0200, Luis R. Rodriguez wrote:
> 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@gmail.com>
> > > 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 ?

No, I am saying that making it configurable based on system size makes
no sense at all, and making it configurable given you already have
patches removing hard failures gives no benefit.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1650649

On Thu, May 25, 2017 at 9:38 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, May 25, 2017 at 06:22:01PM +0200, Luis R. Rodriguez wrote:
>> 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@gmail.com>
>> > > 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 ?
>
> No, I am saying that making it configurable based on system size makes
> no sense at all, and making it configurable given you already have
> patches removing hard failures gives no benefit.

Ah no, the problem is that hard failures are not yet removed in this
patch set at all! This series only contains the things I thought were
non-radical really.

In fact even with the subsequent patches from my 2nd series I'll
eventually post post -- these fatal issues are not cured at all unless
we dance with userspace a bit, or unless as you suggest we have *all*
pending theads wait without killing any.

*This* small patch should enable folks to move the needle to a more
*fair* limit, its also useful to backport a simple fix even if the
other stuff is not merged, *but* it *also* provides a way for systems
to move away from the slippery slope if they know what they are doing.

I'll post the 2nd series next.

 Luis

[toc] | [prev] | [next] | [standalone]

#1650693

On Thu, May 25, 2017 at 09:50:15AM -0700, Luis R. Rodriguez wrote:
> On Thu, May 25, 2017 at 9:38 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Thu, May 25, 2017 at 06:22:01PM +0200, Luis R. Rodriguez wrote:
> >> 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@gmail.com>
> >> > > 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 ?
> >
> > No, I am saying that making it configurable based on system size makes
> > no sense at all, and making it configurable given you already have
> > patches removing hard failures gives no benefit.
> 
> Ah no, the problem is that hard failures are not yet removed in this
> patch set at all! This series only contains the things I thought were
> non-radical really.

I know they are not removed in this patch set.

> 
> In fact even with the subsequent patches from my 2nd series I'll
> eventually post post -- these fatal issues are not cured at all unless
> we dance with userspace a bit, or unless as you suggest we have *all*
> pending theads wait without killing any.

Well, that is too bad, I understood you already implemented what I
suggested.

> 
> *This* small patch should enable folks to move the needle to a more
> *fair* limit, its also useful to backport a simple fix even if the
> other stuff is not merged, *but* it *also* provides a way for systems
> to move away from the slippery slope if they know what they are doing.

Look, you are trying to push a band-aid solution for a problem that is
purely theoretical (as you say in your patch description we are not
hitting this problem in practice, only your test does). There is
no slippery slope for systems to move away, no need to backport
anything. We seem to agree that a better solution is possible (throttle
number of concurrently running modprobes without killing requesters),
and with that solution the band-aid will no longer be needed.

So please implement and post the proper fix for the issue.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1650694

On Thu, May 25, 2017 at 10:30 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, May 25, 2017 at 09:50:15AM -0700, Luis R. Rodriguez wrote:
>> On Thu, May 25, 2017 at 9:38 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Thu, May 25, 2017 at 06:22:01PM +0200, Luis R. Rodriguez wrote:
>> >> 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@gmail.com>
>> >> > > 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 ?
>> >
>> > No, I am saying that making it configurable based on system size makes
>> > no sense at all, and making it configurable given you already have
>> > patches removing hard failures gives no benefit.
>>
>> Ah no, the problem is that hard failures are not yet removed in this
>> patch set at all! This series only contains the things I thought were
>> non-radical really.
>
> I know they are not removed in this patch set.
>
>>
>> In fact even with the subsequent patches from my 2nd series I'll
>> eventually post post -- these fatal issues are not cured at all unless
>> we dance with userspace a bit, or unless as you suggest we have *all*
>> pending theads wait without killing any.
>
> Well, that is too bad, I understood you already implemented what I
> suggested.

We seem to want the same so that is good actually.

>> *This* small patch should enable folks to move the needle to a more
>> *fair* limit, its also useful to backport a simple fix even if the
>> other stuff is not merged, *but* it *also* provides a way for systems
>> to move away from the slippery slope if they know what they are doing.
>
> Look, you are trying to push a band-aid solution for a problem that is
> purely theoretical (as you say in your patch description we are not
> hitting this problem in practice, only your test does).

I have a stress test driver which reveals we can easily hit it. In
practice the only way to know if we hit the limit is a:
"request_module: runaway loop modprobe %s" message on dmesg, however
its fatal, how often people inspect a kernel log to see if that came
up though... not sure. So a module could not be loaded and we may not
realize it.

> There is
> no slippery slope for systems to move away, no need to backport
> anything. We seem to agree that a better solution is possible (throttle
> number of concurrently running modprobes without killing requesters),
> and with that solution the band-aid will no longer be needed.
>
> So please implement and post the proper fix for the issue.

Alright, will do away with this patch and just go for the jugular of the issue.

 Luis

[toc] | [prev] | [next] | [standalone]

#1650712

On Thu, May 25, 2017 at 10:38:40AM -0700, Luis R. Rodriguez wrote:
> On Thu, May 25, 2017 at 10:30 AM, Dmitry Torokhov
> > There is
> > no slippery slope for systems to move away, no need to backport
> > anything. We seem to agree that a better solution is possible (throttle
> > number of concurrently running modprobes without killing requesters),
> > and with that solution the band-aid will no longer be needed.
> >
> > So please implement and post the proper fix for the issue.
> 
> Alright, will do away with this patch and just go for the jugular of the issue.

I gave this some more thought, even if we go with the throttling right away in
practice you'll end up with a dmesg notice of a throttle kicking in once you *do*
reach this. We are forcing only 50 concurrent threads and making this a static
limit with no good reason than 2.3.38 days evaluation from 16 years ago (2000).
If we throttle we are going to throttle with a 2.3.38 days limit. And you
advocate that ?

  Luis

[toc] | [prev] | [next] | [standalone]

#1650720

On Thu, May 25, 2017 at 08:06:03PM +0200, Luis R. Rodriguez wrote:
> On Thu, May 25, 2017 at 10:38:40AM -0700, Luis R. Rodriguez wrote:
> > On Thu, May 25, 2017 at 10:30 AM, Dmitry Torokhov
> > > There is
> > > no slippery slope for systems to move away, no need to backport
> > > anything. We seem to agree that a better solution is possible (throttle
> > > number of concurrently running modprobes without killing requesters),
> > > and with that solution the band-aid will no longer be needed.
> > >
> > > So please implement and post the proper fix for the issue.
> > 
> > Alright, will do away with this patch and just go for the jugular of the issue.
> 
> I gave this some more thought, even if we go with the throttling right away in
> practice you'll end up with a dmesg notice of a throttle kicking in once you *do*

So remove it. The warning was meaningful when we rejected requests, now
it is not.

> reach this. We are forcing only 50 concurrent threads and making this a static
> limit with no good reason than 2.3.38 days evaluation from 16 years ago (2000).
> If we throttle we are going to throttle with a 2.3.38 days limit. And you
> advocate that ?

Yes. Can you give me reason why slamming the system with more than 50
modprobes is a good idea in 4.12 days? Does the increased limit
decreases boot time? By how much?

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1650758

On Thu, May 25, 2017 at 11:26 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, May 25, 2017 at 08:06:03PM +0200, Luis R. Rodriguez wrote:
>> On Thu, May 25, 2017 at 10:38:40AM -0700, Luis R. Rodriguez wrote:
>> > On Thu, May 25, 2017 at 10:30 AM, Dmitry Torokhov
>> > > There is
>> > > no slippery slope for systems to move away, no need to backport
>> > > anything. We seem to agree that a better solution is possible (throttle
>> > > number of concurrently running modprobes without killing requesters),
>> > > and with that solution the band-aid will no longer be needed.
>> > >
>> > > So please implement and post the proper fix for the issue.
>> >
>> > Alright, will do away with this patch and just go for the jugular of the issue.
>>
>> I gave this some more thought, even if we go with the throttling right away in
>> practice you'll end up with a dmesg notice of a throttle kicking in once you *do*
>
> So remove it. The warning was meaningful when we rejected requests, now
> it is not.

Great.

>> reach this. We are forcing only 50 concurrent threads and making this a static
>> limit with no good reason than 2.3.38 days evaluation from 16 years ago (2000).
>> If we throttle we are going to throttle with a 2.3.38 days limit. And you
>> advocate that ?
>
> Yes. Can you give me reason why slamming the system with more than 50
> modprobes is a good idea in 4.12 days? Does the increased limit
> decreases boot time? By how much?

If in practice we are not hitting the limit the point is moot, and
when we do I agree we can re-evaluate. With my stress test driver on a
test case we can push as hard as bringing out the OOM killer even if
we throttle, fun.

  Luis

[toc] | [prev] | [next] | [standalone]

#1650838

On Thu, May 25, 2017 at 12:01 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Thu, May 25, 2017 at 11:26 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Thu, May 25, 2017 at 08:06:03PM +0200, Luis R. Rodriguez wrote:
>>> On Thu, May 25, 2017 at 10:38:40AM -0700, Luis R. Rodriguez wrote:
>>> > On Thu, May 25, 2017 at 10:30 AM, Dmitry Torokhov
>>> > > There is
>>> > > no slippery slope for systems to move away, no need to backport
>>> > > anything. We seem to agree that a better solution is possible (throttle
>>> > > number of concurrently running modprobes without killing requesters),
>>> > > and with that solution the band-aid will no longer be needed.
>>> > >
>>> > > So please implement and post the proper fix for the issue.
>>> >
>>> > Alright, will do away with this patch and just go for the jugular of the issue.
>>>
>>> I gave this some more thought, even if we go with the throttling right away in
>>> practice you'll end up with a dmesg notice of a throttle kicking in once you *do*
>>
>> So remove it. The warning was meaningful when we rejected requests, now
>> it is not.
>
> Great.
>
>>> reach this. We are forcing only 50 concurrent threads and making this a static
>>> limit with no good reason than 2.3.38 days evaluation from 16 years ago (2000).
>>> If we throttle we are going to throttle with a 2.3.38 days limit. And you
>>> advocate that ?
>>
>> Yes. Can you give me reason why slamming the system with more than 50
>> modprobes is a good idea in 4.12 days? Does the increased limit
>> decreases boot time? By how much?
>
> If in practice we are not hitting the limit the point is moot, and
> when we do I agree we can re-evaluate. With my stress test driver on a
> test case we can push as hard as bringing out the OOM killer even if
> we throttle, fun.

Alright, I don't see these OOMs anymore *after* I actually nuked that
patch which incremented the kmod limit. The reason we can OOM is
finit_module() can consume gobs of memory, the current value then
seems fair, and work well for my tests provided we do use the proper
throttle. Will respin.

  Luis

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web