[haiku-commits] Re: haiku: hrev44773 - src/system/kernel/vm

  • From: "Ingo Weinhold" <ingo_weinhold@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 06 Nov 2012 12:32:37 +0100

korli@xxxxxxxxxxxxxxxx wrote:
> diff --git a/src/system/kernel/vm/vm_page.cpp 
> b/src/system/kernel/vm/vm_page.cpp
> index 9e4c591..29df4e1 100644
> --- a/src/system/kernel/vm/vm_page.cpp
> +++ b/src/system/kernel/vm/vm_page.cpp
> @@ -3862,13 +3862,13 @@ vm_page_allocate_page_run(uint32 flags, page_num_t 
> length,
> Â page_num_t offsetStart = start + sPhysicalPageOffset;
> Â
> Â // enforce alignment
> - if ((offsetStart & alignmentMask) != 0) {
> + if (alignmentMask != 0 && (offsetStart & alignmentMask) != 0) {

The additional check is superfluous, since the & would yield 0 in this case 
anyway.

> Â offsetStart = ((offsetStart + alignmentMask) & ~alignmentMask)
> Â - sPhysicalPageOffset;
> Â }
> Â
> Â // enforce boundary
> - if (offsetStart << boundaryShift
> + if (boundaryShift != 0 && offsetStart << boundaryShift
> Â != (offsetStart + length - 1) << boundaryShift) {
> Â offsetStart = (offsetStart + length - 1) << boundaryShift
> Â >> boundaryShift;

Here the check is necessary indeed. On the other hand all the shifts are the 
wrong way around. I wonder why I didn't use a mask for the boundary, anyway. 
The checks would be similiar, but a mask doesn't need a loop to be computed.

> @@ -3887,7 +3887,10 @@ vm_page_allocate_page_run(uint32 flags, page_num_t 
> length,
> Â }
> Â
> Â dprintf("vm_page_allocate_page_run(): Failed to allocate run of "
> - "length %" B_PRIuPHYSADDR " in second iteration!", length);
> + "length %" B_PRIuPHYSADDR " (%" B_PRIuPHYSADDR " %"
> + B_PRIuPHYSADDR ") in second iteration (align: %" B_PRIuPHYSADDR
> + " boundary: %" B_PRIuPHYSADDR ") !", length, requestedStart,
> + end, restrictions->alignment, restrictions->boundary);
> Â
> Â freeClearQueueLocker.Unlock();
> Â vm_page_unreserve_pages(&reservation);
> @@ -3916,7 +3919,7 @@ vm_page_allocate_page_run(uint32 flags, page_num_t 
> length,
> Â freeClearQueueLocker.Lock();
> Â }
> Â
> - start += i + 1;
> + start += max_c(i, alignmentMask) + 1;

Superfluous. At the beginning of the next iteration start is aligned anyway.

CU, Ingo

Other related posts: