On Sat, 2020-11-21 at 20:10 -0800, Andy Lutomirski wrote: > On Fri, Nov 20, 2020 at 12:30 PM Rick Edgecombe > <rick.p.edgecombe@xxxxxxxxx> wrote: > > In order to allow for future arch specific optimizations for > > vmalloc > > permissions, first add an implementation of a new interface that > > will > > work cross arch by using the existing set_memory_() functions. > > > > When allocating some memory that will be RO, for example it should > > be used > > like: > > > > /* Reserve va */ > > struct perm_allocation *alloc = perm_alloc(vstart, vend, page_cnt, > > PERM_R); > > I'm sure I could reverse-engineer this from the code, but: > > Where do vstart and vend come from? They are the start and end virtual address range to try to allocate in, like __vmalloc_node_range() has. So like, MODULES_VADDR and MODULES_END. Sorry for the terse example. The header in this patch has some comments about each of the new functions to supplement it a bit. > Does perm_alloc() allocate memory or just virtual addresses? Is the > caller expected to call vmalloc()? The caller does not need to call vmalloc(). perm_alloc() behaves similar to __vmalloc_node_range(), where it allocates both memory and virtual addresses. I left a little wiggle room in the descriptions in the header, that the virtual address range doesn't actually need to be mapped until after perm_writable_finish(). But both of the implementations in this series will map it right away like a vmalloc(). So the interface could actually pretty easily be changed to look like another flavor of vmalloc() that just returns a pointer to allocation. The reason why it returns this new struct instead is that, unlike most vmalloc()'s, the callers will be looking up metadata about the allocation a bunch of times (the writable address). Having this metadata stored in some struct inside vmalloc would mean perm_writable_addr() would have to do something like find_vmap_area() every time in order to find the writable allocation address from the allocations address passed in. So returning a struct makes it so the writable translation can skip a global lock and lookup. Another option could be putting the new metadata in vm_struct and just return that, like get_vm_area(). Then we don't need to invent a new struct. But then normal vmalloc()'s would have a bit of wasted memory since they don't need this metadata. A nice thing about that though, is there would be a central place to translate to the writable addresses in cases where only a virtual address is available. In later patches here, a similar lookup happens anyway for modules using __module_address() to get the writable address. This is due to some existing code where plumbing the new struct all the way through would have resulted in too many changes. I'm not sure which is best. > How does one free this thing? void perm_free(struct perm_allocation *alloc);
