On Thu, Feb 03, 2022 at 07:05:13AM +0000, Christophe Leroy wrote: > > > Le 03/02/2022 à 01:01, Luis Chamberlain a écrit : > > On Sat, Jan 29, 2022 at 05:02:09PM +0000, Christophe Leroy wrote: > >> diff --git a/kernel/module.c b/kernel/module.c > >> index 11f51e17fb9f..f3758115ebaa 100644 > >> --- a/kernel/module.c > >> +++ b/kernel/module.c > >> @@ -81,7 +81,9 @@ > >> /* If this is set, the section belongs in the init part of the module */ > >> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1)) > >> > >> +#ifndef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC > >> #define data_layout core_layout > >> +#endif > >> > >> /* > >> * Mutex protects: > >> @@ -111,6 +113,12 @@ static struct mod_tree_root { > >> #define module_addr_min mod_tree.addr_min > >> #define module_addr_max mod_tree.addr_max > >> > >> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC > >> +static struct mod_tree_root mod_data_tree __cacheline_aligned = { > >> + .addr_min = -1UL, > >> +}; > >> +#endif > >> + > >> #ifdef CONFIG_MODULES_TREE_LOOKUP > >> > >> /* > >> @@ -186,6 +194,11 @@ static void mod_tree_insert(struct module *mod) > >> __mod_tree_insert(&mod->core_layout.mtn, &mod_tree); > >> if (mod->init_layout.size) > >> __mod_tree_insert(&mod->init_layout.mtn, &mod_tree); > >> + > >> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC > >> + mod->data_layout.mtn.mod = mod; > >> + __mod_tree_insert(&mod->data_layout.mtn, &mod_data_tree); > >> +#endif > > > > > > kernel/ directory has quite a few files, module.c is the second to > > largest file, and it has tons of stuff. Aaron is doing work to > > split things out to make code easier to read and so that its easier > > to review changes. See: > > > > https://lkml.kernel.org/r/20220130213214.1042497-1-atom...@redhat.com > > > > I think this is a good patch example which could benefit from that work. > > So I'd much prefer to see that work go in first than this, so to see if > > we can make the below changes more compartamentalized. > > > > Curious, how much testing has been put into this series? > > > I tested the change up to (including) patch 4 to verify it doesn't > introduce regression when not using > CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC,
> Then I tested with patch 5. I first tried with the 'hello world' test > module. After that I loaded several important modules and checked I > didn't get any regression, both with and without STRICT_MODULES_RWX and > I checked the consistency in /proc/vmallocinfo > /proc/modules /sys/class/modules/* I wonder if we have a test for STRICT_MODULES_RWX. > I also tested with a hacked module_alloc() to force branch trampolines. So to verify that reducing these trampolines actually helps on an architecture? I wonder if we can generalize this somehow to let archs verify such strategies can help. I was hoping for a bit more wider testing, like actually users, etc. It does not seem like so. So we can get to that by merging this soon into modules-next and having this bleed out issues with linux-next. We are in good time to do this now. The kmod tree has tons of tests: https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/ Can you use that to verify there are no regressions? Aaron, Michal, if you can do the same that'd be appreciated. Luis