Re: [PATCH] module: Don't wait for GOING modules

[Petr Mladek <pmladek@suse.com>]

On Sun 2022-11-27 11:21:45, David Laight wrote:
> From: Petr Pavlu
> > Sent: 26 November 2022 14:43
> > 
> > On 11/23/22 16:29, Petr Mladek wrote:
> > > On Wed 2022-11-23 14:12:26, Petr Pavlu wrote:
> > >> During a system boot, it can happen that the kernel receives a burst of
> > >> requests to insert the same module but loading it eventually fails
> > >> during its init call. For instance, udev can make a request to insert
> > >> a frequency module for each individual CPU when another frequency module
> > >> is already loaded which causes the init function of the new module to
> > >> return an error.
> > >>
> > >> Since commit 6e6de3dee51a ("kernel/module.c: Only return -EEXIST for
> > >> modules that have finished loading"), the kernel waits for modules in
> > >> MODULE_STATE_GOING state to finish unloading before making another
> > >> attempt to load the same module.
> > >>
> > >> This creates unnecessary work in the described scenario and delays the
> > >> boot. In the worst case, it can prevent udev from loading drivers for
> > >> other devices and might cause timeouts of services waiting on them and
> > >> subsequently a failed boot.
> > >>
> > >> This patch attempts a different solution for the problem 6e6de3dee51a
> > >> was trying to solve. Rather than waiting for the unloading to complete,
> > >> it returns a different error code (-EBUSY) for modules in the GOING
> > >> state. This should avoid the error situation that was described in
> > >> 6e6de3dee51a (user space attempting to load a dependent module because
> > >> the -EEXIST error code would suggest to user space that the first module
> > >> had been loaded successfully), while avoiding the delay situation too.
> > >>
> 
> While people have all this code cached in their brains
> there is related problem I can easily hit.
> 
> If two processes create sctp sockets at the same time and sctp
> module has to be loaded then the second process can enter the
> module code before is it fully initialised.
> This might be because the try_module_get() succeeds before the
> module initialisation function returns.

Right, the race is there. And it is true that nobody should use
the module until mod->init() succeeds.

Well, I am not sure if there is an easy solution. It might require
reviewing what all try_module_get() callers expect.

We could not easily wait. For example, __sock_create() calls
try_module_get() under rcu_read_lock().

And various callers might want special handing when the module
is coming, going, and when it is not there at all.

I guess that it would require adding some new API and update
the various callers.

> I've avoided the issue by ensuring the socket creates are serialised.

I see. It would be great to have a clean solution, definitely.

Sigh, there are more issues with the module life time. For example,
kobjects might call the release() callback asynchronously and
it might happen when the module/code has gone, see
https://lore.kernel.org/all/20211105063710.4092936-1-ming.lei@redhat.com/

Best Regards,
PEtr