Page MenuHomeFreeBSD

Re-check for wirings after busying the page in vm_page_release_locked().
ClosedPublic

Authored by markj on Apr 27 2020, 5:06 PM.
Tags
None
Referenced Files
F85964772: D24592.diff
Thu, Jun 13, 9:31 AM
Unknown Object (File)
Mon, May 27, 4:08 AM
Unknown Object (File)
Apr 8 2024, 9:05 PM
Unknown Object (File)
Apr 8 2024, 9:00 PM
Unknown Object (File)
Apr 8 2024, 9:59 AM
Unknown Object (File)
Feb 13 2024, 12:50 PM
Unknown Object (File)
Jan 27 2024, 6:59 AM
Unknown Object (File)
Jan 15 2024, 9:54 PM
Subscribers

Details

Summary

vm_page_release_locked() attempts to free a previously wired page when
sendfile(SF_NOCACHE) completes, or the buffer cache releases a page
following direct I/O. A concurrent unlocked vm_page_grab() can wire the
page using only the busy lock, so we must check for new wirings after
the busy lock is acquired here, and avoid freeing the page if any are
detected. Once the (exclusive) busy lock is held, that, the object lock
and the fact that the page is unmapped guarantee that the wire count is
stable. If a new wiring is found, it is up to the unwiring thread to
ensure that it ends up in the page queues, so vm_page_release_locked()
can simply return.

Other places where we rely on this logic (e.g., page daemon, contig
reclaim) already handle it correctly.

Update the comment above vm_page_wired() to reflect the new
synchronization rules.

Test Plan

None yet, Gleb reported a panic that looks like it is caused by this.

Diff Detail

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

Event Timeline

markj requested review of this revision.Apr 27 2020, 5:06 PM
markj created this revision.
markj added reviewers: kib, alc, jeff, glebius.
This revision is now accepted and ready to land.Apr 27 2020, 6:37 PM
jeff added inline comments.
sys/vm/vm_page.c
4173–4177 ↗(On Diff #71064)

This pattern repeats all over and I have made a similar mistake with it. I would like to consider a wrapper or even just having vm_page_free() detect it.

sys/vm/vm_page.c
4173–4177 ↗(On Diff #71064)

Looking through a few VM files I think having a wrapper which does the tryxbusy, checks for wirings, and returns the result while also handling the xunbusy would be useful. There are many cases where we do not immediately want to free the page though, so I think having a standalone function would be the most useful.