Date: Sun, 27 Dec 2020 22:01:55 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 81846def3495 - main - vm: Fix some bugs in the page busying code Message-ID: <202012272201.0BRM1tqm043360@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=81846def349538d6dcd8d5efcc1c8d67ff0b4b41 commit 81846def349538d6dcd8d5efcc1c8d67ff0b4b41 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2020-12-27 21:50:54 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2020-12-27 22:01:44 +0000 vm: Fix some bugs in the page busying code In vm_page_busy_acquire(), load the object pointer using atomic_load_ptr() as we do elsewhere. Per the comment, the object identity must be consistent across sleeps. In vm_page_grab_sleep(), pass the correct pindex to _vm_page_busy_sleep(). The pindex is used to re-check the page's identity before going to sleep. In particular, vm_page_grab_sleep() is used in unlocked grab, so the object lock is not necessarily held when verifying the page's identity, and the pindex may change if the page is moved, or freed and re-allocated. I believe this can result in spurious VM_PAGER_FAILs from vm_page_grab_valid_unlocked() or early termination of vm_page_grab_pages_unlocked(). In vm_page_grab_pages(), pass the correct pindex to vm_page_grab_sleep(). Otherwise I believe vm_page_grab_pages() will effectively spin when attempting to busy a busy page after the first index in the range. Reviewed by: alc, kib Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D27607 --- sys/vm/vm_page.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c index cb5b5091ebb1..e668bbdc6178 100644 --- a/sys/vm/vm_page.c +++ b/sys/vm/vm_page.c @@ -882,7 +882,7 @@ vm_page_busy_acquire(vm_page_t m, int allocflags) * It is assumed that a reference to the object is already * held by the callers. */ - obj = m->object; + obj = atomic_load_ptr(&m->object); for (;;) { if (vm_page_tryacquire(m, allocflags)) return (true); @@ -4386,8 +4386,8 @@ vm_page_grab_sleep(vm_object_t object, vm_page_t m, vm_pindex_t pindex, if (locked && (allocflags & VM_ALLOC_NOCREAT) == 0) vm_page_reference(m); - if (_vm_page_busy_sleep(object, m, m->pindex, wmesg, allocflags, - locked) && locked) + if (_vm_page_busy_sleep(object, m, pindex, wmesg, allocflags, locked) && + locked) VM_OBJECT_WLOCK(object); if ((allocflags & VM_ALLOC_WAITFAIL) != 0) return (false); @@ -4780,7 +4780,7 @@ retrylookup: for (; i < count; i++) { if (m != NULL) { if (!vm_page_tryacquire(m, allocflags)) { - if (vm_page_grab_sleep(object, m, pindex, + if (vm_page_grab_sleep(object, m, pindex + i, "grbmaw", allocflags, true)) goto retrylookup; break;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202012272201.0BRM1tqm043360>