Page MenuHomeFreeBSD
Differential D46217

vm_pageout: simplify pageout_cluster
ClosedPublic
Actions

Authored by dougm on Aug 3 2024, 5:43 AM.
Tags
None
Referenced Files
F95177983: D46217.diff
Thu, Sep 19, 4:52 PM
Unknown Object (File)
Thu, Sep 12, 9:25 PM
Unknown Object (File)
Tue, Sep 3, 1:46 AM
Unknown Object (File)
Mon, Sep 2, 12:01 AM
Unknown Object (File)
Sun, Sep 1, 9:57 PM
Unknown Object (File)
Aug 14 2024, 10:14 AM
Unknown Object (File)
Aug 9 2024, 7:46 AM
Unknown Object (File)
Aug 7 2024, 4:13 AM
View All 21 Files
Subscribers
imp

Details

Reviewers
alc
kib
Commits
rGacb4cb33d358: vm_pageout: simplify pageout_cluster
Summary

Rewrite vm_pageout_cluster to eliminate redundant variables and duplicated code, without changing its functionality.

Remove tests on pindex to check for object boundary conditions, since the page_next and page_prev functions return NULL at the object boundaries. Fix an alignment error that could happen if pindex is aligned, and the first of vm_pageout_page_count flushable pages, and the page at pindex-1 is also flushable.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

dougm requested review of this revision.Aug 3 2024, 5:43 AM
dougm created this revision.
alc added a comment.Aug 3 2024, 6:51 AM

For the most part, I am fine with this. Personally, I would prefer to see the NULL check remain in the caller, and I dislike the ok2cluster in vm_pageout_ok2cluster.

dougm updated this revision to Diff 141766.Aug 3 2024, 6:58 AM

Amend to address @alc comments.

dougm updated this revision to Diff 141767.Aug 3 2024, 7:44 AM

Add a comment to explain the ib > pindex test.

dougm added inline comments.Aug 3 2024, 7:53 AM
sys/vm/vm_pageout.c
394–395

Actually, this test is unnecessary. If ib > pindex, then m->pindex == 0 and vm_page_prev(m) is NULL, so the break will happen anyway.

dougm added inline comments.Aug 3 2024, 8:05 AM
sys/vm/vm_pageout.c
404–405

The object->size test is unnecessary, since if pindex + is == object->size, vm_page_next(m) will be NULL and terminate the loop.

Better to do that, and set a right_terminated flag before the break.

Then, in the first loop, move the % vm_pageout_page_count test to the top of the loop, but only test it if right_terminated is false.

And replace "ib == 0" with left_terminated ==true, and get rid of ib and is variables.

dougm updated this revision to Diff 141768.Aug 3 2024, 9:29 AM

More changes, to remove needless tests and more variables.

dougm updated this revision to Diff 141774.Aug 3 2024, 5:08 PM

Rewrite the alignment-test expression to make it more readable.

dougm updated this revision to Diff 141779.Aug 3 2024, 7:24 PM

Move the "% vm_pageout_page_count" calculation out of the loop.

dougm updated this revision to Diff 141781.Aug 3 2024, 7:38 PM

Remove a redundant 'num_ends' check from the alignment test.

dougm updated this revision to Diff 141782.Aug 3 2024, 7:46 PM

Undo the last change, since it breaks the case with the first iteration of the forward loop fails.

dougm edited the summary of this revision. (Show Details)Aug 4 2024, 3:21 AM
kib accepted this revision.Aug 4 2024, 10:07 AM
This revision is now accepted and ready to land.Aug 4 2024, 10:07 AM
Closed by commit rGacb4cb33d358: vm_pageout: simplify pageout_cluster (authored by dougm). · Explain WhyAug 4 2024, 8:34 PM
This revision was automatically updated to reflect the committed changes.
dougm added a commit: rGacb4cb33d358: vm_pageout: simplify pageout_cluster.
Herald added a subscriber: imp. · View Herald TranscriptAug 4 2024, 8:34 PM

Revision Contents
Changeset List

PathSize
sys/
vm/
114 lines

Diff 141768

View Options

sys/vm/vm_pageout.c

Loading...