Differential D4497

vm_page_replace: add wrapper to KASSERT about old page
ClosedPublic
Actions

Authored by rlibby on Dec 11 2015, 8:49 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Nov 22, 6:48 AM

	Unknown Object (File)
	Fri, Nov 22, 1:02 AM

	Unknown Object (File)
	Fri, Nov 22, 12:03 AM

	Unknown Object (File)
	Thu, Nov 21, 11:31 PM

	Unknown Object (File)
	Mon, Nov 18, 11:25 PM

	Unknown Object (File)
	Mon, Nov 18, 11:28 AM

	Unknown Object (File)
	Mon, Nov 18, 1:14 AM

	Unknown Object (File)
	Sun, Nov 17, 10:09 PM

Subscribers

Details

Reviewers

kib
alc

Commits

rS292406: vm_page_replace: add wrapper to KASSERT about old page

Summary

It turns out the callers of vm_page_replace know exactly which page they
are replacing and would like to assert about it. Change those from hard
panics to KASSERTs, and provide them with a wrapper so they don't have
to deal with warnings from an INVARIANTS-dependent dead store of the
return value of vm_page_replace.

Preliminary work for D4478.

I'm definitely open to suggestions for renaming the wrapper and
vm_page_replace if you have any ideas.

Test Plan

kyua test -k /usr/tests/sys/Kyuafile

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1569
Build 1575: arc lint + arc unit

Event Timeline

rlibby updated this revision to Diff 11080.Dec 11 2015, 8:49 AM

rlibby retitled this revision from to vm_page_replace: add wrapper to KASSERT about old page.

rlibby updated this object.

rlibby edited the test plan for this revision. (Show Details)

rlibby added reviewers: alc, kib.

Herald added a subscriber: imp. · View Herald TranscriptDec 11 2015, 8:49 AM

Traditionally we used names like XXX_locked(), might be name it vm_page_replace_checked(). IMO the _check/checked suffix is telling enough.

This revision is now accepted and ready to land.Dec 11 2015, 12:39 PM

vm_page_replace: kib feedback: s/check/checked/

This revision now requires review to proceed.Dec 11 2015, 4:47 PM

kib accepted this revision.Dec 11 2015, 4:52 PM

kib edited edge metadata.

This revision is now accepted and ready to land.Dec 11 2015, 4:52 PM

rlibby mentioned this in D4478: vm_fault_hold: use vm_page_replace instead of vm_page_rename.Dec 11 2015, 5:06 PM

cem added a subscriber: cem.Dec 16 2015, 11:33 PM

cem added inline comments.

sys/vm/sg_pager.c
190–193	sg_pager_getpages has been rototilled out from underneath this patch, needs rebasing before I can apply it.

Rebase, resolving conflict.

Resolution was trivial, I don't think this needs re-review, but I'm
unsure if there's some kind of policy for that.

I am having environment issues so I build-tested the fix up but didn't
run any tests.

This revision now requires review to proceed.Dec 17 2015, 11:06 AM

alc accepted this revision.Dec 17 2015, 5:27 PM

alc edited edge metadata.

This revision is now accepted and ready to land.Dec 17 2015, 5:27 PM

Closed by commit rS292406: vm_page_replace: add wrapper to KASSERT about old page (authored by cem). · Explain WhyDec 17 2015, 5:49 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

vm/

3 lines

5 lines

15 lines

Diff 11080

sys/vm/device_pager.c

Loading...

sys/vm/sg_pager.c

Loading...

sys/vm/vm_page.h

Loading...