Page MenuHomeFreeBSD

Reduce duplication in grab functions. Make the code more readable.
ClosedPublic

Authored by jeff on Mon, Dec 2, 11:00 PM.

Details

Summary

These functions have become somewhat sprawling. This is partially my fault. Now that we never grab or grab_pages without a busy lock held the same asserts work in both cases. Other than this, I do not intend for there to be any functional differences.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jeff created this revision.Mon, Dec 2, 11:00 PM
jeff edited the summary of this revision. (Show Details)Mon, Dec 2, 11:03 PM
jeff added reviewers: kib, markj, alc, dougm.
jeff set the repository for this revision to rS FreeBSD src repository.
jeff added inline comments.Mon, Dec 2, 11:05 PM
sys/vm/vm_page.c
867 ↗(On Diff #65143)

I renamed this to vm_page_acquire_flags in a different patch since it will also wire the page. I think I like this name better.

I also feel like this file should really be split. Can svn do that while maintaining version history?

kib accepted this revision.Tue, Dec 3, 3:36 PM
This revision is now accepted and ready to land.Tue, Dec 3, 3:36 PM
markj added inline comments.Tue, Dec 3, 4:24 PM
sys/vm/vm_page.c
867 ↗(On Diff #65143)

I believe you can preserve history by doing an svn cp of the file before removing most of its contents.

The file is kind of unwieldy but I also like the fact that basically all logic for manipulating vm_page structures is in one file. How do you propose splitting it?

4430 ↗(On Diff #65143)

It is confusing that we use pflags as allocation flags and allocflags for everything else.

4516 ↗(On Diff #65143)

Why test == 0 when the function returns a bool?

jeff added inline comments.Tue, Dec 3, 5:55 PM
sys/vm/vm_page.c
4430 ↗(On Diff #65143)

It is somewhat confusing but unfortunately necessary. I could also break out the asserts and only compute pflags when calling vm_page_alloc so the visibility is limited.

jeff added inline comments.Tue, Dec 3, 6:04 PM
sys/vm/vm_page.c
867 ↗(On Diff #65143)

I think lookup/grab/busy/wire and allocation could be split. I don't think it's absolutely necessary to do though.

jeff updated this revision to Diff 65176.Tue, Dec 3, 9:30 PM

style fixes.

This revision now requires review to proceed.Tue, Dec 3, 9:30 PM
markj accepted this revision.Wed, Dec 4, 5:06 PM
This revision is now accepted and ready to land.Wed, Dec 4, 5:06 PM