On 11/14/2013 9:26 AM, Dave Hansen wrote:
On 11/11/2013 03:26 PM, Laura Abbott wrote:With CONFIG_ENABLE_VMALLOC_SAVINGS, all lowmem is tracked in vmalloc. This means that all the kernel virtual address space can be treated as part of the vmalloc region. Allow vm areas to be allocated from the full kernel address range. Signed-off-by: Laura Abbott <lauraa@xxxxxxxxxxxxxx> Signed-off-by: Neeti Desai <neetid@xxxxxxxxxxxxxx> --- mm/vmalloc.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/mm/vmalloc.c b/mm/vmalloc.c index c7b138b..181247d 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -1385,16 +1385,27 @@ struct vm_struct *__get_vm_area_caller(unsigned long size, unsigned long flags, */ struct vm_struct *get_vm_area(unsigned long size, unsigned long flags) { +#ifdef CONFIG_ENABLE_VMALLOC_SAVING + return __get_vm_area_node(size, 1, flags, PAGE_OFFSET, VMALLOC_END, + NUMA_NO_NODE, GFP_KERNEL, + __builtin_return_address(0)); +#else return __get_vm_area_node(size, 1, flags, VMALLOC_START, VMALLOC_END, NUMA_NO_NODE, GFP_KERNEL, __builtin_return_address(0)); +#endif } struct vm_struct *get_vm_area_caller(unsigned long size, unsigned long flags, const void *caller) { +#ifdef CONFIG_ENABLE_VMALLOC_SAVING + return __get_vm_area_node(size, 1, flags, PAGE_OFFSET, VMALLOC_END, + NUMA_NO_NODE, GFP_KERNEL, caller); +#else return __get_vm_area_node(size, 1, flags, VMALLOC_START, VMALLOC_END, NUMA_NO_NODE, GFP_KERNEL, caller); +#endif }Couple of nits: first of all, there's no reason to copy, paste, and #ifdef this much code. This just invites one of the copies to bitrot. I'd much rather see this: #ifdef CONFIG_ENABLE_VMALLOC_SAVING #define LOWEST_VMALLOC_VADDR PAGE_OFFSET #else #define LOWEST_VMALLOC_VADDR VMALLOC_START #endif Then just replace the PAGE_OFFSET in the function arguments with LOWEST_VMALLOC_VADDR.
Good point.
Have you done any audits to make sure that the rest of the code that deals with vmalloc addresses in the kernel is using is_vmalloc_addr()? I'd be a bit worried that we might have picked up an assumption or two that *all* vmalloc addresses are _above_ VMALLOC_START. The percpu.c code looks like it might do this, and maybe the kcore code. The vmalloc.c code itself has this in get_vmalloc_info():/* * Some archs keep another range for modules in vmalloc space */ if (addr < VMALLOC_START) continue;Seems like that would break as well. With this patch, VMALLOC_START loses enough of its meaning that I wonder if we should even keep it around. It's the start of the _dedicated_ vmalloc space, but it's mostly useless and obscure enough that maybe we should get rid of its use in common code.
Yes, there are plenty of clients who are using VMALLOC_START. There might still be a use for VMALLOC_START as marking 'no more direct mapped memory above this address' . To start making some if the cleanup easier, it would help to have an already calculated total amount of vmalloc for the clients who are trying to work off a vmalloc percentage.
Thanks, Laura -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>