Thread (14 messages) 14 messages, 4 authors, 2021-08-27

Re: [PATCH v3 1/3] vmalloc: Choose a better start address in vm_area_register_early()

From: Kefeng Wang <hidden>
Date: 2021-08-27 08:36:33
Also in: linux-mm, lkml

On 2021/8/26 1:59, Catalin Marinas wrote:
On Mon, Aug 09, 2021 at 05:37:48PM +0800, Kefeng Wang wrote:
quoted
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d5cd52805149..1e8fe08725b8 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2238,11 +2238,17 @@ void __init vm_area_add_early(struct vm_struct *vm)
   */
  void __init vm_area_register_early(struct vm_struct *vm, size_t align)
  {
-	static size_t vm_init_off __initdata;
-	unsigned long addr;
-
-	addr = ALIGN(VMALLOC_START + vm_init_off, align);
-	vm_init_off = PFN_ALIGN(addr + vm->size) - VMALLOC_START;
+	struct vm_struct *head = vmlist, *curr, *next;
+	unsigned long addr = ALIGN(VMALLOC_START, align);
+
+	while (head != NULL) {
Nitpick: I'd use the same pattern as in vm_area_add_early(), i.e. a
'for' loop. You might as well insert it directly than calling the add
function and going through the loop again. Not a strong preference
either way.
quoted
+		next = head->next;
+		curr = head;
+		head = next;
+		addr = ALIGN((unsigned long)curr->addr + curr->size, align);
+		if (next && (unsigned long)next->addr - addr > vm->size)
Is greater or equal sufficient?
quoted
+			break;
+	}
  
  	vm->addr = (void *)addr;
Another nitpick: it's very unlikely on a 64-bit architecture but not
impossible on 32-bit to hit VMALLOC_END here. Maybe some BUG_ON.
Hi Catalin, thank for your review, I will update in the next version,

Could you take a look the following change, is it OK?

void __init vm_area_register_early(struct vm_struct *vm, size_t align)

{

          struct vm_struct *next, *cur, **p;
          unsigned long addr = ALIGN(VMALLOC_START, align);
BUG_ON(vmap_initialized);

          for (p = &vmlist; (cur = *p) != NULL, next = cur->next; p = 
&next) {
                  addr = ALIGN((unsigned long)cur->addr + cur->size, 
align);
                  if (next && (unsigned long)next->addr - addr >= 
vm->size) {
                          p = &next;
break;
}
}

          BUG_ON(addr > VMALLOC_END - vm->size);
          vm->addr = (void *)addr;
          vm->next = *p;
          *p = vm;
}

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help