As long as the page daemon will still launder the pages when we are short of memory, I don't see a problem with msync() ignoring them.

The next to last change to this patch that zeroed the bitmap in terminator nodes addressed my last concern. I'm going to commit this patch shortly.

I've performed some testing with the new sysctl that calls blist_stats(). Specifically, I've configured a test machine with 2GB of RAM and 64GB of swap space, and run a "make -j7 buildworld" in a loop. After 6 consecutive builds, the before and after results are as follows.

In D12281#255294, @mjg wrote:

-       u_int v_free_count;     /* (f) pages free */
        u_int v_inactive_target; /* (c) pages desired inactive */
        u_int v_pageout_free_min;   /* (c) min pages reserved for kernel */
        u_int v_interrupt_free_min; /* (c) reserved pages for int code */
        u_int v_free_severe;    /* (c) severe page depletion point */
+       u_int v_free_count VMMETER_ALIGNED;     /* (f) pages free */
        u_int v_wire_count VMMETER_ALIGNED; /* (a) pages wired down */

Made no difference in the buildkernel test, even with kib's page batching patch. Something of the sort will definitely have to be done later anyway and probably helps a little elsewhere, so I can include this bit in the patch if you want.

Have you evaluated the effects of isolating the free count? The fields surrounding it are read only.

swap_pager.c needs #include <sys/sbuf.h>

In D12134#251911, @kib wrote:

In D12134#251899, @alc wrote:

This change reveals a problem: vm_fault_soft_fast() is (indirectly) calling vm_pager_page_unswapped() with only a read lock.

My understanding is that this is a valid assert in the following situation: we have a swap object with a valid page which was swapped in and not dirtied. Then, on a write fault, any (fast or normal) fault handlers could free the swap space. It is possible because swap pager page-in leaves the backed swap space intact as (IMO very small) optimization.

Update vm_fault_dirty().

This change reveals a problem: vm_fault_soft_fast() is (indirectly) calling vm_pager_page_unswapped() with only a read lock.

In D12106#251258, @dougm_rice.edu wrote:

A sentinel is not sufficient. While cursor sits at the end of managed memory, we start with scan = bl_root, and compute blk == cursor, so child == 0, so we loop over the 16 subchildren of root and if we find a free block at p, we report a free block at p+radix. Nothing gets us to actually look at the new sentinel, unless we make the tree one level higher, with the 2nd child of the root consisting of the sentinel only.

I've now completed 8+ hours of testing under a "make -j7 buildworld" workload on 1.5GB of RAM swapping to a Samsung 850 PRO, both with and without the patch. I think that there is too much variability in the execution time conclude anything about the execution time. However, here is the memory utilization story:

In D11435#251430, @kib wrote:

In D11435#251395, @alc wrote:

I've tried to create a test where the vm object has discontiguous swap space usage and the radix tree should be advantageous. In particular, I've tried to create a situation where vm object destruction should be faster, but I'm getting inconsistent results.

In regards to memory utilization, I've looked at a best-case scenario for the old code, where you have contiguous swap space usage. (To be clear, I'm talking about contiguous page index ranges within the vm object being swapped out, not contiguous on-disk storage.) My scenario was using about 48GB of swap space, and the total kernel memory used by the new radix trie code was slightly less than the old code if you account for the size of the swhash array.

Isn't the main advantage of the new code is the fact that the per-object tracking of the pindex->swap block relation is protected by the vm object lock instead of the global hash lock. If the memory usage by the new code is approximately the same, as expected, then this reason alone should be significant.

I've tried to create a test where the vm object has discontiguous swap space usage and the radix tree should be advantageous. In particular, I've tried to create a situation where vm object destruction should be faster, but I'm getting inconsistent results.

I have run numerous tests since r322459 was committed that have wrapped around the swap area without crashing, so there must be another prerequisite to a crash: Your swap area is a fully allocated tree, i.e., the number of blocks equals the radix. Can we also solve this problem by placing a sentinel entry at the end of a fully allocated tree?

