From: Petr Pavlu <petr.pavlu@suse.com>
To: David Hildenbrand <david@redhat.com>
Cc: mcgrof@kernel.org, pmladek@suse.com,
linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] module: Merge same-name module load requests
Date: Sat, 15 Oct 2022 11:49:27 +0200 [thread overview]
Message-ID: <21491c35-a9b8-9161-311d-a01507dc296f@suse.com> (raw)
In-Reply-To: <CADFyXm5AP8pvXAKRBVNsZd5SUPziKBV0UktwORokuLU7c6Sbvg@mail.gmail.com>
On 10/14/22 09:54, David Hildenbrand wrote:
> On Mon, Sep 19, 2022 at 2:33 PM Petr Pavlu <petr.pavlu@suse.com> 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.
>>
>> 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
next prev parent reply other threads:[~2022-10-15 9:49 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-19 12:32 [PATCH v2 0/2] module: Merge same-name module load requests Petr Pavlu
2022-09-19 12:32 ` [PATCH v2 1/2] module: Correct wake up of module_wq Petr Pavlu
2022-09-30 20:22 ` Luis Chamberlain
2022-10-14 8:40 ` Petr Mladek
2022-09-19 12:32 ` [PATCH v2 2/2] module: Merge same-name module load requests Petr Pavlu
2022-09-30 20:30 ` Luis Chamberlain
2022-10-15 9:27 ` Petr Pavlu
2022-10-18 18:33 ` Luis Chamberlain
2022-10-18 19:19 ` Prarit Bhargava
2022-10-18 19:53 ` Prarit Bhargava
2022-10-20 7:19 ` Petr Mladek
2022-10-24 13:22 ` Prarit Bhargava
2022-10-24 17:08 ` Luis Chamberlain
2022-10-24 12:37 ` Petr Pavlu
2022-10-24 14:00 ` Prarit Bhargava
2022-11-13 16:44 ` Petr Pavlu
2022-10-19 12:00 ` Petr Pavlu
2022-10-20 7:03 ` Petr Mladek
2022-10-24 17:53 ` Luis Chamberlain
2022-11-12 1:47 ` Luis Chamberlain
2022-11-14 8:57 ` David Hildenbrand
2022-11-14 15:38 ` Luis Chamberlain
2022-11-14 15:45 ` David Hildenbrand
2022-11-15 19:29 ` Luis Chamberlain
2022-11-16 16:03 ` Prarit Bhargava
2022-11-21 16:00 ` Petr Pavlu
2022-11-21 19:03 ` Luis Chamberlain
2022-11-21 19:50 ` David Hildenbrand
2022-11-21 20:27 ` Luis Chamberlain
2022-11-22 13:59 ` Petr Pavlu
2022-11-22 17:58 ` Luis Chamberlain
2022-11-16 16:04 ` David Hildenbrand
2022-11-18 17:32 ` David Hildenbrand
2022-11-28 16:29 ` Prarit Bhargava
2022-11-29 13:13 ` Petr Pavlu
2022-12-02 16:36 ` Petr Mladek
2022-12-06 12:31 ` Prarit Bhargava
2022-12-07 13:23 ` Petr Pavlu
2022-12-04 19:58 ` Prarit Bhargava
2022-10-14 7:54 ` David Hildenbrand
2022-10-15 9:49 ` Petr Pavlu [this message]
2022-10-14 13:52 ` Petr Mladek
2022-10-16 12:25 ` Petr Pavlu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=21491c35-a9b8-9161-311d-a01507dc296f@suse.com \
--to=petr.pavlu@suse.com \
--cc=david@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-modules@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=pmladek@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).