Page MenuHomeFreeBSD

vm: Micro-optimize page rename
ClosedPublic

Authored by alc on Thu, Nov 28, 5:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 10, 4:33 AM
Unknown Object (File)
Mon, Dec 2, 5:58 PM
Unknown Object (File)
Sat, Nov 30, 5:35 PM
Unknown Object (File)
Thu, Nov 28, 6:22 PM
Subscribers

Details

Summary

Rename vm_page_rename() to vm_page_iter_rename() to reflect its reimplementation using iterators, and pass the page to this function rather than spending clock cycles looking it up. Change its return value from 0/1 to a bool.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

alc requested review of this revision.Thu, Nov 28, 5:06 PM
alc created this revision.

Comments in vm_reserv.c need to be updated to reflect this change.

If you have measured the improvement, how big is it?

markj added inline comments.
sys/vm/vm_page.c
2072

Perhaps further clarify that failure happens only when memory allocation fails. In particular, if a page already exists at the destination, we panic.

I'd also clarify that the page is passed directly only as an optimization.

2093

I believe we can strengthen this to KASSERT((m->ref_count & VPRC_OBJREF) != 0, as in vm_page_insert_radixdone() for example.

This revision is now accepted and ready to land.Thu, Nov 28, 5:14 PM
sys/vm/vm_object.c
1793

Fix comment.

alc marked 3 inline comments as done.Fri, Nov 29, 5:54 PM
alc added inline comments.
sys/vm/vm_page.c
2072

I didn't describe passing the page as being an optimization as I'm afraid that doing so could be misinterpreted as meaning that NULL can be passed.

alc marked an inline comment as done.
alc edited the summary of this revision. (Show Details)

Update in response to reviewer comments.

This revision now requires review to proceed.Fri, Nov 29, 5:58 PM
sys/vm/vm_page.c
2075

I don't see a panic anywhere.

sys/vm/vm_reserv.c
681

'm' and 'new_object' are now the second and third arguments to vm_page_iter_rename, so you should be either less precise ("vm_page_iter_rename(...)") or more accurate ("vm_page_iter_rename(&old_pages, m, new_object. ...)").

Comments in vm_reserv.c need to be updated to reflect this change.

If you have measured the improvement, how big is it?

The conversion to iterators made vm_object_collapse_scan() and vm_object_split() slower by 14% and 5-6%, respectively. This change eliminates about half of that slowdown in each case.

Update in response to reviewer comments.

alc marked an inline comment as done.Fri, Nov 29, 6:53 PM
alc added inline comments.
sys/vm/vm_page.c
2075

vm_radix_insert_lookup_lt() panics.

This revision is now accepted and ready to land.Fri, Nov 29, 6:58 PM
This revision was automatically updated to reflect the committed changes.
alc marked an inline comment as done.