On Sat, May 27, 2023 at 10:58:37PM -0700, Song Liu wrote: > I don't think we are exposing architecture specific options to users. > Some layer need to handle arch specifics. If the new allocator is > built on top of module_alloc, module_alloc is handling that. If the new > allocator is to replace module_alloc, it needs to handle arch specifics. Ok, I went back and read more thoroughly, I got this part wrong. The actual interface is the mod_mem_type enum, not mod_alloc_params or vmalloc_params. So this was my main complaint, but this actually looks ok now. It would be better to have those structs in a .c file, not the header file - it looks like those are the public interface the way you have it. > > The memory protection interface also needs to go, we've got a better > > interface to model after (text_poke(), although that code needs work > > too!). And the instruction fill features need a thorough justification > > if they're to be included. > > I guess the first step to use text_poke() is to make it available on all > archs? That doesn't seem easy to me. We just need a helper that either calls text_poke() or does the page permission dance in a single place. If we do the same thing for other mod_mem_types, we could potentially allow them to be shared on hugepages too.