Page MenuHomeFreeBSD

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

Authored by markj on Jul 18 2019, 6:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 5:40 AM
Unknown Object (File)
Nov 26 2023, 5:26 PM
Unknown Object (File)
Nov 26 2023, 9:34 AM
Unknown Object (File)
Nov 25 2023, 6:58 PM
Unknown Object (File)
Nov 25 2023, 9:31 AM
Unknown Object (File)
Nov 21 2023, 3:54 PM
Unknown Object (File)
Nov 11 2023, 8:27 AM
Unknown Object (File)
Nov 11 2023, 12:03 AM
Subscribers

Details

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

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 ?

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.

sys/vm/vm_page.c
3768 ↗(On Diff #59888)

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

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

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
This revision now requires review to proceed.Jul 20 2019, 8:33 PM
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 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().
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.

This revision is now accepted and ready to land.Jul 23 2019, 7:42 AM
sys/vm/vm_page.c
3782 ↗(On Diff #60035)

"managed" -> "unmanaged"

3823 ↗(On Diff #60035)

"managed" -> "unmanaged"

This revision now requires review to proceed.Jul 27 2019, 11:51 PM
This revision is now accepted and ready to land.Jul 28 2019, 9:37 PM