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

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