Page MenuHomeFreeBSD

exploit sparsity in swap_pager_copy
ClosedPublic

Authored by dougm on Nov 8 2019, 7:42 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 29, 9:46 PM
Unknown Object (File)
Fri, Mar 29, 9:46 PM
Unknown Object (File)
Fri, Mar 29, 9:46 PM
Unknown Object (File)
Fri, Mar 29, 9:41 PM
Unknown Object (File)
Fri, Mar 29, 9:41 PM
Unknown Object (File)
Mar 7 2024, 2:38 AM
Unknown Object (File)
Mar 6 2024, 4:21 PM
Unknown Object (File)
Feb 19 2024, 1:08 AM

Details

Summary

This is based on the observation by Yoshihiro Ota expressed in D22256, but exploits that observation differently.

Modify swp_pager_meta_free to create swp_pager_meta_copy_free that can implement swp_pager_meta_free and also do the "Transfer source to destination" work of swap_pager_copy. The benefit of this change is fewer pctrie accesses and more efficient freeing of trie elements in swap_pager_copy.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I don't think this code duplication in swap_pager_copy() is a good idea.
I updated D22256 to avoid the double lookup cost from the first revision.
I'm still testing the 2nd version though.

I've also bothered with the naming of swp_pager_meta_ctl(), this function indeed looks up and then optionally removal, too. On the other hand, I don't think reducing code reuse benefit duplicating code.

Define swp_pager_meta_copy_free to implement swp_pager_meta_free and part of swap_pager_copy, to avoid code duplication and to exploit some of the optimizations of the meta_free code when copying.

dougm edited the summary of this revision. (Show Details)

Discard the changes that eliminate swap_meta_ctl. Those changes distract from the primary purpose of this change.

With Diff 64139 I got a "panic: vm_object_madvise: page 0xfffffe0000174cf8 is not managed", which to me seems unrelated.
https://people.freebsd.org/~pho/stress/log/dougm063.txt

sys/vm/swap_pager.c
947

How the 'represented by a resident page' part is checked ?

955

I do not think this blank comment line is useful.

2010

I think this part of the description is not longer valid.

2014

Indent of the continuation line is wrong.

2040

Again indent is wrong.

dougm marked 5 inline comments as done.

Fix indentation. Discard some comments.

sys/vm/swap_pager.c
2013–2014

This function now either transfer to another object or free.
The source doesn't keep at all.
I don't think "copy" is appropriate.

How about swp_pager_meta_xfer / swp_pager_meta_transfer with implication of transferring to NULL is a removal?

swp_pager_meta_free() is exclusive use for free.

dougm marked an inline comment as done.

Rename "copy" to "xfer" or "transfer" in several places.
Move a dstobject==NULL test from callee to caller.

This looks like a reasonable change.

sys/vm/swap_pager.c
2010–2011

I don't think that this sentence is true.

dougm marked an inline comment as done.

Remove an inaccurate comment.

This revision is now accepted and ready to land.Nov 11 2019, 10:04 AM
sys/vm/swap_pager.c
949

I don't think that the comment about the destination not being resident is guaranteed to be true.