Extra align in malloc_mgt_init?

[James Cook]

Summary: I think line 294 in kern_malloc.c can be deleted, and a tiny
bit of memory might be saved. L294 is the first of two lines that look
like:

	offset = __VM_CACHELINE_ALIGN(offset);

I did a quick test to confirm it's okay to remove.

If you buy that, you can skip the rest of this email!


****

In more detail:

I can't see why line 294 is there. There's already code later in the
function to deal with the alignment issue, and it looks correct on its
own.

To test my theory, I deleted the line and added a bunch of kprintfs.
Result: no failed KKASSERTs, no crashes, and sometimes a malloc_slab
can store one more object than before.



****

In even more detail:

I booted a kernel with the following patch, which deletes line 294 and
adds a bunch of printfs. (The new function malloc_mgt_old_count is just
a copy of the old calculation, so I can show a comparison with
kprintf.)

Output of "dmesg|grep XXX" at bottom.

+static size_t malloc_mgt_old_count(size_t size)
+{
+	size_t offset;
+	size_t count;
+
+	/*
+	 * Figure out the count by taking into account the size of the fobjs[]
+	 * array by adding it to the object size.
+	 */
+	offset = offsetof(struct kmalloc_slab, fobjs[0]);
+	offset = __VM_CACHELINE_ALIGN(offset);
+	count = (KMALLOC_SLAB_SIZE - offset) / (size + sizeof(void *));
+
+	/*
+	 * However, the fobj[] array itself must be aligned, so we might
+	 * have to reduce the count by 1.  (We can do this becaues 'size'
+	 * is already aligned as well).
+	 */
+	offset = offsetof(struct kmalloc_slab, fobjs[count]);
+	offset = __VM_CACHELINE_ALIGN(offset);
+
+	if (offset + size * count > KMALLOC_SLAB_SIZE) {
+		--count;
+		offset = offsetof(struct kmalloc_slab, fobjs[count]);
+		offset = __VM_CACHELINE_ALIGN(offset);
+		KKASSERT (offset + size * count <= KMALLOC_SLAB_SIZE);
+	}
+
+	return count;
+}
+
 void
 malloc_mgt_init(struct malloc_type *type __unused,
 		struct kmalloc_mgt *mgt, size_t size)
 {
 	size_t offset;
-	size_t count;
+	size_t count, old_count;
 
 	bzero(mgt, sizeof(*mgt));
 	spin_init(&mgt->spin, "kmmgt");
@@ -291,7 +322,7 @@ malloc_mgt_init(struct malloc_type *type __unused,
 	 * array by adding it to the object size.
 	 */
 	offset = offsetof(struct kmalloc_slab, fobjs[0]);
-	offset = __VM_CACHELINE_ALIGN(offset);
+	kprintf("XXX malloc_mgt_init fobjs[0] offset %jx\n", (uintmax_t)offset);
 	count = (KMALLOC_SLAB_SIZE - offset) / (size + sizeof(void *));
 
 	/*
@@ -309,6 +340,13 @@ malloc_mgt_init(struct malloc_type *type __unused,
 		KKASSERT (offset + size * count <= KMALLOC_SLAB_SIZE);
 	}
 
+	old_count = malloc_mgt_old_count(size);
+        kprintf("XXX malloc_mgt_init count before %jx now %jx\n", old_count, count);
+	if (old_count != count) {
+	  kprintf("XXX malloc_mgt_init count changed! offset + size * count = %jx\n",
+		  (uintmax_t)(offset + size * count));
+	}
+
 	mgt->slab_offset = offset;
 	mgt->slab_count	 = count;
 }



Output of dmesg|grep xxx; note all numbers are hex.

XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 14d now 14d
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 14d now 14d
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 1ef now 1ef
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 1ef now 1ef
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 1ef now 1ef
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 1ef now 1ef
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 14d now 14d
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 14d now 14d
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 11e now 11e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before a8 now a8
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 18e now 18e
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000
XXX malloc_mgt_init fobjs[0] offset 160
XXX malloc_mgt_init count before 3c0 now 3c1
XXX malloc_mgt_init count changed! offset + size * count = 20000


-- 
James