More Mark notes handled.

In D11435#241087, @kib wrote:

Optimize swap_pager_find_least().

I think this is not quite what Alan suggested - with this change, we're doing an unnecessary lookup when no swap block with rounddown(pindex, SWAP_META_PAGES) <= sb->p < roundup(pindex, SWAP_META_PAGES) is present in the trie.

I did some digging in the swap pager's history. swap_pager_isswapped() was introduced to implement "swapoff". Specifically, to determine whether the U area for a process needed to be swapped in. As such, it probably should have been deleted when most U area code was removed in r137910. I think that it would fine to commit a change to HEAD eliminating it. It's been dead code for a very long time.

Kostik, I suggest committing the remaining pctrie changes.

Yes. However, there is a call to uma_zone_get_max() where we add swap space that you might also need to deal with, because I think that uma_zone_get_max() will return non-sense without the reservation or a call to uma_zone_set_max().

In D11435#243657, @alc wrote:

Yes. However, there is a call to uma_zone_get_max() where we add swap space that you might also need to deal with, because I think that uma_zone_get_max() will return non-sense without the reservation or a call to uma_zone_set_max().

I always return 0 from swapon_check_swzone(), it seems to be a simplest way to ignore.

diff --git a/sys/vm/swap_pager.c b/sys/vm/swap_pager.c
index c906b96eee4..77fcd6f89d7 100644
--- a/sys/vm/swap_pager.c
+++ b/sys/vm/swap_pager.c
@@ -526,28 +526,32 @@ swap_pager_swap_init(void)
 	swblk_zone = uma_zcreate("swblk", sizeof(struct swblk), NULL, NULL,
 	    NULL, NULL, _Alignof(struct swblk) - 1,
 	    UMA_ZONE_NOFREE | UMA_ZONE_VM);
 	if (swblk_zone == NULL)
 		panic("failed to create swap blk zone.");
 	n2 = n;
+#if 0
 	do {
 		if (uma_zone_reserve_kva(swblk_zone, n))
 			break;
 		/*
 		 * if the allocation failed, try a zone two thirds the
 		 * size of the previous attempt.
 		 */
 		n -= ((n + 2) / 3);
 	} while (n > 0);
+#endif
 	if (n2 != n)
 		printf("Swap blk zone entries reduced from %lu to %lu.\n",
 		    n2, n);
 	swap_maxpages = n * SWAP_META_PAGES;
 	swzone = n * sizeof(struct swblk);
+#if 0
 	if (!uma_zone_reserve_kva(swpctrie_zone, n))
 		panic("Cannot reverse swap pctrie zone entries.");
+#endif
 }
 
 static vm_object_t
 swap_pager_alloc_init(void *handle, struct ucred *cred, vm_ooffset_t size,
     vm_ooffset_t offset)
 {
@@ -2069,12 +2073,13 @@ done:
  * exceed half the theoretical maximum.  If it does, print a warning
  * message and return -1; otherwise, return 0.
  */
 static int
 swapon_check_swzone(unsigned long npages)
 {
+#if 0
 	unsigned long maxpages;
 
 	/* absolute maximum we can handle assuming 100% efficiency */
 	maxpages = uma_zone_get_max(swblk_zone) * SWAP_META_PAGES;
 
 	/* recommend using no more than half that amount */
@@ -2083,12 +2088,13 @@ swapon_check_swzone(unsigned long npages)
 		    "exceeds maximum recommended amount (%lu pages).\n",
 		    npages, maxpages / 2);
 		printf("warning: increase kern.maxswzone "
 		    "or reduce amount of swap.\n");
 		return (-1);
 	}
+#endif
 	return (0);
 }
 
 static void
 swaponsomething(struct vnode *vp, void *id, u_long nblks,
     sw_strategy_t *strategy, sw_close_t *close, dev_t dev, int flags)

Yes, let's try this patched version on a swap-intensive workload.

In D11435#243818, @alc wrote:

Yes, let's try this patched version on a swap-intensive workload.

This somehow got lost in my private inbox.

@pho wrote:
I ran tests for four hours, without finding any thing interesting, I think.
I ran the PAE test with the #if 0 added. On amd64 I ran the original patch.

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.

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.

Does that make sense?

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.

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.

My recollection from Bonwick's paper, which I haven't looked at in very long time, is that vmem doesn't coalesce allocated ranges for at least two reasons. Chief among the reasons, it allows vmem to trivially guarantee that a free will never fail because of failure to allocate a boundary tag. Any new interface that allowed for partially freeing an allocated range would have to allow for the possibility of failure.

That said, maintaining boundary tags in vmem for UMA _NOFREE slabs is downright silly.

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

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.

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.

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.

Originally, I proposed the conversion from a single, global hash table to a per-object radix trie because I thought it would make operations like swap_pager_find_least() and swp_pager_meta_free_all() faster because they would no longer have to probe the hash table iteratively. However, I don't disagree with your statement about the change in locking being important. For example, vm_object_scan_all_shadowed() should hypothetically benefit from both a more efficient swap_pager_find_least() and the finer-grained locking.

Returning to memory utilization, the new code should deal with fragmented swap usage within vm objects more efficiently than the old code because the size of swblk is smaller than swblock, especially on 32-bit architectures where it is roughly 1/4 the size.

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:

With the patch: 4699560 bytes of swblk and swpctrie structures

Without the patch: 5399776 bytes of SWAPMETA structures and swhash array

I'm pretty happy with this result.