[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZHXEktFq7NPYLtGn@hovoldconsulting.com>
Date: Tue, 30 May 2023 11:40:34 +0200
From: Johan Hovold <johan@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Luis Chamberlain <mcgrof@...nel.org>,
Lucas De Marchi <lucas.demarchi@...el.com>,
Petr Pavlu <petr.pavlu@...e.com>, gregkh@...uxfoundation.org,
rafael@...nel.org, song@...nel.org, lucas.de.marchi@...il.com,
christophe.leroy@...roup.eu, peterz@...radead.org, rppt@...nel.org,
dave@...olabs.net, willy@...radead.org, vbabka@...e.cz,
mhocko@...e.com, dave.hansen@...ux.intel.com,
colin.i.king@...il.com, jim.cromie@...il.com,
catalin.marinas@....com, jbaron@...mai.com,
rick.p.edgecombe@...el.com, yujie.liu@...el.com, david@...hat.com,
tglx@...utronix.de, hch@....de, patches@...ts.linux.dev,
linux-modules@...r.kernel.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, pmladek@...e.com, prarit@...hat.com,
lennart@...ttering.net
Subject: Re: [PATCH 2/2] module: add support to avoid duplicates early on load
On Mon, May 29, 2023 at 09:55:15PM -0400, Linus Torvalds wrote:
> On Mon, May 29, 2023 at 11:18 AM Johan Hovold <johan@...nel.org> wrote:
> >
> > I took a closer look at some of the modules that failed to load and
> > noticed a pattern in that they have dependencies that are needed by more
> > than one device.
>
> Ok, this is a "maybe something like this" RFC series of two patches -
> one trivial one to re-organize things a bit so that we can then do the
> real one which uses a filter based on the inode pointer to return an
> "idempotent return value" for module loads that share the same inode.
>
> It's entirely untested, and since I'm on the road I'm going to not
> really be able to test it. It compiles for me, and the code looks
> fairly straightforward, but it's probably buggy.
>
> It's very loosely based on Luis' attempt, but it
> (a) is internal to module loading
> (b) uses a reliable cookie
> (c) doesn't leave the cookie around randomly for later
> (d) has seen absolutely no testing
>
> Put another way: if somebody wants to play with this, please treat it
> as a starting point, not the final thing. You might need to debug
> things, and fix silly mistakes.
With the missing spinlock initialisation fixed:
-static struct spinlock idem_lock;
+static DEFINE_SPINLOCK(idem_lock);
this passes basic smoke testing and allows the X13s to boot.
It does not seem to have any significant impact on boot time, but it
avoids some of the unnecessary load attempts as intended:
Before:
Mods ever loaded 131
Mods failed on kread 0
Mods failed on decompress 0
Mods failed on becoming 24
Mods failed on load 14
Total module size 12587008
Total mod text size 5058560
Failed kread bytes 0
Failed decompress bytes 0
Failed becoming bytes 2437992
Failed kmod bytes 1858992
Virtual mem wasted bytes 4296984
Average mod size 96085
Average mod text size 38615
Avg fail becoming bytes 101583
Average fail load bytes 132786
After:
Mods ever loaded 131
Mods failed on kread 0
Mods failed on decompress 0
Mods failed on becoming 4
Mods failed on load 0
Total module size 12587008
Total mod text size 5058560
Failed kread bytes 0
Failed decompress bytes 0
Failed becoming bytes 109776
Failed kmod bytes 0
Virtual mem wasted bytes 109776
Average mod size 96085
Average mod text size 38615
Avg fail becoming bytes 27444
Johan
Powered by blists - more mailing lists