svn commit: r352802 - head/sys/vm
Mark Johnston
markj at FreeBSD.org
Fri Sep 27 16:46:08 UTC 2019
Author: markj
Date: Fri Sep 27 16:46:08 2019
New Revision: 352802
URL: https://svnweb.freebsd.org/changeset/base/352802
Log:
Fix a race in vm_page_swapqueue().
vm_page_swapqueue() atomically transitions a page between queues. To do
so, it must hold the page queue lock for the old queue. However, once
the queue index has been updated, the queue lock no longer protects the
page's queue state. Thus, we must speculatively remove the page from
the old queue before committing the queue state update, and roll back if
the update fails.
Reported and tested by: pho
Reviewed by: kib
Sponsored by: Intel, Netflix
Differential Revision: https://reviews.freebsd.org/D21791
Modified:
head/sys/vm/vm_page.c
Modified: head/sys/vm/vm_page.c
==============================================================================
--- head/sys/vm/vm_page.c Fri Sep 27 16:44:29 2019 (r352801)
+++ head/sys/vm/vm_page.c Fri Sep 27 16:46:08 2019 (r352802)
@@ -3462,27 +3462,58 @@ void
vm_page_swapqueue(vm_page_t m, uint8_t oldq, uint8_t newq)
{
struct vm_pagequeue *pq;
+ vm_page_t next;
+ bool queued;
KASSERT(oldq < PQ_COUNT && newq < PQ_COUNT && oldq != newq,
("vm_page_swapqueue: invalid queues (%d, %d)", oldq, newq));
- KASSERT((m->oflags & VPO_UNMANAGED) == 0,
- ("vm_page_swapqueue: page %p is unmanaged", m));
vm_page_assert_locked(m);
- /*
- * Atomically update the queue field and set PGA_REQUEUE while
- * ensuring that PGA_DEQUEUE has not been set.
- */
pq = &vm_pagequeue_domain(m)->vmd_pagequeues[oldq];
vm_pagequeue_lock(pq);
- if (!vm_page_pqstate_cmpset(m, oldq, newq, PGA_DEQUEUE, PGA_REQUEUE)) {
+
+ /*
+ * The physical queue state might change at any point before the page
+ * queue lock is acquired, so we must verify that we hold the correct
+ * lock before proceeding.
+ */
+ if (__predict_false(m->queue != oldq)) {
vm_pagequeue_unlock(pq);
return;
}
- if ((m->aflags & PGA_ENQUEUED) != 0) {
- vm_pagequeue_remove(pq, m);
+
+ /*
+ * Once the queue index of the page changes, there is nothing
+ * synchronizing with further updates to the physical queue state.
+ * Therefore we must remove the page from the queue now in anticipation
+ * of a successful commit, and be prepared to roll back.
+ */
+ if (__predict_true((m->aflags & PGA_ENQUEUED) != 0)) {
+ next = TAILQ_NEXT(m, plinks.q);
+ TAILQ_REMOVE(&pq->pq_pl, m, plinks.q);
vm_page_aflag_clear(m, PGA_ENQUEUED);
+ queued = true;
+ } else {
+ queued = false;
}
+
+ /*
+ * Atomically update the queue field and set PGA_REQUEUE while
+ * ensuring that PGA_DEQUEUE has not been set.
+ */
+ if (__predict_false(!vm_page_pqstate_cmpset(m, oldq, newq, PGA_DEQUEUE,
+ PGA_REQUEUE))) {
+ if (queued) {
+ vm_page_aflag_set(m, PGA_ENQUEUED);
+ if (next != NULL)
+ TAILQ_INSERT_BEFORE(next, m, plinks.q);
+ else
+ TAILQ_INSERT_TAIL(&pq->pq_pl, m, plinks.q);
+ }
+ vm_pagequeue_unlock(pq);
+ return;
+ }
+ vm_pagequeue_cnt_dec(pq);
vm_pagequeue_unlock(pq);
vm_page_pqbatch_submit(m, newq);
}
More information about the svn-src-head
mailing list