Bug 267621

Summary:	panic: page ... already unswappable
Product:	Base System	Reporter:	Eric van Gyzen <vangyzen>
Component:	kern	Assignee:	Eric van Gyzen <vangyzen>
Status:	In Progress ---
Severity:	Affects Some People	CC:	grahamperrin, markj
Priority:	---	Keywords:	crash
Version:	14.0-STABLE	Flags:	koobs: mfc-stable13?
Hardware:	Any
OS:	Any
URL:	https://reviews.freebsd.org/D37320

Description Eric van Gyzen freebsd_committer

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

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

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

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

2022-11-09 20:21:47 UTC

https://reviews.freebsd.org/D37320

Comment 5 commit-hook freebsd_committer

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

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.