In swap_pager_meta_free and swap_pager_meta_free_all, identify ranges of consecutive blocks to be freed, and invoke swp_pager_freeswapspace on each range, rather than on each block.
Details
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
In swap_pager_meta_free, shrink the upper and lower bounds on the i-loop, to omit iterations that just set empty to false, and instead set empty based on whether or not the loop range gets shrunk.
In swap_pager_meta_free, avoid iterations of the i loop that can only set 'empty' to false when 'empty' has already been made false.
sys/vm/swap_pager.c | ||
---|---|---|
1912–1915 | Why isn't this subject to overflow/truncation errors when a 64-bit "pindex - sb->p" is truncated to 32 bits, yielding a large positive 32-bit value even if the 64-bit value is "negative"? |
Avoid casting vm_pindex_t differences to ints before comparing them.
sys/vm/swap_pager.c | ||
---|---|---|
1912–1915 | It is. |
I've created a test program that mmap(2)s an anonymous region and writes to every page within the region. The region is 2.5 times larger than physical memory.
On one of my test machines, the region is 85810350080 bytes, and the average times to munmap(2) the region before and after this change are:
x before + after +------------------------------------------------------------------------------+ | + x | |++ + + x x | |++++ + xx x x xx x| ||_A_| |____AM___| | +------------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 10 2.8984399 3.2803552 3.0843697 3.0496894 0.13407384 + 10 1.3185973 1.4514709 1.3727556 1.3737111 0.045713057 Difference at 95.0% confidence -1.67598 +/- 0.0941133 -54.9557% +/- 3.08599% (Student's t, pooled s = 0.100164)
The change to swp_pager_freeswapspace() signature and handling of empty runs can be committed in advance.
sys/vm/swap_pager.c | ||
---|---|---|
1925 | () are not needed. | |
1926 | Create a function like static bool swp_pager_meta_empty(struct swblk *sb, int start, int limit) { int i; MPASS(i > 0 && i <= limit && limit <= SWAP_META_PAGES); for (i = start; i < limit; i++) { if (sb->d[i] != SWAPBLK_NONE) return (false); return (true); } and use it twice in this function. Perhaps it could be useful in more places, but I did not checked. That said, why do you need to check the prefix of the sb for emptiness first ? Can't you do the pass over the whole sb at the end of the iteration ? | |
1930 | () are not needed. | |
1932 | () are not needed. |
sys/vm/swap_pager.c | ||
---|---|---|
1934 | Hmm, I am not sure that I follow this. Suppose that pindex > sb->p and that there was a non-empty block recorded before the pindex. Wouldn't the loop cause the free of that block ? |
sys/vm/swap_pager.c | ||
---|---|---|
1926 | I'll create your function, with corrections, and use it in a few places. I'd rather not do the pass over the whole iteration at the end, since that would have me checking for emptiness blocks that I just set empty myself. | |
1934 | Line 1931 increases i to the limit value that was the upper bound on the first loop, to guarantee that the second loop - the one that frees blocks - does not free a block before the pindex. Not that this matters, since I'm rewriting the code based on other comments. |
sys/vm/swap_pager.c | ||
---|---|---|
1901 | This has been de-indented enough that a continuation line is probably not needed. |
sys/vm/swap_pager.c | ||
---|---|---|
1770 | I think that swp_pager_swblk_empty() would be a better name. |
sys/vm/swap_pager.c | ||
---|---|---|
1774 | By 'i <= limit' I mean 'start <= limit'. |