[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPhsuW6AR9CorYPL88-92_VBn-7uZoXj4SfWXn5N_bRC_SXSKQ@mail.gmail.com>
Date: Wed, 18 Jan 2023 09:52:29 -0800
From: Song Liu <song@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Christophe Leroy <christophe.leroy@...roup.eu>,
"linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"songliubraving@...com" <songliubraving@...com>,
Luis Chamberlain <mcgrof@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH/RFC] module: replace module_layout with module_memory
On Wed, Jan 18, 2023 at 7:07 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Tue, Jan 10, 2023 at 06:31:41AM +0000, Christophe Leroy wrote:
> > Le 09/01/2023 ą 21:51, Song Liu a écrit :
>
> > > Do you mean one tree will cause addr_[min|max] to be inaccurate?
> > >
> >
> > Yes at least. On powerpc you will have module text below kernel,
> > somewhere between 0xb0000000 and 0xcfffffff, and you will have module
> > data in vmalloc area, somewhere between 0xf0000000 and 0xffffffff.
> >
> > If you have only one tree, any address between 0xc0000000 and 0xefffffff
> > will trigger a tree search.
>
> The current min/max thing is tied to the tree because of easy update on
> remove, but module-insert/remove is not a performance critical path.
>
> So I think it should be possible to have {min,max}[TYPES] pairs. Either
> brute force the removal -- using a linear scan of the mod->list to find
> the new bounds on removal.
I think keeping an array of min/max pairs is an overkill.
w/o CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC, all the
types will be allocated in the same range (MODULES_VADDR, MODULES_END),
so one min/max pair should be enough.
w/ CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC, there
is a big gap between text allocation and data allocation. I think a second
min/max pair will be useful here.
>
> Or overengineer the whole thing and use an augmented tree to keep that
> many heaps in sync during the update -- but this seems total overkill.
>
> The only consideration is testing that many ranges in
> __module_address(), this is already 2 cachelines worth of range-checks
> -- which seems a little excessive.
Currently, min/max are updated on module load, but not on module unload.
I guess we won't really need __module_address() to be that fast.
If there are no objections or suggestions. I will update the patches with a
second min/max pair with CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC.
Thanks,
Song
>
> (also, I note that module_addr_{min,max} are unused these days)
Powered by blists - more mailing lists