On 10/14/22 09:54, David Hildenbrand wrote:

On Mon, Sep 19, 2022 at 2:33 PM Petr Pavlu [off-list ref] wrote:

quoted

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.

The module loader currently serializes all such requests, with the
barrier in add_unformed_module(). This creates a lot of unnecessary work
and delays the boot.

This patch improves the behavior as follows:
* A check whether a module load matches an already loaded module is
  moved right after a module name is determined. -EEXIST continues to be
  returned if the module exists and is live, -EBUSY is returned if
  a same-name module is going.
* A new reference-counted shared_load_info structure is introduced to
  keep track of duplicate load requests. Two loads are considered
  equivalent if their module name matches. In case a load duplicates
  another running insert, the code waits for its completion and then
  returns -EEXIST or -EBUSY depending on whether it succeeded.

Note that prior to 6e6de3dee51a ("kernel/module.c: Only return -EEXIST
for modules that have finished loading"), the kernel already did merge
some of same load requests but it was more by accident and relied on
specific timing. The patch brings this behavior back in a more explicit
form.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---

Hi Petr,

as you might have seen I sent a patch/fix yesterday (not being aware
of this patch and that
this is also a performance issue, which is interesting), that
similarly makes sure that modules
are unique early.

https://lkml.kernel.org/r/20221013180518.217405-1-david@redhat.com

It doesn't perform the -EBUSY changes or use something like
shared_load_info/refcounts;
it simply uses a second list while the module cannot be placed onto
the module list yet.

Not sure if that part is really required (e.g., for performance
reasons). Like Luis, I feel like
some of these parts could be split into separate patches, if the other
parts are really required.

The shared_load_info/refcounts/-EBUSY logic is actually an important part
which addresses the regression mentioned in the commit message and which I'm
primarily trying to fix.

I just tested your patch in the environment where I can reproduce the
vmap allocation issue, and
(unsurprisingly) this patch similarly seems to fix the issue.

So if your patch ends up upstream, it would be good to add some details
of my patch description (vmap allocation issue) to this patch description.

Thanks for testing this patch. I will add a note about the vmap allocation
issue to the patch description.

Petr