Bug 267621 - panic: page ... already unswappable
Summary: panic: page ... already unswappable
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 14.0-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Eric van Gyzen
URL: https://reviews.freebsd.org/D37320
Keywords: crash
Depends on:
Blocks:
 
Reported: 2022-11-07 17:17 UTC by Eric van Gyzen
Modified: 2024-10-17 21:59 UTC (History)
2 users (show)

See Also:
koobs: mfc-stable13?

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric van Gyzen freebsd_committer freebsd_triage 2022-11-07 17:17:14 UTC 
We saw

  panic: page ... already unswappable

on a downstream codebase.  The page was wired.  Another thread in physio() -> vm_fault_quick_hold_pages() -> vm_page_wire_mapped() had just wired it.

Can we simply relax the assertion?  Or should we test for a wired page (with appropriate synchronization)?

This is trivially reproduced by adding vm_page_wire(mc[0]) at the top of vm_pageout_flush(), which seems to be a fair repro due to the lack of synchronization.
Comment 1 Eric van Gyzen freebsd_committer freebsd_triage 2022-11-07 17:18:34 UTC 
...due to the lack of synchronization with vm_fault_quick_hold_pages(), that is.
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2022-11-07 17:28:11 UTC 
Yes, I agree that the assertion is incorrect and should simply be removed.  It has been racy since we removed the use of the page hash lock to synchronize wiring of pages.  We should assert that the page's object is locked, though.

Just for some background on this queue, I added it to try and work around a problem on swapless systems where the page daemon would spend inordinate amounts of CPU in low memory situations trying to swap out anonymous pages, only to fail over and over.  We can't simply not scan queues since we might have a large number of dirty filesystem pages which *can* be paged out.
Comment 3 Eric van Gyzen freebsd_committer freebsd_triage 2022-11-07 21:02:08 UTC 
Thanks as always, Mark.  I'll test that and get back to you.
Comment 4 Eric van Gyzen freebsd_committer freebsd_triage 2022-11-09 20:21:47 UTC 
https://reviews.freebsd.org/D37320
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-11-09 20:30:55 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=cfbf1da0deede6b82ac197471b6cd439706a2dea

commit cfbf1da0deede6b82ac197471b6cd439706a2dea
Author:     Anton Rang <rang@acm.org>
AuthorDate: 2022-11-09 20:13:01 +0000
Commit:     Eric van Gyzen <vangyzen@FreeBSD.org>
CommitDate: 2022-11-09 20:28:03 +0000

    vm_page_unswappable: remove wrong assertion

    markj says:

        ...the assertion is incorrect and should simply be removed.
        It has been racy since we removed the use of the page hash
        lock to synchronize wiring of pages.

    PR:             267621
    Reviewed by:    markj, Anton Rang <rang@acm.org>
    MFC after:      1 week
    Sponsored by:   Dell Inc.
    Differential Revision:  https://reviews.freebsd.org/D37320

 sys/vm/vm_page.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 6 Mark Linimon freebsd_committer freebsd_triage 2024-01-02 20:01:20 UTC 
^Triage: FreeBSD 12 is now out of support.

While this PR is open, do some QA to it.
Comment 7 Mark Johnston freebsd_committer freebsd_triage 2024-10-17 21:59:53 UTC 
I believe there's nothing left to do here, please re-open if I'm wrong.