vm_phys_init: Quiet unused but set warnings about npages.

[jhb]

Summary

npages is used in two optional cases:

• to conditionally create a separate DMA32 free list

• to index vm_page_array for VM_PHYSSEG_SPARSE

Add in more #ifdef's around npages statements.

[markj]

I really dislike all the ifdefs and would be tempted to define VM_DMA32_NPAGES_THRESHOLD to 0 if it's not defined. (And currently it's defined iff VM_FREELIST_DMA32 is defined anyway.) But this is ok.

[jhb]

In D34887#790697, @markj wrote:

I really dislike all the ifdefs and would be tempted to define VM_DMA32_NPAGES_THRESHOLD to 0 if it's not defined. (And currently it's defined iff VM_FREELIST_DMA32 is defined anyway.) But this is ok.

I would be fine with defaulting VM_DMA32_NPAGES_THRESHOLD to 0. I think npages declaration is still stuck with being under #ifdef's though as the read is only under #ifdef VM_FREELIST_DMA32 so you'd need all the new #ifdef's just with that name instead. (Right now I'm assuming that VM_DMA32_NPAGES_THRESHOLD basically implies VM_FREELIST_DMA32.)

[jhb]

sys/vm/vm_phys.c

515

The code here also seems wrong. The first segment below VM_DMA32_BOUNDARY will see a npages of 0 and will fall through to the else case and count those pages as "> 4G" pages even though they aren't. I feel like the logic you kind of want is more like:

bool seen_dma32;
...
seen_dma32 = false;
...
#ifdef VM_FREELIST_DMA32
       if (seg->end <= VM_DMA32_BOUNDARY) {
           seen_dma32 = true;
#ifdef VM_DMA32_NPAGES_THRESHOLD
           if n(pages > VM_DMA32_NPAGES_THRESHOLD)
#endif
               vm_freelist_to_flind[VM_FREELIST_DMA32] = 1;
       else {
#ifdef VM_DMA32_NPAGES_THRESHOLD
           npages += atop(seg->end - seg->start);
#endif
#else
       {
#endif
           vm_freelist_to_flind[VM_FREELIST_DEFAULT] = 1;
       }

Default VM_DMA32_NPAGES_THRESHOLD to 0 would then just leave the VM_FREELIST_DMA32 #ifdef which is slightly less terrible?

[markj]

In D34887#790746, @jhb wrote:

In D34887#790697, @markj wrote:

I really dislike all the ifdefs and would be tempted to define VM_DMA32_NPAGES_THRESHOLD to 0 if it's not defined. (And currently it's defined iff VM_FREELIST_DMA32 is defined anyway.) But this is ok.

I would be fine with defaulting VM_DMA32_NPAGES_THRESHOLD to 0. I think npages declaration is still stuck with being under #ifdef's though as the read is only under #ifdef VM_FREELIST_DMA32 so you'd need all the new #ifdef's just with that name instead. (Right now I'm assuming that VM_DMA32_NPAGES_THRESHOLD basically implies VM_FREELIST_DMA32.)

Yeah, adding a default value for this threshold would just let us remove some of the existing ifdefs.

sys/vm/vm_phys.c

515

Note that the loop steps over the segs in reverse order, i.e., starting from the highest PAs. I agree that the comment still isn't quite correct, though. We are indeed counting pages below 4G towards the threshold. I don't think it matters very much and would be inclined to fix the comment instead.

[jhb]

sys/vm/vm_phys.c

515

Ok. I'd be happy to have you fix the comment. :)