Page MenuHomeFreeBSD
Differential D20986

Centralize the logic in vfs_vmio_unwire() and sendfile_free_page().
ClosedPublic
Actions

Authored by markj on Jul 18 2019, 6:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 5:46 PM
Unknown Object (File)
Fri, Nov 8, 2:50 AM
Unknown Object (File)
Fri, Nov 1, 7:15 PM
Unknown Object (File)
Oct 22 2024, 9:35 AM
Unknown Object (File)
Oct 19 2024, 12:48 AM
Unknown Object (File)
Oct 3 2024, 6:06 AM
Unknown Object (File)
Oct 1 2024, 2:18 PM
Unknown Object (File)
Sep 30 2024, 10:03 AM
View All 97 Files
Subscribers
imp

Details

Reviewers
alc
dougm
kib
jeff
glebius
Commits
rS350431: Centralize the logic in vfs_vmio_unwire() and sendfile_free_page().
Summary

These functions are similar and I would like to have their functionality
live in generic VM functions so that the details of vm_page
synchronization can be encapsulated in vm_page.c.

The change adds vm_page_release() and vm_page_release_locked(). They
both unwire the page and optionally attempt to free the page.
vm_page_release_locked() requires the object lock. The main difference
is that vfs_vmio_unwire() may attempt to free a mapped page, while
sendfile_free_page() does not. This change thus modifies the handling
of direct I/O and B_NOREUSE to avoid freeing a mapped page.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj created this revision.Jul 18 2019, 6:53 PM
Harbormaster completed remote builds in B25402: Diff 59888.Jul 18 2019, 6:53 PM
markj added reviewers: alc, dougm, kib, jeff, glebius.Jul 18 2019, 6:54 PM
kib added inline comments.Jul 18 2019, 8:11 PM
sys/vm/vm_page.c
171 ↗(On Diff #59888)

Is this unrelated ?

3768 ↗(On Diff #59888)

What prevents other thread from busying the page after we tested for !xbusy but before TRYWLOCK() succeeded ?

markj added inline comments.Jul 18 2019, 8:16 PM
sys/vm/vm_page.c
171 ↗(On Diff #59888)

No, I renamed the old (static) vm_page_release() to vm_page_zone_release(), and renamed vm_page_import() as well for consistency.

3768 ↗(On Diff #59888)

Nothing, but there is a vm_page_busied(m) check after we acquire the object lock. This is identical to the logic in sendfile_free_page(). I accidentally removed a comment explaining that the first check is racy, I will restore it.

markj updated this revision to Diff 59893.Jul 18 2019, 8:21 PM

Restore a lost comment.

Harbormaster completed remote builds in B25406: Diff 59893.Jul 18 2019, 8:21 PM
kib added inline comments.Jul 18 2019, 8:56 PM
sys/vm/vm_page.c
3768 ↗(On Diff #59888)

Then why the first (racy) check is not vm_page_busied() ?

markj updated this revision to Diff 59894.Jul 18 2019, 9:01 PM

Don't attempt to free the page if it is busy at all.

Harbormaster completed remote builds in B25407: Diff 59894.Jul 18 2019, 9:01 PM
kib accepted this revision.Jul 18 2019, 9:14 PM
kib added inline comments.
sys/vm/vm_page.c
171 ↗(On Diff #59888)

I think it is useful to commit this part separately, and definitely merge it to 12.

This revision is now accepted and ready to land.Jul 18 2019, 9:14 PM
markj updated this revision to Diff 59969.Jul 20 2019, 8:33 PM

Rebase.

This revision now requires review to proceed.Jul 20 2019, 8:33 PM
Harbormaster completed remote builds in B25439: Diff 59969.Jul 20 2019, 8:33 PM
dougm added inline comments.Jul 22 2019, 4:45 AM
sys/vm/vm_page.c
3758 ↗(On Diff #59969)

assert that m->object is not locked.

3797 ↗(On Diff #59969)

Consider defining these seven lines as a function of m and flags, and using it here and below.

markj updated this revision to Diff 60035.Jul 23 2019, 1:39 AM
markj marked 2 inline comments as done.
  • Use a common subrouting for releasing to the page queues.
  • Assert that the object is unlocked in vm_page_release().
Harbormaster completed remote builds in B25463: Diff 60035.Jul 23 2019, 1:39 AM
markj added inline comments.Jul 23 2019, 1:39 AM
sys/vm/vm_page.c
3758 ↗(On Diff #59969)

Note that m->object may be NULL. I added the conditional assertion under the page lock, since the page lock is required in order to modify the object field.

kib accepted this revision.Jul 23 2019, 7:42 AM
This revision is now accepted and ready to land.Jul 23 2019, 7:42 AM
alc added inline comments.Jul 27 2019, 4:10 AM
sys/vm/vm_page.c
3782 ↗(On Diff #60035)

"managed" -> "unmanaged"

3823 ↗(On Diff #60035)

"managed" -> "unmanaged"

markj updated this revision to Diff 60206.Jul 27 2019, 11:51 PM

Fix typos.

This revision now requires review to proceed.Jul 27 2019, 11:51 PM
Harbormaster completed remote builds in B25548: Diff 60206.Jul 27 2019, 11:51 PM
alc accepted this revision.Jul 28 2019, 9:37 PM
This revision is now accepted and ready to land.Jul 28 2019, 9:37 PM
Closed by commit rS350431: Centralize the logic in vfs_vmio_unwire() and sendfile_free_page(). (authored by markj). · Explain WhyJul 29 2019, 10:01 PM
This revision was automatically updated to reflect the committed changes.
markj added a commit: rS350431: Centralize the logic in vfs_vmio_unwire() and sendfile_free_page()..
Herald added a subscriber: imp. · View Herald TranscriptJul 29 2019, 10:01 PM

Revision Contents
Changeset List

PathSize
head/
sys/
kern/
70 lines
62 lines
vm/
7 lines
97 lines

Diff 60260

View Options

head/sys/kern/kern_sendfile.c

Loading...
View Options

head/sys/kern/vfs_bio.c

Loading...
View Options

head/sys/vm/vm_page.h

Loading...
View Options

head/sys/vm/vm_page.c

Loading...