Fix alignment issues in vm_reserv_reclaim_contig

[dougm]

Summary

Fix misuse of alignment and boundary in vm_reserv_reclaim_contig.

Change the computations with alignment and boundary in vm_reserv_test_contig, since the author of the current computations no longer thinks that they are correct. Add assertions to test the effect of alignment and boundary in vm_reserv_test_contig. Add assertions to vm_reserv_test_contig to verify the conditions established by vm_reserv_reclaim_contig.

Fail vm_reserv_reclaim_contig immediately if size > boundary, since in that case, jumping the range start to the next multiple of boundary cannot satisfy the boundary condition; indeed, nothing can satisfy that condition.

Not for final check-in, add support for a sysctl to test vm_reserv_reclaim_contig, to see if the new assertions hold.
Currently:

1. sysctl debug.vm_reserv_reclaim_contig=1

debug.vm_reserv_reclaim_contig: 0
cycles/break: 85
-> 0

[dougm]

Don't bother checking a reservation if every address within it violates alignment.

Consequently, the page update to achieve alignment for a gap in a reservation can't jump to a page way way outside the reservation, but only as far as one past the last reservation.

[dougm]

Don't allow boundary to exceed reservation size, so that boundary arithmetic cannot be a problem.

[dougm]

Use roundup2() to hide the bit-twiddling in alignment and boundary adjustment. Work with the page number directly instead of going back to the address for calculation.

[dougm]

Make all the arithmetic in the helper function use ints, and let the caller handle conversion to int. Let the caller manage bounds calculation. Return the found page and let the caller check assertions to verify alignment.

[alc]

sys/vm/vm_reserv.c

1247–1248

Don't use __FUNCTION__. It was a gcc extension. Use __func__. It is standard C.

1325–1326

Unless this is needed for correctness, which I don't think it is, I don't see the point of this check. It amounts to an optimization for the unexpected case.

1327–1328

This is incorrect. The units for boundary are bytes, not pages.

[dougm]

Shrink whitespace. Fix pages vs bytes error. Stop using FUNCTION.

sys/vm/vm_reserv.c

1325–1326

In the patch at present, when we find that the prospective range spans a multiple of boundary, we move the range start to the next multiple of boundary, and assume that the boundary constraint is satisfied. However, if the size is bigger than boundary, the boundary constraint cannot be satisfied and the code may return true anyway.

So I can ignore size>boundary and just be wrong in that case, or assert size<= boundary, or keep the code as is.

[dougm]

Rename a couple of parameters to vm_reserv_find_contig. Add assertions and comments to ensure/explain that arithmetic with bound and alignment cannot compute values beyond the number of pages in a reservation.

[alc]

sys/vm/vm_reserv.c

1366

Remove + PAGE_MASK. You want to round down, not up, here. Otherwise, the allocated memory could go beyond high.

1374

Deindent this.

1377

Deindent this.

[dougm]

Accept 3 reviewer suggestions.

[alc]

sys/vm/vm_reserv.c

1375

Drop the outermost ()'s.

[dougm]

Apply reviewer suggestion. Remove some upper bound checks on alignment and boundary. Add some zero and power-of-two kasserts on alignment and boundary.

[dougm]

Correct identifier name. Verify compilation.

[alc]

sys/vm/vm_reserv.c

1253–1256

alignment and boundary are frequently zero.

[dougm]

Compute the shifted alignment and boundary values once, not with every find function call, and make sure they aren't zero-valued.

[alc]

In the commit message, I would somehow explain that the consequence of the bug was only that the wrong reservations might be broken, not that unaligned or inappropriately boundary crossing memory would be returned by contigmalloc(), et al.

sys/vm/vm_reserv.c

1276–1277

What happens if ppn_align is 2^31, and, thus, a negative value since lo is a signed int? To be clear, we will probably never see such a value passed in our lifetimes, but a static analyzer might still flag this.

[dougm]

Drop debug code. Push ppn_align and ppn_bound into [1,VM_LEVEL_0_NPAGES] to avoid overflow problems.

[dougm]

sys/vm/vm_reserv.c

1276–1277

I have made a change to ensure that ppn_align and ppn_bound are in [1, VM_LEVEL_0_NPAGES].

[markj]

sys/vm/vm_reserv.c

1330

Extra whitespace after >>.

[dougm]

sys/vm/vm_reserv.c

1331

alc (vocally):
use MIN, MAX macros here since sizeof(boundary) > sizeof(u_long) for some architectures.

[dougm]

Change stray tab to space.
Avoid ulmin, ulmax for vm_paddr_t boundary.