Messages in this thread | | | From | Song Liu <> | Subject | Re: [PATCH v5 bpf-next 1/5] module: introduce module_alloc_huge | Date | Wed, 6 Jul 2022 04:39:13 +0000 |
| |
> On Jul 1, 2022, at 4:20 PM, Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Fri, Jun 24, 2022 at 02:57:08PM -0700, Song Liu wrote: >> Introduce module_alloc_huge, which allocates huge page backed memory in >> module memory space. The primary user of this memory is bpf_prog_pack >> (multiple BPF programs sharing a huge page). >> >> Signed-off-by: Song Liu <song@kernel.org> > > I see mm not Cc'd. I'd like review from them.
I will CC mm in the next version (or resend). Thanks for the reminder.
> >> --- >> arch/x86/kernel/module.c | 21 +++++++++++++++++++++ >> include/linux/moduleloader.h | 5 +++++ >> kernel/module/main.c | 8 ++++++++ >> 3 files changed, 34 insertions(+) >> >> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c >> index b98ffcf4d250..63f6a16c70dc 100644 >> --- a/arch/x86/kernel/module.c >> +++ b/arch/x86/kernel/module.c >> @@ -86,6 +86,27 @@ void *module_alloc(unsigned long size) >> return p; >> } >> >> +void *module_alloc_huge(unsigned long size) >> +{ >> + gfp_t gfp_mask = GFP_KERNEL; >> + void *p; >> + >> + if (PAGE_ALIGN(size) > MODULES_LEN) >> + return NULL; >> + >> + p = __vmalloc_node_range(size, MODULE_ALIGN, >> + MODULES_VADDR + get_module_load_offset(), >> + MODULES_END, gfp_mask, PAGE_KERNEL, >> + VM_DEFER_KMEMLEAK | VM_ALLOW_HUGE_VMAP, >> + NUMA_NO_NODE, __builtin_return_address(0)); >> + if (p && (kasan_alloc_module_shadow(p, size, gfp_mask) < 0)) { >> + vfree(p); >> + return NULL; >> + } >> + >> + return p; >> +} > > 1) When things like kernel/bpf/core.c start using a module alloc it > is time to consider genearlizing this.
I am not quite sure what the generalization would look like. IMHO, the ideal case would have: a) A kernel_text_rw_allocator, similar to current module_alloc. b) A kernel_text_ro_allocator, similar to current bpf_prog_pack_alloc. This is built on top of kernel_text_rw_allocator. Different allocations could share a page, thus it requires text_poke like support from the arch. c) If the arch supports text_poke, kprobes, ftrace trampolines, and bpf trampolines should use kernel_text_ro_allocator. d) Major archs should support CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC, and they should use kernel_text_ro_allocator for module text.
Does this sound reasonable to you?
I tried to enable CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC for x86_64, but that doesn't really work. Do we have plan to make this combination work?
> > 2) How we free is important, and each arch does something funky for > this. This is not addressed here.
How should we address this? IIUC, x86_64 just calls vfree.
> > And yes I welcome generalizing generic module_alloc() too as suggested > before. The concern on my part is the sloppiness this enables.
One question I have is, does module_alloc (or kernel_text_*_allocator above) belong to module code, or mm code (maybe vmalloc)?
I am planning to let BPF trampoline use bpf_prog_pack on x86_64, which is another baby step of c) above.
Thanks, Song
| |