On Sat, May 27, 2023 at 12:04 AM Kent Overstreet <kent.overstreet@xxxxxxxxx> wrote: > > On Thu, May 25, 2023 at 10:15:26PM -0700, Song Liu wrote: > > This set implements the second part of module type aware allocator > > (module_alloc_type), which was discussed in [1]. This part contains the > > interface of the new allocator, as well as changes in x86 code to use the > > new allocator (modules, BPF, ftrace, kprobe). > > > > The set does not contain a binpack allocator to enable sharing huge pages > > among different allocations. But this set defines the interface used by > > the binpack allocator. [2] has some discussion on different options of the > > binpack allocator. > > I'm afraid the more I look at this patchset the more it appears to be > complete nonsense. I don't think it is nonsense, as you clearly got most of the points here. :) > > The exposed interface appears to be both for specifying architecture > dependent options (which should be hidden inside the allocator > internals!) _and_ a general purpose interface for module/bpf/kprobes - > but it's not very clear, and the rational appears to be completely > missing. The rationale is to have something to replace module_alloc(). Therefore, it needs to handle architecture specific requirements, and provide interface to all current users of module_alloc(). It may look a little weird at the moment, because the actual allocator logic is very thin. But that's where we will plug in the next step of the work. > > I think this needs to back to the drawing board and we need something > simpler just targeted at executable memory; architecture specific > options should definitely _not_ be part of the exposed interface. 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. > > 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. Thanks, Song
