Re: [PATCH 00/13] mm: jit/text allocator
On Mon, Jun 05, 2023 at 11:09:34AM +0100, Mark Rutland wrote: > On Mon, Jun 05, 2023 at 12:20:40PM +0300, Mike Rapoport wrote: > > On Fri, Jun 02, 2023 at 10:35:09AM +0100, Mark Rutland wrote: > > > On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > > > > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > > > > For a while I have wanted to give kprobes its own allocator so that > > > > > it can work > > > > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA > > > > > space in > > > > > the modules area. > > > > > > > > > > Given that, I think these should have their own allocator functions > > > > > that can be > > > > > provided independently, even if those happen to use common > > > > > infrastructure. > > > > > > > > How much memory can kprobes conceivably use? I think we also want to try > > > > to push back on combinatorial new allocators, if we can. > > > > > > That depends on who's using it, and how (e.g. via BPF). > > > > > > To be clear, I'm not necessarily asking for entirely different > > > allocators, but > > > I do thinkg that we want wrappers that can at least pass distinct > > > start+end > > > parameters to a common allocator, and for arm64's modules code I'd expect > > > that > > > we'd keep the range falblack logic out of the common allcoator, and just > > > call > > > it twice. > > > > > > > > > Several architectures override module_alloc() because of various > > > > > > constraints where the executable memory can be located and this > > > > > > causes > > > > > > additional obstacles for improvements of code allocation. > > > > > > > > > > > > This set splits code allocation from modules by introducing > > > > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces > > > > > > call > > > > > > sites of module_alloc() and module_memfree() with the new APIs and > > > > > > implements core text and related allocation in a central place. > > > > > > > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > > > > architectures that require non-default behaviour for text > > > > > > allocation must > > > > > > fill jit_alloc_params structure and implement > > > > > > jit_alloc_arch_params() that > > > > > > returns a pointer to that structure. If an architecture does not > > > > > > implement > > > > > > jit_alloc_arch_params(), the defaults compatible with the current > > > > > > modules::module_alloc() are used. > > > > > > > > > > As above, I suspect that each of the callsites should probably be > > > > > using common > > > > > infrastructure, but I don't think that a single > > > > > jit_alloc_arch_params() makes > > > > > sense, since the parameters for each case may need to be distinct. > > > > > > > > I don't see how that follows. The whole point of function parameters is > > > > that they may be different :) > > > > > > What I mean is that jit_alloc_arch_params() tries to aggregate common > > > parameters, but they aren't actually common (e.g. the actual start+end > > > range > > > for allocation). > > > > jit_alloc_arch_params() tries to aggregate architecture constraints and > > requirements for allocations of executable memory and this exactly what > > the first 6 patches of this set do. > > > > A while ago Thomas suggested to use a structure that parametrizes > > architecture constraints by the memory type used in modules [1] and Song > > implemented the infrastructure for it and x86 part [2]. > > > > I liked the idea of defining parameters in a single structure, but I > > thought that approaching the problem from the arch side rather than from > > modules perspective will be better starting point, hence these patches. > > > > I don't see a fundamental reason why a single structure cannot describe > > what is needed for different code allocation cases, be it modules, kprobes > > or bpf. There is of course an assumption that the core allocations will be > > the same for all the users, and it seems to me that something like > > > > * allocate physical memory if allocator caches are empty > > * map it in vmalloc or modules address space > > * return memory from the allocator cache to the caller > > > > will work for all usecases. > > > > We might need separate caches for different cases on different > > architectures, and a way to specify what cache should be used in the > > allocator API, but that does not contradict a single structure for arch > > specific parameters, but only makes it more elaborate, e.g. something like > > > > enum jit_type { > > JIT_MODULES_TEXT, > > JIT_MODULES_DATA, > > JIT_KPROBES, > > JIT_FTRACE, > > JIT_BPF, > > JIT_TYPE_MAX, > > }; > > > > struct jit_alloc_params { > > struct jit_rangeranges[JIT_TYPE_MAX]; > > /* ... */ > > }; > > > > > > Can you give more detail on what parameters you need? If the only extra > > > > parameter is just "does this allocation need to live close to kernel > > >
Re: [PATCH 00/13] mm: jit/text allocator
On Tue, Jun 13, 2023 at 02:56:14PM -0400, Kent Overstreet wrote: > On Thu, Jun 08, 2023 at 09:41:16PM +0300, Mike Rapoport wrote: > > On Tue, Jun 06, 2023 at 11:21:59AM -0700, Song Liu wrote: > > > On Mon, Jun 5, 2023 at 3:09 AM Mark Rutland wrote: > > > > > > [...] > > > > > > > > > > Can you give more detail on what parameters you need? If the only > > > > > > > extra > > > > > > > parameter is just "does this allocation need to live close to > > > > > > > kernel > > > > > > > text", that's not that big of a deal. > > > > > > > > > > > > My thinking was that we at least need the start + end for each > > > > > > caller. That > > > > > > might be it, tbh. > > > > > > > > > > Do you mean that modules will have something like > > > > > > > > > > jit_text_alloc(size, MODULES_START, MODULES_END); > > > > > > > > > > and kprobes will have > > > > > > > > > > jit_text_alloc(size, KPROBES_START, KPROBES_END); > > > > > ? > > > > > > > > Yes. > > > > > > How about we start with two APIs: > > > jit_text_alloc(size); > > > jit_text_alloc_range(size, start, end); > > > > > > AFAICT, arm64 is the only arch that requires the latter API. And TBH, I am > > > not quite convinced it is needed. > > > > Right now arm64 and riscv override bpf and kprobes allocations to use the > > entire vmalloc address space, but having the ability to allocate generated > > code outside of modules area may be useful for other architectures. > > > > Still the start + end for the callers feels backwards to me because the > > callers do not define the ranges, but rather the architectures, so we still > > need a way for architectures to define how they want allocate memory for > > the generated code. > > So, the start + end just comes from the need to keep relative pointers > under a certain size. I think this could be just a flag, I see no reason > to expose actual addresses here. It's the other way around. The start + end comes from the need to restrict allocation to certain range because of the relative addressing. I don't see how a flag can help here. -- Sincerely yours, Mike.
Re: [PATCH 00/13] mm: jit/text allocator
On Thu, Jun 08, 2023 at 09:41:16PM +0300, Mike Rapoport wrote: > On Tue, Jun 06, 2023 at 11:21:59AM -0700, Song Liu wrote: > > On Mon, Jun 5, 2023 at 3:09 AM Mark Rutland wrote: > > > > [...] > > > > > > > > Can you give more detail on what parameters you need? If the only > > > > > > extra > > > > > > parameter is just "does this allocation need to live close to kernel > > > > > > text", that's not that big of a deal. > > > > > > > > > > My thinking was that we at least need the start + end for each > > > > > caller. That > > > > > might be it, tbh. > > > > > > > > Do you mean that modules will have something like > > > > > > > > jit_text_alloc(size, MODULES_START, MODULES_END); > > > > > > > > and kprobes will have > > > > > > > > jit_text_alloc(size, KPROBES_START, KPROBES_END); > > > > ? > > > > > > Yes. > > > > How about we start with two APIs: > > jit_text_alloc(size); > > jit_text_alloc_range(size, start, end); > > > > AFAICT, arm64 is the only arch that requires the latter API. And TBH, I am > > not quite convinced it is needed. > > Right now arm64 and riscv override bpf and kprobes allocations to use the > entire vmalloc address space, but having the ability to allocate generated > code outside of modules area may be useful for other architectures. > > Still the start + end for the callers feels backwards to me because the > callers do not define the ranges, but rather the architectures, so we still > need a way for architectures to define how they want allocate memory for > the generated code. So, the start + end just comes from the need to keep relative pointers under a certain size. I think this could be just a flag, I see no reason to expose actual addresses here.
Re: [PATCH 00/13] mm: jit/text allocator
On Fri, Jun 09, 2023 at 10:02:16AM -0700, Song Liu wrote: > On Thu, Jun 8, 2023 at 11:41 AM Mike Rapoport wrote: > > > > On Tue, Jun 06, 2023 at 11:21:59AM -0700, Song Liu wrote: > > > On Mon, Jun 5, 2023 at 3:09 AM Mark Rutland wrote: > > > > > > [...] > > > > > > > > > > Can you give more detail on what parameters you need? If the only > > > > > > > extra > > > > > > > parameter is just "does this allocation need to live close to > > > > > > > kernel > > > > > > > text", that's not that big of a deal. > > > > > > > > > > > > My thinking was that we at least need the start + end for each > > > > > > caller. That > > > > > > might be it, tbh. > > > > > > > > > > Do you mean that modules will have something like > > > > > > > > > > jit_text_alloc(size, MODULES_START, MODULES_END); > > > > > > > > > > and kprobes will have > > > > > > > > > > jit_text_alloc(size, KPROBES_START, KPROBES_END); > > > > > ? > > > > > > > > Yes. > > > > > > How about we start with two APIs: > > > jit_text_alloc(size); > > > jit_text_alloc_range(size, start, end); > > > > > > AFAICT, arm64 is the only arch that requires the latter API. And TBH, I am > > > not quite convinced it is needed. > > > > Right now arm64 and riscv override bpf and kprobes allocations to use the > > entire vmalloc address space, but having the ability to allocate generated > > code outside of modules area may be useful for other architectures. > > > > Still the start + end for the callers feels backwards to me because the > > callers do not define the ranges, but rather the architectures, so we still > > need a way for architectures to define how they want allocate memory for > > the generated code. > > Yeah, this makes sense. > > > > > > > > It sill can be achieved with a single jit_alloc_arch_params(), just by > > > > > adding enum jit_type parameter to jit_text_alloc(). > > > > > > > > That feels backwards to me; it centralizes a bunch of information about > > > > distinct users to be able to shove that into a static array, when the > > > > callsites > > > > can pass that information. > > > > > > I think we only two type of users: module and everything else (ftrace, > > > kprobe, > > > bpf stuff). The key differences are: > > > > > > 1. module uses text and data; while everything else only uses text. > > > 2. module code is generated by the compiler, and thus has stronger > > > requirements in address ranges; everything else are generated via some > > > JIT or manual written assembly, so they are more flexible with address > > > ranges (in JIT, we can avoid using instructions that requires a specific > > > address range). > > > > > > The next question is, can we have the two types of users share the same > > > address ranges? If not, we can reserve the preferred range for modules, > > > and let everything else use the other range. I don't see reasons to > > > further > > > separate users in the "everything else" group. > > > > I agree that we can define only two types: modules and everything else and > > let the architectures define if they need different ranges for these two > > types, or want the same range for everything. > > > > With only two types we can have two API calls for alloc, and a single > > structure that defines the ranges etc from the architecture side rather > > than spread all over. > > > > Like something along these lines: > > > > struct execmem_range { > > unsigned long start; > > unsigned long end; > > unsigned long fallback_start; > > unsigned long fallback_end; > > pgprot_tpgprot; > > unsigned intalignment; > > }; > > > > struct execmem_modules_range { > > enum execmem_module_flags flags; > > struct execmem_range text; > > struct execmem_range data; > > }; > > > > struct execmem_jit_range { > > struct execmem_range text; > > }; > > > > struct execmem_params { > > struct execmem_modules_rangemodules; > > struct execmem_jit_rangejit; > > }; > > > > struct execmem_params *execmem_arch_params(void); > > > > void *execmem_text_alloc(size_t size); > > void *execmem_data_alloc(size_t size); > > void execmem_free(void *ptr); > > With the jit variation, maybe we can just call these > module_[text|data]_alloc()? I was thinking about "execmem_*_alloc()" for allocations that must be close to kernel image, like modules, ftrace on x86 and s390 and maybe something else in the future. And jit_text_alloc() for allocations that can reside anywhere. I tried to find a different name for 'struct execmem_modules_range' but couldn't think of anything better than 'struct execmem_close_to_kernel', so I've left modules in the name. > btw: Depending on the implementation of the allocator, we
Re: [PATCH 00/13] mm: jit/text allocator
On Thu, Jun 8, 2023 at 11:41 AM Mike Rapoport wrote: > > On Tue, Jun 06, 2023 at 11:21:59AM -0700, Song Liu wrote: > > On Mon, Jun 5, 2023 at 3:09 AM Mark Rutland wrote: > > > > [...] > > > > > > > > Can you give more detail on what parameters you need? If the only > > > > > > extra > > > > > > parameter is just "does this allocation need to live close to kernel > > > > > > text", that's not that big of a deal. > > > > > > > > > > My thinking was that we at least need the start + end for each > > > > > caller. That > > > > > might be it, tbh. > > > > > > > > Do you mean that modules will have something like > > > > > > > > jit_text_alloc(size, MODULES_START, MODULES_END); > > > > > > > > and kprobes will have > > > > > > > > jit_text_alloc(size, KPROBES_START, KPROBES_END); > > > > ? > > > > > > Yes. > > > > How about we start with two APIs: > > jit_text_alloc(size); > > jit_text_alloc_range(size, start, end); > > > > AFAICT, arm64 is the only arch that requires the latter API. And TBH, I am > > not quite convinced it is needed. > > Right now arm64 and riscv override bpf and kprobes allocations to use the > entire vmalloc address space, but having the ability to allocate generated > code outside of modules area may be useful for other architectures. > > Still the start + end for the callers feels backwards to me because the > callers do not define the ranges, but rather the architectures, so we still > need a way for architectures to define how they want allocate memory for > the generated code. Yeah, this makes sense. > > > > > It sill can be achieved with a single jit_alloc_arch_params(), just by > > > > adding enum jit_type parameter to jit_text_alloc(). > > > > > > That feels backwards to me; it centralizes a bunch of information about > > > distinct users to be able to shove that into a static array, when the > > > callsites > > > can pass that information. > > > > I think we only two type of users: module and everything else (ftrace, > > kprobe, > > bpf stuff). The key differences are: > > > > 1. module uses text and data; while everything else only uses text. > > 2. module code is generated by the compiler, and thus has stronger > > requirements in address ranges; everything else are generated via some > > JIT or manual written assembly, so they are more flexible with address > > ranges (in JIT, we can avoid using instructions that requires a specific > > address range). > > > > The next question is, can we have the two types of users share the same > > address ranges? If not, we can reserve the preferred range for modules, > > and let everything else use the other range. I don't see reasons to further > > separate users in the "everything else" group. > > I agree that we can define only two types: modules and everything else and > let the architectures define if they need different ranges for these two > types, or want the same range for everything. > > With only two types we can have two API calls for alloc, and a single > structure that defines the ranges etc from the architecture side rather > than spread all over. > > Like something along these lines: > > struct execmem_range { > unsigned long start; > unsigned long end; > unsigned long fallback_start; > unsigned long fallback_end; > pgprot_tpgprot; > unsigned intalignment; > }; > > struct execmem_modules_range { > enum execmem_module_flags flags; > struct execmem_range text; > struct execmem_range data; > }; > > struct execmem_jit_range { > struct execmem_range text; > }; > > struct execmem_params { > struct execmem_modules_rangemodules; > struct execmem_jit_rangejit; > }; > > struct execmem_params *execmem_arch_params(void); > > void *execmem_text_alloc(size_t size); > void *execmem_data_alloc(size_t size); > void execmem_free(void *ptr); With the jit variation, maybe we can just call these module_[text|data]_alloc()? btw: Depending on the implementation of the allocator, we may also need separate free()s for text and data. > > void *jit_text_alloc(size_t size); > void jit_free(void *ptr); > [...] How should we move ahead from here? AFAICT, all these changes can be easily extended and refactored in the future, so we don't have to make it perfect the first time. OTOH, having the interface committed (either this set or my module_alloc_type version) can unblock works in the binpack allocator and the users side. Therefore, I think we can move relatively fast here? Thanks, Song
Re: [PATCH 00/13] mm: jit/text allocator
On Tue, Jun 06, 2023 at 11:21:59AM -0700, Song Liu wrote: > On Mon, Jun 5, 2023 at 3:09 AM Mark Rutland wrote: > > [...] > > > > > > Can you give more detail on what parameters you need? If the only > > > > > extra > > > > > parameter is just "does this allocation need to live close to kernel > > > > > text", that's not that big of a deal. > > > > > > > > My thinking was that we at least need the start + end for each caller. > > > > That > > > > might be it, tbh. > > > > > > Do you mean that modules will have something like > > > > > > jit_text_alloc(size, MODULES_START, MODULES_END); > > > > > > and kprobes will have > > > > > > jit_text_alloc(size, KPROBES_START, KPROBES_END); > > > ? > > > > Yes. > > How about we start with two APIs: > jit_text_alloc(size); > jit_text_alloc_range(size, start, end); > > AFAICT, arm64 is the only arch that requires the latter API. And TBH, I am > not quite convinced it is needed. Right now arm64 and riscv override bpf and kprobes allocations to use the entire vmalloc address space, but having the ability to allocate generated code outside of modules area may be useful for other architectures. Still the start + end for the callers feels backwards to me because the callers do not define the ranges, but rather the architectures, so we still need a way for architectures to define how they want allocate memory for the generated code. > > > It sill can be achieved with a single jit_alloc_arch_params(), just by > > > adding enum jit_type parameter to jit_text_alloc(). > > > > That feels backwards to me; it centralizes a bunch of information about > > distinct users to be able to shove that into a static array, when the > > callsites > > can pass that information. > > I think we only two type of users: module and everything else (ftrace, kprobe, > bpf stuff). The key differences are: > > 1. module uses text and data; while everything else only uses text. > 2. module code is generated by the compiler, and thus has stronger > requirements in address ranges; everything else are generated via some > JIT or manual written assembly, so they are more flexible with address > ranges (in JIT, we can avoid using instructions that requires a specific > address range). > > The next question is, can we have the two types of users share the same > address ranges? If not, we can reserve the preferred range for modules, > and let everything else use the other range. I don't see reasons to further > separate users in the "everything else" group. I agree that we can define only two types: modules and everything else and let the architectures define if they need different ranges for these two types, or want the same range for everything. With only two types we can have two API calls for alloc, and a single structure that defines the ranges etc from the architecture side rather than spread all over. Like something along these lines: struct execmem_range { unsigned long start; unsigned long end; unsigned long fallback_start; unsigned long fallback_end; pgprot_tpgprot; unsigned intalignment; }; struct execmem_modules_range { enum execmem_module_flags flags; struct execmem_range text; struct execmem_range data; }; struct execmem_jit_range { struct execmem_range text; }; struct execmem_params { struct execmem_modules_rangemodules; struct execmem_jit_rangejit; }; struct execmem_params *execmem_arch_params(void); void *execmem_text_alloc(size_t size); void *execmem_data_alloc(size_t size); void execmem_free(void *ptr); void *jit_text_alloc(size_t size); void jit_free(void *ptr); Modules or anything that must live close to the kernel image can use execmem_*_alloc() and the callers that don't generally care about relative addressing will use jit_text_alloc(), presuming that arch will restrict jit range if necessary, like e.g. below for arm64 jit can be anywhere in vmalloc and for x86 and s390 it will share the modules range. struct execmem_params arm64_execmem = { .modules = { .flags = KASAN, .text = { .start = MODULES_VADDR, .end = MODULES_END, .pgprot = PAGE_KERNEL_ROX, .fallback_start = VMALLOC_START, .fallback_start = VMALLOC_END, }, }, .jit = { .text = { .start = VMALLOC_START, .end = VMALLOC_END, .pgprot =
Re: [PATCH 00/13] mm: jit/text allocator
On Mon, Jun 5, 2023 at 3:09 AM Mark Rutland wrote: [...] > > > > Can you give more detail on what parameters you need? If the only extra > > > > parameter is just "does this allocation need to live close to kernel > > > > text", that's not that big of a deal. > > > > > > My thinking was that we at least need the start + end for each caller. > > > That > > > might be it, tbh. > > > > Do you mean that modules will have something like > > > > jit_text_alloc(size, MODULES_START, MODULES_END); > > > > and kprobes will have > > > > jit_text_alloc(size, KPROBES_START, KPROBES_END); > > ? > > Yes. How about we start with two APIs: jit_text_alloc(size); jit_text_alloc_range(size, start, end); AFAICT, arm64 is the only arch that requires the latter API. And TBH, I am not quite convinced it is needed. > > > It sill can be achieved with a single jit_alloc_arch_params(), just by > > adding enum jit_type parameter to jit_text_alloc(). > > That feels backwards to me; it centralizes a bunch of information about > distinct users to be able to shove that into a static array, when the > callsites > can pass that information. I think we only two type of users: module and everything else (ftrace, kprobe, bpf stuff). The key differences are: 1. module uses text and data; while everything else only uses text. 2. module code is generated by the compiler, and thus has stronger requirements in address ranges; everything else are generated via some JIT or manual written assembly, so they are more flexible with address ranges (in JIT, we can avoid using instructions that requires a specific address range). The next question is, can we have the two types of users share the same address ranges? If not, we can reserve the preferred range for modules, and let everything else use the other range. I don't see reasons to further separate users in the "everything else" group. > > What's *actually* common after separating out the ranges? Is it just the > permissions? I believe permission is the key, as we need the hardware to enforce permission. > > If we want this to be able to share allocations and so on, why can't we do > this > like a kmem_cache, and have the callsite pass a pointer to the allocator data? > That would make it easy for callsites to share an allocator or use a distinct > one. Sharing among different call sites will give us more benefit (in TLB misses rate, etc.). For example, a 2MB page may host text of two kernel modules, 4 kprobes, 6 ftrace trampolines, and 10 BPF programs. All of these only require one entry in the iTLB. Thanks, Song
Re: [PATCH 00/13] mm: jit/text allocator
On Mon, Jun 05, 2023 at 11:09:34AM +0100, Mark Rutland wrote: > On Mon, Jun 05, 2023 at 12:20:40PM +0300, Mike Rapoport wrote: > > On Fri, Jun 02, 2023 at 10:35:09AM +0100, Mark Rutland wrote: > > > > It sill can be achieved with a single jit_alloc_arch_params(), just by > > adding enum jit_type parameter to jit_text_alloc(). > > That feels backwards to me; it centralizes a bunch of information about > distinct users to be able to shove that into a static array, when the > callsites > can pass that information. The goal was not to shove everything into an array, but centralize architecture requirements for code allocations. The callsites don't have that information per se, they get it from the arch code, so having this information in a single place per arch is better than spreading MODULE_START, KPROBES_START etc all over. I'd agree though that having types for jit_text_alloc is ugly and this should be handled differently. > What's *actually* common after separating out the ranges? Is it just the > permissions? On x86 everything, on arm64 apparently just the permissions. I've started to summarize what are the restrictions for code placement for modules, kprobes and bpf on different architectures, that's roughly what I've got so far: * x86 and s390 need everything within modules address space because of PC-relative * arm, arm64, loongarch, sparc64, riscv64, some of mips and powerpc32 configurations require a dedicated modules address space; the rest just use vmalloc address space * all architectures that support kprobes except x86 and s390 don't use relative jumps, so they don't care where kprobes insn_page will live * not sure yet about BPF. Looks like on arm and arm64 it does not use relative jumps, so it can be anywhere, didn't dig enough about the others. > If we want this to be able to share allocations and so on, why can't we do > this > like a kmem_cache, and have the callsite pass a pointer to the allocator data? > That would make it easy for callsites to share an allocator or use a distinct > one. This maybe something worth exploring. > Thanks, > Mark. -- Sincerely yours, Mike.
Re: [PATCH 00/13] mm: jit/text allocator
On Mon, Jun 05, 2023 at 12:20:40PM +0300, Mike Rapoport wrote: > On Fri, Jun 02, 2023 at 10:35:09AM +0100, Mark Rutland wrote: > > On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > > > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > > > For a while I have wanted to give kprobes its own allocator so that it > > > > can work > > > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA > > > > space in > > > > the modules area. > > > > > > > > Given that, I think these should have their own allocator functions > > > > that can be > > > > provided independently, even if those happen to use common > > > > infrastructure. > > > > > > How much memory can kprobes conceivably use? I think we also want to try > > > to push back on combinatorial new allocators, if we can. > > > > That depends on who's using it, and how (e.g. via BPF). > > > > To be clear, I'm not necessarily asking for entirely different allocators, > > but > > I do thinkg that we want wrappers that can at least pass distinct start+end > > parameters to a common allocator, and for arm64's modules code I'd expect > > that > > we'd keep the range falblack logic out of the common allcoator, and just > > call > > it twice. > > > > > > > Several architectures override module_alloc() because of various > > > > > constraints where the executable memory can be located and this causes > > > > > additional obstacles for improvements of code allocation. > > > > > > > > > > This set splits code allocation from modules by introducing > > > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > > > > sites of module_alloc() and module_memfree() with the new APIs and > > > > > implements core text and related allocation in a central place. > > > > > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > > > architectures that require non-default behaviour for text allocation > > > > > must > > > > > fill jit_alloc_params structure and implement jit_alloc_arch_params() > > > > > that > > > > > returns a pointer to that structure. If an architecture does not > > > > > implement > > > > > jit_alloc_arch_params(), the defaults compatible with the current > > > > > modules::module_alloc() are used. > > > > > > > > As above, I suspect that each of the callsites should probably be using > > > > common > > > > infrastructure, but I don't think that a single jit_alloc_arch_params() > > > > makes > > > > sense, since the parameters for each case may need to be distinct. > > > > > > I don't see how that follows. The whole point of function parameters is > > > that they may be different :) > > > > What I mean is that jit_alloc_arch_params() tries to aggregate common > > parameters, but they aren't actually common (e.g. the actual start+end range > > for allocation). > > jit_alloc_arch_params() tries to aggregate architecture constraints and > requirements for allocations of executable memory and this exactly what > the first 6 patches of this set do. > > A while ago Thomas suggested to use a structure that parametrizes > architecture constraints by the memory type used in modules [1] and Song > implemented the infrastructure for it and x86 part [2]. > > I liked the idea of defining parameters in a single structure, but I > thought that approaching the problem from the arch side rather than from > modules perspective will be better starting point, hence these patches. > > I don't see a fundamental reason why a single structure cannot describe > what is needed for different code allocation cases, be it modules, kprobes > or bpf. There is of course an assumption that the core allocations will be > the same for all the users, and it seems to me that something like > > * allocate physical memory if allocator caches are empty > * map it in vmalloc or modules address space > * return memory from the allocator cache to the caller > > will work for all usecases. > > We might need separate caches for different cases on different > architectures, and a way to specify what cache should be used in the > allocator API, but that does not contradict a single structure for arch > specific parameters, but only makes it more elaborate, e.g. something like > > enum jit_type { > JIT_MODULES_TEXT, > JIT_MODULES_DATA, > JIT_KPROBES, > JIT_FTRACE, > JIT_BPF, > JIT_TYPE_MAX, > }; Why would we actually need different enums for modules_text, kprobes, ftrace and bpf? Why can't we treat all text allocations the same? The reason we can't do that currently is because modules need to go in a 128Mb region on some archs, and without sub page allocation bpf/kprobes/etc. burn a full page for each allocation. But we're doing sub page allocation - right? That leaves module data - which really needs to be split out into rw, ro, ro_after_init - but I'm not sure we'd even want the same API for those, they need fairly different page permissions
Re: [PATCH 00/13] mm: jit/text allocator
On Mon, Jun 05, 2023 at 12:20:40PM +0300, Mike Rapoport wrote: > On Fri, Jun 02, 2023 at 10:35:09AM +0100, Mark Rutland wrote: > > On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > > > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > > > For a while I have wanted to give kprobes its own allocator so that it > > > > can work > > > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA > > > > space in > > > > the modules area. > > > > > > > > Given that, I think these should have their own allocator functions > > > > that can be > > > > provided independently, even if those happen to use common > > > > infrastructure. > > > > > > How much memory can kprobes conceivably use? I think we also want to try > > > to push back on combinatorial new allocators, if we can. > > > > That depends on who's using it, and how (e.g. via BPF). > > > > To be clear, I'm not necessarily asking for entirely different allocators, > > but > > I do thinkg that we want wrappers that can at least pass distinct start+end > > parameters to a common allocator, and for arm64's modules code I'd expect > > that > > we'd keep the range falblack logic out of the common allcoator, and just > > call > > it twice. > > > > > > > Several architectures override module_alloc() because of various > > > > > constraints where the executable memory can be located and this causes > > > > > additional obstacles for improvements of code allocation. > > > > > > > > > > This set splits code allocation from modules by introducing > > > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > > > > sites of module_alloc() and module_memfree() with the new APIs and > > > > > implements core text and related allocation in a central place. > > > > > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > > > architectures that require non-default behaviour for text allocation > > > > > must > > > > > fill jit_alloc_params structure and implement jit_alloc_arch_params() > > > > > that > > > > > returns a pointer to that structure. If an architecture does not > > > > > implement > > > > > jit_alloc_arch_params(), the defaults compatible with the current > > > > > modules::module_alloc() are used. > > > > > > > > As above, I suspect that each of the callsites should probably be using > > > > common > > > > infrastructure, but I don't think that a single jit_alloc_arch_params() > > > > makes > > > > sense, since the parameters for each case may need to be distinct. > > > > > > I don't see how that follows. The whole point of function parameters is > > > that they may be different :) > > > > What I mean is that jit_alloc_arch_params() tries to aggregate common > > parameters, but they aren't actually common (e.g. the actual start+end range > > for allocation). > > jit_alloc_arch_params() tries to aggregate architecture constraints and > requirements for allocations of executable memory and this exactly what > the first 6 patches of this set do. > > A while ago Thomas suggested to use a structure that parametrizes > architecture constraints by the memory type used in modules [1] and Song > implemented the infrastructure for it and x86 part [2]. > > I liked the idea of defining parameters in a single structure, but I > thought that approaching the problem from the arch side rather than from > modules perspective will be better starting point, hence these patches. > > I don't see a fundamental reason why a single structure cannot describe > what is needed for different code allocation cases, be it modules, kprobes > or bpf. There is of course an assumption that the core allocations will be > the same for all the users, and it seems to me that something like > > * allocate physical memory if allocator caches are empty > * map it in vmalloc or modules address space > * return memory from the allocator cache to the caller > > will work for all usecases. > > We might need separate caches for different cases on different > architectures, and a way to specify what cache should be used in the > allocator API, but that does not contradict a single structure for arch > specific parameters, but only makes it more elaborate, e.g. something like > > enum jit_type { > JIT_MODULES_TEXT, > JIT_MODULES_DATA, > JIT_KPROBES, > JIT_FTRACE, > JIT_BPF, > JIT_TYPE_MAX, > }; > > struct jit_alloc_params { > struct jit_rangeranges[JIT_TYPE_MAX]; > /* ... */ > }; > > > > Can you give more detail on what parameters you need? If the only extra > > > parameter is just "does this allocation need to live close to kernel > > > text", that's not that big of a deal. > > > > My thinking was that we at least need the start + end for each caller. That > > might be it, tbh. > > Do you mean that modules will have something like > > jit_text_alloc(size, MODULES_START, MODULES_END); > > and kprobes will have > >
Re: [PATCH 00/13] mm: jit/text allocator
On Fri, Jun 02, 2023 at 10:35:09AM +0100, Mark Rutland wrote: > On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > > For a while I have wanted to give kprobes its own allocator so that it > > > can work > > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA space > > > in > > > the modules area. > > > > > > Given that, I think these should have their own allocator functions that > > > can be > > > provided independently, even if those happen to use common infrastructure. > > > > How much memory can kprobes conceivably use? I think we also want to try > > to push back on combinatorial new allocators, if we can. > > That depends on who's using it, and how (e.g. via BPF). > > To be clear, I'm not necessarily asking for entirely different allocators, but > I do thinkg that we want wrappers that can at least pass distinct start+end > parameters to a common allocator, and for arm64's modules code I'd expect that > we'd keep the range falblack logic out of the common allcoator, and just call > it twice. > > > > > Several architectures override module_alloc() because of various > > > > constraints where the executable memory can be located and this causes > > > > additional obstacles for improvements of code allocation. > > > > > > > > This set splits code allocation from modules by introducing > > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > > > sites of module_alloc() and module_memfree() with the new APIs and > > > > implements core text and related allocation in a central place. > > > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > > architectures that require non-default behaviour for text allocation > > > > must > > > > fill jit_alloc_params structure and implement jit_alloc_arch_params() > > > > that > > > > returns a pointer to that structure. If an architecture does not > > > > implement > > > > jit_alloc_arch_params(), the defaults compatible with the current > > > > modules::module_alloc() are used. > > > > > > As above, I suspect that each of the callsites should probably be using > > > common > > > infrastructure, but I don't think that a single jit_alloc_arch_params() > > > makes > > > sense, since the parameters for each case may need to be distinct. > > > > I don't see how that follows. The whole point of function parameters is > > that they may be different :) > > What I mean is that jit_alloc_arch_params() tries to aggregate common > parameters, but they aren't actually common (e.g. the actual start+end range > for allocation). jit_alloc_arch_params() tries to aggregate architecture constraints and requirements for allocations of executable memory and this exactly what the first 6 patches of this set do. A while ago Thomas suggested to use a structure that parametrizes architecture constraints by the memory type used in modules [1] and Song implemented the infrastructure for it and x86 part [2]. I liked the idea of defining parameters in a single structure, but I thought that approaching the problem from the arch side rather than from modules perspective will be better starting point, hence these patches. I don't see a fundamental reason why a single structure cannot describe what is needed for different code allocation cases, be it modules, kprobes or bpf. There is of course an assumption that the core allocations will be the same for all the users, and it seems to me that something like * allocate physical memory if allocator caches are empty * map it in vmalloc or modules address space * return memory from the allocator cache to the caller will work for all usecases. We might need separate caches for different cases on different architectures, and a way to specify what cache should be used in the allocator API, but that does not contradict a single structure for arch specific parameters, but only makes it more elaborate, e.g. something like enum jit_type { JIT_MODULES_TEXT, JIT_MODULES_DATA, JIT_KPROBES, JIT_FTRACE, JIT_BPF, JIT_TYPE_MAX, }; struct jit_alloc_params { struct jit_rangeranges[JIT_TYPE_MAX]; /* ... */ }; > > Can you give more detail on what parameters you need? If the only extra > > parameter is just "does this allocation need to live close to kernel > > text", that's not that big of a deal. > > My thinking was that we at least need the start + end for each caller. That > might be it, tbh. Do you mean that modules will have something like jit_text_alloc(size, MODULES_START, MODULES_END); and kprobes will have jit_text_alloc(size, KPROBES_START, KPROBES_END); ? It sill can be achieved with a single jit_alloc_arch_params(), just by adding enum jit_type parameter to jit_text_alloc(). [1] https://lore.kernel.org/linux-mm/87v8mndy3y.ffs@tglx/ [2]
Re: [PATCH 00/13] mm: jit/text allocator
On Sun, Jun 4, 2023 at 2:40 PM Kent Overstreet wrote: > > On Sun, Jun 04, 2023 at 02:22:30PM -0700, Song Liu wrote: > > On Sun, Jun 4, 2023 at 11:02 AM Kent Overstreet > > wrote: > > > > > > On Fri, Jun 02, 2023 at 11:20:58AM -0700, Song Liu wrote: > > > > IIUC, arm64 uses VMALLOC address space for BPF programs. The reason > > > > is each BPF program uses at least 64kB (one page) out of the 128MB > > > > address space. Puranjay Mohan (CC'ed) is working on enabling > > > > bpf_prog_pack for arm64. Once this work is done, multiple BPF programs > > > > will be able to share a page. Will this improvement remove the need to > > > > specify a different address range for BPF programs? > > > > > > Can we please stop working on BPF specific sub page allocation and focus > > > on doing this in mm/? This never should have been in BPF in the first > > > place. > > > > That work is mostly independent of the allocator work we are discussing > > here. > > The goal Puranjay's work is to enable the arm64 BPF JIT engine to use a > > ROX allocator. The allocator could be the bpf_prog_pack allocator, or > > jitalloc, > > or module_alloc_type. Puranjay is using bpf_prog_alloc for now. But once > > jitalloc or module_alloc_type (either one) is merged, we will migrate BPF > > JIT engines (x86_64 and arm64) to the new allocator and then tear down > > bpf_prog_pack. > > > > Does this make sense? > > Yeah, as long as that's the plan. Maybe one of you could tell us what > issues were preventing prog_pack from being used in the first place, it > might be relevant - this is the time to get the new allocator API right. The JIT engine does a lot of writes. Instead of doing many text_poke(), we are using a temporary RW write buffer, and then do text_poke_copy() at the end. To make this work, we need the JIT engine to be able to handle an RW temporary buffer and an RO final memory region. There is nothing preventing prog_pack to work. It is just we need to do the work. Thanks, Song
Re: [PATCH 00/13] mm: jit/text allocator
On Sun, Jun 04, 2023 at 02:22:30PM -0700, Song Liu wrote: > On Sun, Jun 4, 2023 at 11:02 AM Kent Overstreet > wrote: > > > > On Fri, Jun 02, 2023 at 11:20:58AM -0700, Song Liu wrote: > > > IIUC, arm64 uses VMALLOC address space for BPF programs. The reason > > > is each BPF program uses at least 64kB (one page) out of the 128MB > > > address space. Puranjay Mohan (CC'ed) is working on enabling > > > bpf_prog_pack for arm64. Once this work is done, multiple BPF programs > > > will be able to share a page. Will this improvement remove the need to > > > specify a different address range for BPF programs? > > > > Can we please stop working on BPF specific sub page allocation and focus > > on doing this in mm/? This never should have been in BPF in the first > > place. > > That work is mostly independent of the allocator work we are discussing here. > The goal Puranjay's work is to enable the arm64 BPF JIT engine to use a > ROX allocator. The allocator could be the bpf_prog_pack allocator, or > jitalloc, > or module_alloc_type. Puranjay is using bpf_prog_alloc for now. But once > jitalloc or module_alloc_type (either one) is merged, we will migrate BPF > JIT engines (x86_64 and arm64) to the new allocator and then tear down > bpf_prog_pack. > > Does this make sense? Yeah, as long as that's the plan. Maybe one of you could tell us what issues were preventing prog_pack from being used in the first place, it might be relevant - this is the time to get the new allocator API right.
Re: [PATCH 00/13] mm: jit/text allocator
On Sun, Jun 4, 2023 at 11:02 AM Kent Overstreet wrote: > > On Fri, Jun 02, 2023 at 11:20:58AM -0700, Song Liu wrote: > > IIUC, arm64 uses VMALLOC address space for BPF programs. The reason > > is each BPF program uses at least 64kB (one page) out of the 128MB > > address space. Puranjay Mohan (CC'ed) is working on enabling > > bpf_prog_pack for arm64. Once this work is done, multiple BPF programs > > will be able to share a page. Will this improvement remove the need to > > specify a different address range for BPF programs? > > Can we please stop working on BPF specific sub page allocation and focus > on doing this in mm/? This never should have been in BPF in the first > place. That work is mostly independent of the allocator work we are discussing here. The goal Puranjay's work is to enable the arm64 BPF JIT engine to use a ROX allocator. The allocator could be the bpf_prog_pack allocator, or jitalloc, or module_alloc_type. Puranjay is using bpf_prog_alloc for now. But once jitalloc or module_alloc_type (either one) is merged, we will migrate BPF JIT engines (x86_64 and arm64) to the new allocator and then tear down bpf_prog_pack. Does this make sense? Thanks, Song
Re: [PATCH 00/13] mm: jit/text allocator
On Fri, Jun 02, 2023 at 11:20:58AM -0700, Song Liu wrote: > IIUC, arm64 uses VMALLOC address space for BPF programs. The reason > is each BPF program uses at least 64kB (one page) out of the 128MB > address space. Puranjay Mohan (CC'ed) is working on enabling > bpf_prog_pack for arm64. Once this work is done, multiple BPF programs > will be able to share a page. Will this improvement remove the need to > specify a different address range for BPF programs? Can we please stop working on BPF specific sub page allocation and focus on doing this in mm/? This never should have been in BPF in the first place.
Re: [PATCH 00/13] mm: jit/text allocator
On Fri, Jun 2, 2023 at 8:21 PM Song Liu wrote: > > On Fri, Jun 2, 2023 at 2:35 AM Mark Rutland wrote: > > > > On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > > > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > > > For a while I have wanted to give kprobes its own allocator so that it > > > > can work > > > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA > > > > space in > > > > the modules area. > > > > > > > > Given that, I think these should have their own allocator functions > > > > that can be > > > > provided independently, even if those happen to use common > > > > infrastructure. > > > > > > How much memory can kprobes conceivably use? I think we also want to try > > > to push back on combinatorial new allocators, if we can. > > > > That depends on who's using it, and how (e.g. via BPF). > > > > To be clear, I'm not necessarily asking for entirely different allocators, > > but > > I do thinkg that we want wrappers that can at least pass distinct start+end > > parameters to a common allocator, and for arm64's modules code I'd expect > > that > > we'd keep the range falblack logic out of the common allcoator, and just > > call > > it twice. > > > > > > > Several architectures override module_alloc() because of various > > > > > constraints where the executable memory can be located and this causes > > > > > additional obstacles for improvements of code allocation. > > > > > > > > > > This set splits code allocation from modules by introducing > > > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > > > > sites of module_alloc() and module_memfree() with the new APIs and > > > > > implements core text and related allocation in a central place. > > > > > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > > > architectures that require non-default behaviour for text allocation > > > > > must > > > > > fill jit_alloc_params structure and implement jit_alloc_arch_params() > > > > > that > > > > > returns a pointer to that structure. If an architecture does not > > > > > implement > > > > > jit_alloc_arch_params(), the defaults compatible with the current > > > > > modules::module_alloc() are used. > > > > > > > > As above, I suspect that each of the callsites should probably be using > > > > common > > > > infrastructure, but I don't think that a single jit_alloc_arch_params() > > > > makes > > > > sense, since the parameters for each case may need to be distinct. > > > > > > I don't see how that follows. The whole point of function parameters is > > > that they may be different :) > > > > What I mean is that jit_alloc_arch_params() tries to aggregate common > > parameters, but they aren't actually common (e.g. the actual start+end range > > for allocation). > > > > > Can you give more detail on what parameters you need? If the only extra > > > parameter is just "does this allocation need to live close to kernel > > > text", that's not that big of a deal. > > > > My thinking was that we at least need the start + end for each caller. That > > might be it, tbh. > > IIUC, arm64 uses VMALLOC address space for BPF programs. The reason > is each BPF program uses at least 64kB (one page) out of the 128MB > address space. Puranjay Mohan (CC'ed) is working on enabling > bpf_prog_pack for arm64. Once this work is done, multiple BPF programs > will be able to share a page. Will this improvement remove the need to > specify a different address range for BPF programs? Hi, Thanks for adding me to the conversation. The ARM64 BPF JIT used to allocate the memory using module_alloc but it was not optimal because BPF programs and modules were sharing the 128 MB module region. This was fixed by 91fc957c9b1d ("arm64/bpf: don't allocate BPF JIT programs in module memory") It created a dedicated 128 MB region set aside for BPF programs. But 128MB could get exhausted especially where PAGE_SIZE is 64KB - one page is needed per program. This restriction was removed by b89ddf4cca43 ("arm64/bpf: Remove 128MB limit for BPF JIT programs") So, currently BPF programs are using a full page from vmalloc (4 KB, 16 KB, or 64 KB). This wastes memory and also causes iTLB pressure. Enabling bpf_prog_pack for ARM64 would fix it. I am doing some final tests and will send the patches in 1-2 days. Thanks, Puranjay
Re: [PATCH 00/13] mm: jit/text allocator
On Fri, Jun 2, 2023 at 2:35 AM Mark Rutland wrote: > > On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > > For a while I have wanted to give kprobes its own allocator so that it > > > can work > > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA space > > > in > > > the modules area. > > > > > > Given that, I think these should have their own allocator functions that > > > can be > > > provided independently, even if those happen to use common infrastructure. > > > > How much memory can kprobes conceivably use? I think we also want to try > > to push back on combinatorial new allocators, if we can. > > That depends on who's using it, and how (e.g. via BPF). > > To be clear, I'm not necessarily asking for entirely different allocators, but > I do thinkg that we want wrappers that can at least pass distinct start+end > parameters to a common allocator, and for arm64's modules code I'd expect that > we'd keep the range falblack logic out of the common allcoator, and just call > it twice. > > > > > Several architectures override module_alloc() because of various > > > > constraints where the executable memory can be located and this causes > > > > additional obstacles for improvements of code allocation. > > > > > > > > This set splits code allocation from modules by introducing > > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > > > sites of module_alloc() and module_memfree() with the new APIs and > > > > implements core text and related allocation in a central place. > > > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > > architectures that require non-default behaviour for text allocation > > > > must > > > > fill jit_alloc_params structure and implement jit_alloc_arch_params() > > > > that > > > > returns a pointer to that structure. If an architecture does not > > > > implement > > > > jit_alloc_arch_params(), the defaults compatible with the current > > > > modules::module_alloc() are used. > > > > > > As above, I suspect that each of the callsites should probably be using > > > common > > > infrastructure, but I don't think that a single jit_alloc_arch_params() > > > makes > > > sense, since the parameters for each case may need to be distinct. > > > > I don't see how that follows. The whole point of function parameters is > > that they may be different :) > > What I mean is that jit_alloc_arch_params() tries to aggregate common > parameters, but they aren't actually common (e.g. the actual start+end range > for allocation). > > > Can you give more detail on what parameters you need? If the only extra > > parameter is just "does this allocation need to live close to kernel > > text", that's not that big of a deal. > > My thinking was that we at least need the start + end for each caller. That > might be it, tbh. IIUC, arm64 uses VMALLOC address space for BPF programs. The reason is each BPF program uses at least 64kB (one page) out of the 128MB address space. Puranjay Mohan (CC'ed) is working on enabling bpf_prog_pack for arm64. Once this work is done, multiple BPF programs will be able to share a page. Will this improvement remove the need to specify a different address range for BPF programs? Thanks, Song
Re: [PATCH 00/13] mm: jit/text allocator
On Thu, Jun 01, 2023 at 02:14:56PM -0400, Kent Overstreet wrote: > On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > > For a while I have wanted to give kprobes its own allocator so that it can > > work > > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA space in > > the modules area. > > > > Given that, I think these should have their own allocator functions that > > can be > > provided independently, even if those happen to use common infrastructure. > > How much memory can kprobes conceivably use? I think we also want to try > to push back on combinatorial new allocators, if we can. That depends on who's using it, and how (e.g. via BPF). To be clear, I'm not necessarily asking for entirely different allocators, but I do thinkg that we want wrappers that can at least pass distinct start+end parameters to a common allocator, and for arm64's modules code I'd expect that we'd keep the range falblack logic out of the common allcoator, and just call it twice. > > > Several architectures override module_alloc() because of various > > > constraints where the executable memory can be located and this causes > > > additional obstacles for improvements of code allocation. > > > > > > This set splits code allocation from modules by introducing > > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > > sites of module_alloc() and module_memfree() with the new APIs and > > > implements core text and related allocation in a central place. > > > > > > Instead of architecture specific overrides for module_alloc(), the > > > architectures that require non-default behaviour for text allocation must > > > fill jit_alloc_params structure and implement jit_alloc_arch_params() that > > > returns a pointer to that structure. If an architecture does not implement > > > jit_alloc_arch_params(), the defaults compatible with the current > > > modules::module_alloc() are used. > > > > As above, I suspect that each of the callsites should probably be using > > common > > infrastructure, but I don't think that a single jit_alloc_arch_params() > > makes > > sense, since the parameters for each case may need to be distinct. > > I don't see how that follows. The whole point of function parameters is > that they may be different :) What I mean is that jit_alloc_arch_params() tries to aggregate common parameters, but they aren't actually common (e.g. the actual start+end range for allocation). > Can you give more detail on what parameters you need? If the only extra > parameter is just "does this allocation need to live close to kernel > text", that's not that big of a deal. My thinking was that we at least need the start + end for each caller. That might be it, tbh. Thanks, Mark.
Re: [PATCH 00/13] mm: jit/text allocator
On Thu, Jun 1, 2023 at 3:13 AM Mike Rapoport wrote: > > From: "Mike Rapoport (IBM)" > > Hi, > > module_alloc() is used everywhere as a mean to allocate memory for code. > > Beside being semantically wrong, this unnecessarily ties all subsystmes > that need to allocate code, such as ftrace, kprobes and BPF to modules > and puts the burden of code allocation to the modules code. > > Several architectures override module_alloc() because of various > constraints where the executable memory can be located and this causes > additional obstacles for improvements of code allocation. > > This set splits code allocation from modules by introducing > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > sites of module_alloc() and module_memfree() with the new APIs and > implements core text and related allocation in a central place. > > Instead of architecture specific overrides for module_alloc(), the > architectures that require non-default behaviour for text allocation must > fill jit_alloc_params structure and implement jit_alloc_arch_params() that > returns a pointer to that structure. If an architecture does not implement > jit_alloc_arch_params(), the defaults compatible with the current > modules::module_alloc() are used. > > The new jitalloc infrastructure allows decoupling of kprobes and ftrace > from modules, and most importantly it enables ROX allocations for > executable memory. This set does look cleaner than my version [1]. However, this is partially because this set only separates text and data; while [1] also separates rw data, ro data, and ro_after_init data. We need such separation to fully cover module usage, and to remove VM_FLUSH_RESET_PERMS. Once we add these logic to this set, the two versions will look similar. OTOH, I do like the fact this version enables kprobes (and potentially ftrace and bpf) without CONFIG_MODULES. And mm/ seems a better home for the logic. That being said, besides comments in a few patches, this version looks good to me. With the fix I suggested for patch 12/13, it passed my tests on x86_64 with modules, kprobes, ftrace, and BPF. If we decided to ship this version, I would appreciate it if I could get more credit for my work in [1] and research work before that. Thanks, Song [1] https://lore.kernel.org/lkml/20230526051529.3387103-1-s...@kernel.org/
Re: [PATCH 00/13] mm: jit/text allocator
On Thu, Jun 01, 2023 at 05:12:03PM +0100, Mark Rutland wrote: > For a while I have wanted to give kprobes its own allocator so that it can > work > even with CONFIG_MODULES=n, and so that it doesn't have to waste VA space in > the modules area. > > Given that, I think these should have their own allocator functions that can > be > provided independently, even if those happen to use common infrastructure. How much memory can kprobes conceivably use? I think we also want to try to push back on combinatorial new allocators, if we can. > > Several architectures override module_alloc() because of various > > constraints where the executable memory can be located and this causes > > additional obstacles for improvements of code allocation. > > > > This set splits code allocation from modules by introducing > > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > > sites of module_alloc() and module_memfree() with the new APIs and > > implements core text and related allocation in a central place. > > > > Instead of architecture specific overrides for module_alloc(), the > > architectures that require non-default behaviour for text allocation must > > fill jit_alloc_params structure and implement jit_alloc_arch_params() that > > returns a pointer to that structure. If an architecture does not implement > > jit_alloc_arch_params(), the defaults compatible with the current > > modules::module_alloc() are used. > > As above, I suspect that each of the callsites should probably be using common > infrastructure, but I don't think that a single jit_alloc_arch_params() makes > sense, since the parameters for each case may need to be distinct. I don't see how that follows. The whole point of function parameters is that they may be different :) Can you give more detail on what parameters you need? If the only extra parameter is just "does this allocation need to live close to kernel text", that's not that big of a deal.
Re: [PATCH 00/13] mm: jit/text allocator
Hi Mike, On Thu, Jun 01, 2023 at 01:12:44PM +0300, Mike Rapoport wrote: > From: "Mike Rapoport (IBM)" > > Hi, > > module_alloc() is used everywhere as a mean to allocate memory for code. > > Beside being semantically wrong, this unnecessarily ties all subsystmes > that need to allocate code, such as ftrace, kprobes and BPF to modules > and puts the burden of code allocation to the modules code. I agree this is a problem, and one key issue here is that these can have different requirements. For example, on arm64 we need modules to be placed within a 128M or 2G window containing the kernel, whereas it would be safe for the kprobes XOL area to be placed arbitrarily far from the kernel image (since we don't allow PC-relative insns to be stepped out-of-line). Likewise arm64 doesn't have ftrace trampolines, and DIRECT_CALL trampolines can safely be placed arbitarily far from the kernel image. For a while I have wanted to give kprobes its own allocator so that it can work even with CONFIG_MODULES=n, and so that it doesn't have to waste VA space in the modules area. Given that, I think these should have their own allocator functions that can be provided independently, even if those happen to use common infrastructure. > Several architectures override module_alloc() because of various > constraints where the executable memory can be located and this causes > additional obstacles for improvements of code allocation. > > This set splits code allocation from modules by introducing > jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call > sites of module_alloc() and module_memfree() with the new APIs and > implements core text and related allocation in a central place. > > Instead of architecture specific overrides for module_alloc(), the > architectures that require non-default behaviour for text allocation must > fill jit_alloc_params structure and implement jit_alloc_arch_params() that > returns a pointer to that structure. If an architecture does not implement > jit_alloc_arch_params(), the defaults compatible with the current > modules::module_alloc() are used. As above, I suspect that each of the callsites should probably be using common infrastructure, but I don't think that a single jit_alloc_arch_params() makes sense, since the parameters for each case may need to be distinct. > The new jitalloc infrastructure allows decoupling of kprobes and ftrace > from modules, and most importantly it enables ROX allocations for > executable memory. > > A centralized infrastructure for code allocation allows future > optimizations for allocations of executable memory, caching large pages for > better iTLB performance and providing sub-page allocations for users that > only need small jit code snippets. This sounds interesting, but I think this can be achieved without requiring a single jit_alloc_arch_params() shared by all users? Thanks, Mark. > > patches 1-5: split out the code allocation from modules and arch > patch 6: add dedicated API for data allocations with constraints similar to > code allocations > patches 7-9: decouple dynamic ftrace and kprobes form CONFIG_MODULES > patches 10-13: enable ROX allocations for executable memory on x86 > > Mike Rapoport (IBM) (11): > nios2: define virtual address space for modules > mm: introduce jit_text_alloc() and use it instead of module_alloc() > mm/jitalloc, arch: convert simple overrides of module_alloc to jitalloc > mm/jitalloc, arch: convert remaining overrides of module_alloc to jitalloc > module, jitalloc: drop module_alloc > mm/jitalloc: introduce jit_data_alloc() > x86/ftrace: enable dynamic ftrace without CONFIG_MODULES > arch: make jitalloc setup available regardless of CONFIG_MODULES > kprobes: remove dependcy on CONFIG_MODULES > modules, jitalloc: prepare to allocate executable memory as ROX > x86/jitalloc: make memory allocated for code ROX > > Song Liu (2): > ftrace: Add swap_func to ftrace_process_locs() > x86/jitalloc: prepare to allocate exectuatble memory as ROX > > arch/Kconfig | 5 +- > arch/arm/kernel/module.c | 32 -- > arch/arm/mm/init.c | 35 ++ > arch/arm64/kernel/module.c | 47 > arch/arm64/mm/init.c | 42 +++ > arch/loongarch/kernel/module.c | 6 - > arch/loongarch/mm/init.c | 16 +++ > arch/mips/kernel/module.c| 9 -- > arch/mips/mm/init.c | 19 > arch/nios2/include/asm/pgtable.h | 5 +- > arch/nios2/kernel/module.c | 24 ++-- > arch/parisc/kernel/module.c | 11 -- > arch/parisc/mm/init.c| 21 +++- > arch/powerpc/kernel/kprobes.c| 4 +- > arch/powerpc/kernel/module.c | 37 --- > arch/powerpc/mm/mem.c| 41 +++ > arch/riscv/kernel/module.c | 10 -- > arch/riscv/mm/init.c | 18 +++ > arch/s390/kernel/ftrace.c| 4 +- > arch/s390/kernel/kprobes.c | 4 +- >
[PATCH 00/13] mm: jit/text allocator
From: "Mike Rapoport (IBM)" Hi, module_alloc() is used everywhere as a mean to allocate memory for code. Beside being semantically wrong, this unnecessarily ties all subsystmes that need to allocate code, such as ftrace, kprobes and BPF to modules and puts the burden of code allocation to the modules code. Several architectures override module_alloc() because of various constraints where the executable memory can be located and this causes additional obstacles for improvements of code allocation. This set splits code allocation from modules by introducing jit_text_alloc(), jit_data_alloc() and jit_free() APIs, replaces call sites of module_alloc() and module_memfree() with the new APIs and implements core text and related allocation in a central place. Instead of architecture specific overrides for module_alloc(), the architectures that require non-default behaviour for text allocation must fill jit_alloc_params structure and implement jit_alloc_arch_params() that returns a pointer to that structure. If an architecture does not implement jit_alloc_arch_params(), the defaults compatible with the current modules::module_alloc() are used. The new jitalloc infrastructure allows decoupling of kprobes and ftrace from modules, and most importantly it enables ROX allocations for executable memory. A centralized infrastructure for code allocation allows future optimizations for allocations of executable memory, caching large pages for better iTLB performance and providing sub-page allocations for users that only need small jit code snippets. patches 1-5: split out the code allocation from modules and arch patch 6: add dedicated API for data allocations with constraints similar to code allocations patches 7-9: decouple dynamic ftrace and kprobes form CONFIG_MODULES patches 10-13: enable ROX allocations for executable memory on x86 Mike Rapoport (IBM) (11): nios2: define virtual address space for modules mm: introduce jit_text_alloc() and use it instead of module_alloc() mm/jitalloc, arch: convert simple overrides of module_alloc to jitalloc mm/jitalloc, arch: convert remaining overrides of module_alloc to jitalloc module, jitalloc: drop module_alloc mm/jitalloc: introduce jit_data_alloc() x86/ftrace: enable dynamic ftrace without CONFIG_MODULES arch: make jitalloc setup available regardless of CONFIG_MODULES kprobes: remove dependcy on CONFIG_MODULES modules, jitalloc: prepare to allocate executable memory as ROX x86/jitalloc: make memory allocated for code ROX Song Liu (2): ftrace: Add swap_func to ftrace_process_locs() x86/jitalloc: prepare to allocate exectuatble memory as ROX arch/Kconfig | 5 +- arch/arm/kernel/module.c | 32 -- arch/arm/mm/init.c | 35 ++ arch/arm64/kernel/module.c | 47 arch/arm64/mm/init.c | 42 +++ arch/loongarch/kernel/module.c | 6 - arch/loongarch/mm/init.c | 16 +++ arch/mips/kernel/module.c| 9 -- arch/mips/mm/init.c | 19 arch/nios2/include/asm/pgtable.h | 5 +- arch/nios2/kernel/module.c | 24 ++-- arch/parisc/kernel/module.c | 11 -- arch/parisc/mm/init.c| 21 +++- arch/powerpc/kernel/kprobes.c| 4 +- arch/powerpc/kernel/module.c | 37 --- arch/powerpc/mm/mem.c| 41 +++ arch/riscv/kernel/module.c | 10 -- arch/riscv/mm/init.c | 18 +++ arch/s390/kernel/ftrace.c| 4 +- arch/s390/kernel/kprobes.c | 4 +- arch/s390/kernel/module.c| 46 +--- arch/s390/mm/init.c | 35 ++ arch/sparc/kernel/module.c | 34 +- arch/sparc/mm/Makefile | 2 + arch/sparc/mm/jitalloc.c | 21 arch/sparc/net/bpf_jit_comp_32.c | 8 +- arch/x86/Kconfig | 2 + arch/x86/kernel/alternative.c| 43 --- arch/x86/kernel/ftrace.c | 59 +- arch/x86/kernel/kprobes/core.c | 4 +- arch/x86/kernel/module.c | 75 + arch/x86/kernel/static_call.c| 10 +- arch/x86/kernel/unwind_orc.c | 13 ++- arch/x86/mm/init.c | 52 + arch/x86/net/bpf_jit_comp.c | 22 +++- include/linux/ftrace.h | 2 + include/linux/jitalloc.h | 69 include/linux/moduleloader.h | 15 --- kernel/bpf/core.c| 14 +-- kernel/kprobes.c | 51 + kernel/module/Kconfig| 1 + kernel/module/main.c | 56 -- kernel/trace/ftrace.c| 13 ++- kernel/trace/trace_kprobe.c | 11 ++ mm/Kconfig | 3 + mm/Makefile | 1 + mm/jitalloc.c| 185 +++ mm/mm_init.c | 2 + 48 files changed, 777 insertions(+), 462 deletions(-) create mode 100644 arch/sparc/mm/jitalloc.c create mode 100644