combine block frees in swap_pager_meta_frees

[dougm]

Summary

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.

[dougm]

Change swap_pager_freeswapspace npages params and args from int to daddr_t.

[dougm]

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.

[dougm]

Undo the most recent update, as it had an unintended effect.

[dougm]

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.

[dougm]

Tidy up.

[dougm]

Handle index - sb->p < 0.

[alc]

sys/vm/swap_pager.c

1912 ↗

(On Diff #33228)

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"?

[dougm]

Avoid casting vm_pindex_t differences to ints before comparing them.

sys/vm/swap_pager.c

1912 ↗

(On Diff #33228)

It is.

[dougm]

Fix prototype.

[alc]

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)

[kib]

The change to swp_pager_freeswapspace() signature and handling of empty runs can be committed in advance.

sys/vm/swap_pager.c

1925 ↗

(On Diff #35790)

() are not needed.

1926 ↗

(On Diff #35790)

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 ↗

(On Diff #35790)

() are not needed.

1932 ↗

(On Diff #35790)

() are not needed.

[kib]

sys/vm/swap_pager.c

1934 ↗

(On Diff #35790)

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 ?

[dougm]

Add a function to test for empty block ranges, and use it to simplify code.

[dougm]

sys/vm/swap_pager.c

1926 ↗

(On Diff #35790)

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 ↗

(On Diff #35790)

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.

[alc]

sys/vm/swap_pager.c

1901 ↗

(On Diff #35804)

This has been de-indented enough that a continuation line is probably not needed.

[dougm]

Fold two lines into one, where de-indentation now allows it.

[alc]

sys/vm/swap_pager.c

1770 ↗

(On Diff #35807)

I think that swp_pager_swblk_empty() would be a better name.

[dougm]

Rename the new function, and reposition its declaration.

[kib]

sys/vm/swap_pager.c

1774 ↗

(On Diff #35807)

By 'i <= limit' I mean 'start <= limit'.

[dougm]

Drop 'empty' variable.

[dougm]

Update an assertion.

[dougm]

Add missing brace.

[alc]

Peter, can you please test this patch?

[pho]

In D12397#276487, @alc wrote:

Peter, can you please test this patch?

Sure, happy to.

[pho]

In D12397#276487, @alc wrote:

Peter, can you please test this patch?

I ran all of the stress2 tests. No problems seen.