Just an FYI, I'm doing some performance testing while sipping my morning coffee.

In D11435#249802, @kib wrote:

In D11435#249799, @alc wrote:

By the way, once upon a time, you asked me about whether we should continue using the blist allocator for swap space or switch to vmem. Perhaps the most compelling reason, which I failed to give at the time, is that every allocated range from an arena consumes a boundary tag. In other words, while vmem coalesces free ranges, it maintains a boundary tag for each allocated range.

Ok.

Also, from a functionality standpoint, with vmem, you have to free an entire allocation at once. In order to support the pager's unswapped function, we would have to allocate swap space one page at a time; so in fact we would wind up with a boundary tag in use for every page of allocated swap space.

Or enhance vmem to allow partial free, with a specialized interface. I think that this could be done in a way similar to the stack gap handling for edges, and new bt allocations if in middle.
I do not propose to do this, just discussing a hypothetical possibility.

By the way, once upon a time, you asked me about whether we should continue using the blist allocator for swap space or switch to vmem. Perhaps the most compelling reason, which I failed to give at the time, is that every allocated range from an arena consumes a boundary tag. In other words, while vmem coalesces free ranges, it maintains a boundary tag for each allocated range.

In D11688#249553, @br wrote:

In D11688#249549, @alc wrote:

I have no objections to this change. I'm just interested in hearing a little more about why it is needed. In your SGX driver are you essentially maintaining a private free page list for managing the pool of physical memory that backs enclaves?

Exactly. We have enclave's page cache (EPC) memory pool reserved by processor and we register it as fictitious range of physical memory. That EPC pool is encrypted and backs enclave's control information, enclave's code, stack, data, etc.
https://reviews.freebsd.org/D11113

I have no objections to this change. I'm just interested in hearing a little more about why it is needed. In your SGX driver are you essentially maintaining a private free page list for managing the pool of physical memory that backs enclaves?

I'm happy with this.

In D11784#248990, @jeff wrote:

Three broad comments;

We don't necessarily know which allocations will be handed off to do DMA on. This may be safe as busdma becomes more sophisticated but in the past network drivers definitely assumed mbufs were contiguous, for example. There may still be advantages to allocating aligned kva or possibly just contig physical addresses and using the direct map. You could in principle still use a constant offset from the aligned boundary to find the slab. You would also produce shorter scatter/gather descriptors for those zones that are involved in dma. Eliminating vtoslab may not be that important with the per-cpu caches in play but we do need to consider the implications of contiguous virtual addresses not being physically contiguous.

I've asked Doug to create some new statistics code (see D11906) so that we can quantify the effects of this patch.

This change reduces the text size by 7% on amd64.

I suggest changing the "radix" field of struct blist to be unsigned as well.

This change doesn't compile:

../../../kern/subr_blist.c:457:46: error: too few arguments to function call,
      expected 4, have 3
                return (blst_leaf_alloc(scan, cursor, count));
                        ~~~~~~~~~~~~~~~                    ^
../../../kern/subr_blist.c:358:1: note: 'blst_leaf_alloc' declared here
static daddr_t
^
1 error generated.

I think that the callers should be rewritten to use vm_radix_lookup_le(), e.g.,

Brett, I added you to this change, because it will decrease the time spent in vm_radix_lookup() by your shm_open()/sendfile() test case.

To be clear, I'm happy with the concept, and in general eliminating vm_page_lookup() calls inside of loops.

Update the description of VM_ALLOC_NOBUSY.

Should vm_page_grab_pages() also include this assertion from vm_page_grab():

KASSERT((allocflags & VM_ALLOC_SBUSY) == 0 ||
    (allocflags & VM_ALLOC_IGN_SBUSY) != 0,
    ("vm_page_grab: VM_ALLOC_SBUSY/VM_ALLOC_IGN_SBUSY mismatch"));