allow frees to aggregate in swap_pager_copy

[dougm]

Summary

swap_pager_copy frees blocks one at at time, via swp_pager_meta_ctl, with no opportunity to recognize freeing of consecutive blocks and free fewer block ranges. To open that opportunity, this change removes the SWM_FREE option from swp_pager_meta_ctl, and compels the caller to do the freeing when a valid block address is returned. In swap_pager_copy, these frees are aggregated, so that a sequence of them can be done at one time.

The only other caller to swp_pager_meta_ctl that passes SWM_FREE, swp_pager_unswapped, is also modified to handle its single free explicitly.

[kib]

sys/vm/swap_pager.c

314 ↗

(On Diff #35944)

Perhaps change the value to 0x1 ?

943 ↗

(On Diff #35944)

Move the variables to the function' locals block, per style.

[dougm]

Adopt reviewer suggestions.

[alc]

sys/vm/swap_pager.c

2004 ↗

(On Diff #37236)

Please remove the "and vm_page_t". It was a stale reference even before your proposed change.

2006 ↗

(On Diff #37236)

"... of either lookup up or removing swapblk ..."

2007 ↗

(On Diff #37236)

"... meta data. It returns ..."

2008 ↗

(On Diff #37236)

"... looked up, popped, or ..."

2010 ↗

(On Diff #37236)

This last sentence makes no sense to me. Just delete it.

2016 ↗

(On Diff #37236)

I don't see the point of the ".. pop it out". However, I do see the point of saying that this option does not free the swap block.

[dougm]

Adopt reviewer comments about comments.

[alc]

sys/vm/swap_pager.c

968–969 ↗

(On Diff #37236)

The "else" should be on the same line as the "}".

[dougm]

Fix 'else' spacing.

[alc]

Is there some reason why we shouldn't do

srcaddr = swp_pager_meta_ctl(srcobject, i + offset, SWM_POP);
if (srcaddr == SWAPBLK_NONE)
        continue;

before the

dstaddr = swp_pager_meta_ctl(dstobject, i, 0);

?

That would seem to eliminate unnecessary lookups on the dstobject, which would be another improvement over the existing code.

[alc]

sys/vm/swap_pager.c

938 ↗

(On Diff #37268)

Let's capitalize the "t" in "transfer" while we're in the neighborhood.

[dougm]

Capitalize in a comment.

As swap_pager_meta_ctl(,,0) looks side-effect free, I agree that it can be called to compute srcaddr before deciding whether to compute dstaddr (with side effects). The comment that precedes all this seems like gibberish, and is mostly deleted.

[alc]

sys/vm/swap_pager.c

947 ↗

(On Diff #37273)

I don't think that this comment adds anything useful. Just delete it.

[dougm]

Drop unwanted comment.