Re: [PATCH v3 0/4] kmod: help make deterministic

[Jessica Yu]

+++ Petr Mladek [27/06/17 10:13 +0200]:
> On Tue 2017-06-27 02:27:44, Luis R. Rodriguez wrote:
> > On Tue, Jun 27, 2017 at 12:44:42AM +0200, Luis R. Rodriguez wrote:
> > > On Mon, Jun 26, 2017 at 11:37:36PM +0200, Jessica Yu wrote:
> > > > +++ Kees Cook [20/06/17 17:23 -0700]:
> > > > > On Tue, Jun 20, 2017 at 1:56 PM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> > > > > > On Fri, May 26, 2017 at 02:12:24PM -0700, Luis R. Rodriguez wrote:
> > > > > > > Luis R. Rodriguez (4):
> > > > > > >   module: use list_for_each_entry_rcu() on find_module_all()
> > > > > > >   kmod: reduce atomic operations on kmod_concurrent and simplify
> > > > > > >   kmod: add test driver to stress test the module loader
> > > > > > >   kmod: throttle kmod thread limit
> > > > > >
> > > > > > About a month now with no further nitpicks. What tree should these changes
> > > > > > go through if there are no issues? Andrew's, Jessica's ?
> > > > >
> > > > > Seems like going through Jessica's would make the most sense?
> > > >
> > > > Would be happy to take patches 01 (which I need to anyway), 02,
> > > > possibly 04 if decoupled from the test driver (03).
> > >
> > > Feel free to decouple it, but note that then the commit log must then be
> > > changed. My own take is this fix is not so critical as it is a corner case, so
> > > I have instead preferred to couple in the test case and respective fix
> > > together. I'll leave it up to you how to proceed.
> >
> > Note: Linus noted swait is actually very special use-case [0] so I'd hate to
> > add a new use case not vetted for. This use case on kmod.c really does *not*
> > require anything but a simple wait though, so really am inclined to let that
> > through unless I hear back...
> >
> > [0] https://lkml.kernel.org/r/20170627001534.GK21846@xxxxxxxxxxxxx
>
> Heh, I was not aware of this special case either. The welcoming
> comment of the swait API confused me as well.
>
> In this light, I suggest to switch the patch to using the normal wait API.

Huh, I wasn't aware either :-/ But I agree, judging from Linus'
response [0], it's probably best to use the well established wait_*
variants. I'm not sure I understood why the patch switched to swait,
but in any case I don't think we'd be hitting the "thundering herd"
problem very often here (and if we do, we could just use exclusive
wait. But in that scenario I'd be more interested in why a normal
system would be battered with more than 50 in-kernel modprobe requests
at a time).

[0] https://marc.info/?l=linux-kernel&m=149851347228696&w=2