From owner-svn-src-all@freebsd.org Fri Sep 27 16:46:08 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C7F84127A54; Fri, 27 Sep 2019 16:46:08 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46fyNc4rkfz4308; Fri, 27 Sep 2019 16:46:08 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 705C41BC7D; Fri, 27 Sep 2019 16:46:08 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x8RGk89K055253; Fri, 27 Sep 2019 16:46:08 GMT (envelope-from markj@FreeBSD.org) Received: (from markj@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x8RGk8IC055252; Fri, 27 Sep 2019 16:46:08 GMT (envelope-from markj@FreeBSD.org) Message-Id: <201909271646.x8RGk8IC055252@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: markj set sender to markj@FreeBSD.org using -f From: Mark Johnston Date: Fri, 27 Sep 2019 16:46:08 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r352802 - head/sys/vm X-SVN-Group: head X-SVN-Commit-Author: markj X-SVN-Commit-Paths: head/sys/vm X-SVN-Commit-Revision: 352802 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Sep 2019 16:46:08 -0000 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); }