imgact_elf: Optimize pagesizes[] loop

[alc]

Summary

Except for elements whose value is zero, the elements of pagesizes[] are always sorted in increasing order, so once a loop starting from the end of the array has found a non-zero element, it has found the largest valued element and can stop iterating.

[alc]

As I wrote this patch, I thought about adding a sanity check on pagesizes[] to the machine-independent layer. Thoughts on this? pmap_init() happens fairly late with only vm_pager_init() coming after it in vm_mem_init(), and that does not seem like a natural place for it.

[kib]

IMO it could be a function like vm_pagesizes_check() called directly from pmap_init()s?

[alc]

In D46215#1053727, @kib wrote:

IMO it could be a function like vm_pagesizes_check() called directly from pmap_init()s?

diff --git a/sys/vm/vm_init.c b/sys/vm/vm_init.c
index 0fd13f73a180..4783a7deaf96 100644
--- a/sys/vm/vm_init.c
+++ b/sys/vm/vm_init.c
@@ -100,6 +100,23 @@ long physmem;
 static void vm_mem_init(void *);
 SYSINIT(vm_mem, SI_SUB_VM, SI_ORDER_FIRST, vm_mem_init, NULL);
 
+#ifdef INVARIANTS
+/*
+ * Ensure that pmap_init() correctly initialized pagesizes[].
+ */
+static void
+vm_check_pagesizes(void)
+{
+       int i;
+
+       KASSERT(pagesizes[0] == PAGE_SIZE, ("pagesizes[0] != PAGE_SIZE"));
+       for (i = 1; i < MAXPAGESIZES; i++) {
+               KASSERT(pagesizes[i - 1] < pagesizes[i] || pagesizes[i] == 0,
+                   ("pagesizes[%d] >= pagesizes[%d]", i - 1, i));
+       }
+}
+#endif
+
 /*
  *     vm_mem_init() initializes the virtual memory system.
  *     This is done only by the first cpu up.
@@ -140,6 +157,10 @@ vm_mem_init(void *dummy)
        kmem_init_zero_region();
        pmap_init();
        vm_pager_init();
+
+#ifdef INVARIANTS
+       vm_check_pagesizes();
+#endif
 }
 
 void

If I'm being paranoid, then I don't want to rely on the pmap calling the check function. Do we want this check? I'm on the fence. @markj

[markj]

In D46215#1053781, @alc wrote:

In D46215#1053727, @kib wrote:

IMO it could be a function like vm_pagesizes_check() called directly from pmap_init()s?

diff --git a/sys/vm/vm_init.c b/sys/vm/vm_init.c
index 0fd13f73a180..4783a7deaf96 100644
--- a/sys/vm/vm_init.c
+++ b/sys/vm/vm_init.c
@@ -100,6 +100,23 @@ long physmem;
 static void vm_mem_init(void *);
 SYSINIT(vm_mem, SI_SUB_VM, SI_ORDER_FIRST, vm_mem_init, NULL);
 
+#ifdef INVARIANTS
+/*
+ * Ensure that pmap_init() correctly initialized pagesizes[].
+ */
+static void
+vm_check_pagesizes(void)
+{
+       int i;
+
+       KASSERT(pagesizes[0] == PAGE_SIZE, ("pagesizes[0] != PAGE_SIZE"));
+       for (i = 1; i < MAXPAGESIZES; i++) {
+               KASSERT(pagesizes[i - 1] < pagesizes[i] || pagesizes[i] == 0,
+                   ("pagesizes[%d] >= pagesizes[%d]", i - 1, i));
+       }
+}
+#endif
+
 /*
  *     vm_mem_init() initializes the virtual memory system.
  *     This is done only by the first cpu up.
@@ -140,6 +157,10 @@ vm_mem_init(void *dummy)
        kmem_init_zero_region();
        pmap_init();
        vm_pager_init();
+
+#ifdef INVARIANTS
+       vm_check_pagesizes();
+#endif
 }
 
 void

If I'm being paranoid, then I don't want to rely on the pmap calling the check function. Do we want this check? I'm on the fence. @markj

I don't feel strongly either. I agree that relying on each pmap to check is a bit unsatisfying.

I can't think of any bugs the check would have caught, but on the other hand it's inobtrusive and might save someone some time someday, or help catch some boot-time memory corruption. I'm for it.

[kib]

IMO the most value of the check is that it documents our assumptions about pagesizes[]. This improves understanding of other places, and eases potential addition of new arches.