[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y01Qlp0E+3FJVVWW@alley>
Date: Mon, 17 Oct 2022 14:54:46 +0200
From: Petr Mladek <pmladek@...e.com>
To: Petr Pavlu <petr.pavlu@...e.com>
Cc: mcgrof@...nel.org, david@...hat.com, linux-modules@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] module: Merge same-name module load requests
On Sun 2022-10-16 14:30:30, 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.
>
> 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. It can prevent udev from loading drivers for other
> devices and might cause timeouts of services waiting on them and
> subsequently a failed boot.
>
> The mentioned serialization was introduced as a side-effect of commit
> 6e6de3dee51a. The kernel before that merged some of same load requests
> although it was more by accident and relied on specific timing. The
> patch brings this behavior back in a more explicit form.
>
> The logic is improved 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.
>
> Moving the check for same-name module loads earlier has also a positive
> effect on reducing memory pressure. For instance, David Hildenbrand and
> Lin Liu reported [1] that when KASAN_INLINE is enabled (resulting in
> large module size), with plenty of devices that udev wants to probe and
> with plenty of CPUs that can carry out that probing concurrently, the
> system can actually run out of module vmap space and trigger vmap
> allocation errors. This is fixed by the patch too as it avoids duplicate
> layout_and_allocate() work.
>
> [1] https://lore.kernel.org/all/20221013180518.217405-1-david@redhat.com/
>
> Fixes: 6e6de3dee51a ("kernel/module.c: Only return -EEXIST for modules that have finished loading")
> Reviewed-by: Petr Mladek <pmladek@...e.com>
> Signed-off-by: Petr Pavlu <petr.pavlu@...e.com>
All these tags should be in the order of (logical) appearance.
Somethings like:
Suggested-by: if any
Signed-off-by: author of the patch
Acked-by: in order of appearance
Revived-by: in order of appearance
Tested-by: in order of appearance
Signed-off-by: committer (upstream maintainer)
I know that I provided Reviewed-by before you did the final changes.
Well, you could use my tag only when you did only cosmetic changes or
changes that I requested. You expect that I would agree with the patch
even after it was updated.
Best Regards,
Petr
Powered by blists - more mailing lists