d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a race in vm_page_busy_sleep(9).

Browse Source 
Suppose that we have an exclusively busy page, and a thread which can
accept shared-busy page.  In this case, typical code waiting for the
page xbusy state to pass is
again:
	VM_OBJECT_WLOCK(object);
	...
	if (vm_page_xbusied(m)) {
		vm_page_lock(m);
 		VM_OBJECT_WUNLOCK(object);    <---1
		vm_page_busy_sleep(p, "vmopax");
 		goto again;
	}

Suppose that the xbusy state owner locked the object, unbusied the
page and unlocked the object after we are at the line [1], but before we
executed the load of the busy_lock word in vm_page_busy_sleep().  If it
happens that there is still no waiters recorded for the busy state,
the xbusy owner did not acquired the page lock, so it proceeded.

More, suppose that some other thread happen to share-busy the page
after xbusy state was relinquished but before the m->busy_lock is read
in vm_page_busy_sleep().  Again, that thread only needs vm_object lock
to proceed.  Then, vm_page_busy_sleep() reads busy_lock value equal to
the VPB_SHARERS_WORD(1).

In this case, all tests in vm_page_busy_sleep(9) pass and we are going
to sleep, despite the page being share-busied.

Update check for m->busy_lock == VPB_UNBUSIED in vm_page_busy_sleep(9)
to also accept shared-busy state if we only wait for the xbusy state to
pass.

Merge sequential if()s with the same 'then' clause in
vm_page_busy_sleep().

Note that the current code does not share-busy pages from parallel
threads, the only way to have more that one sbusy owner is right now
is to recurse.

Reported and tested by:	pho (previous version)
Reviewed by:	alc, markj
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D8196
This commit is contained in:
Konstantin Belousov 2016-10-13 14:41:05 +00:00
parent 859422cc12
commit 5975e53d40
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=307218
7 changed files with 23 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
View File

@ -421,7 +421,7 @@ page_busy(vnode_t *vp, int64_t start, int64_t off, int64_t nbytes)
vm_page_reference(pp);
vm_page_lock(pp);
zfs_vmobject_wunlock(obj);
vm_page_busy_sleep(pp, "zfsmwb");
vm_page_busy_sleep(pp, "zfsmwb", true);
zfs_vmobject_wlock(obj);
continue;
}
@ -476,7 +476,7 @@ page_hold(vnode_t *vp, int64_t start)
vm_page_reference(pp);
vm_page_lock(pp);
zfs_vmobject_wunlock(obj);
vm_page_busy_sleep(pp, "zfsmwb");
vm_page_busy_sleep(pp, "zfsmwb", true);
zfs_vmobject_wlock(obj);
continue;
}

4
sys/dev/drm2/i915/i915_gem.c
View File

@ -1533,7 +1533,7 @@ i915_gem_pager_fault(vm_object_t vm_obj, vm_ooffset_t offset, int prot,
DRM_UNLOCK(dev);
vm_page_lock(page);
VM_OBJECT_WUNLOCK(vm_obj);
vm_page_busy_sleep(page, "915pee");
vm_page_busy_sleep(page, "915pee", false);
goto retry;
}
goto have_page;
@ -1575,7 +1575,7 @@ i915_gem_pager_fault(vm_object_t vm_obj, vm_ooffset_t offset, int prot,
DRM_UNLOCK(dev);
vm_page_lock(page);
VM_OBJECT_WUNLOCK(vm_obj);
vm_page_busy_sleep(page, "915pbs");
vm_page_busy_sleep(page, "915pbs", false);
goto retry;
}
if (vm_page_insert(page, vm_obj, OFF_TO_IDX(offset))) {

2
sys/dev/drm2/ttm/ttm_bo_vm.c
View File

@ -236,7 +236,7 @@ ttm_bo_vm_fault(vm_object_t vm_obj, vm_ooffset_t offset,
if (vm_page_busied(m)) {
vm_page_lock(m);
VM_OBJECT_WUNLOCK(vm_obj);
vm_page_busy_sleep(m, "ttmpbs");
vm_page_busy_sleep(m, "ttmpbs", false);
VM_OBJECT_WLOCK(vm_obj);
ttm_mem_io_unlock(man);
ttm_bo_unreserve(bo);

4
sys/kern/vfs_bio.c
View File

@ -2633,7 +2633,7 @@ vfs_vmio_invalidate(struct buf *bp)
while (vm_page_xbusied(m)) {
vm_page_lock(m);
VM_OBJECT_WUNLOCK(obj);
vm_page_busy_sleep(m, "mbncsh");
vm_page_busy_sleep(m, "mbncsh", true);
VM_OBJECT_WLOCK(obj);
}
if (pmap_page_wired_mappings(m) == 0)
@ -4182,7 +4182,7 @@ vfs_drain_busy_pages(struct buf *bp)
while (vm_page_xbusied(m)) {
vm_page_lock(m);
VM_OBJECT_WUNLOCK(bp->b_bufobj->bo_object);
vm_page_busy_sleep(m, "vbpage");
vm_page_busy_sleep(m, "vbpage", true);
VM_OBJECT_WLOCK(bp->b_bufobj->bo_object);
}
}

10
sys/vm/vm_object.c
View File

@ -1186,7 +1186,7 @@ vm_object_madvise(vm_object_t object, vm_pindex_t pindex, vm_pindex_t end,
if (object != tobject)
VM_OBJECT_WUNLOCK(object);
VM_OBJECT_WUNLOCK(tobject);
vm_page_busy_sleep(m, "madvpo");
vm_page_busy_sleep(m, "madvpo", false);
VM_OBJECT_WLOCK(object);
goto relookup;
}
@ -1365,7 +1365,7 @@ vm_object_split(vm_map_entry_t entry)
VM_OBJECT_WUNLOCK(new_object);
vm_page_lock(m);
VM_OBJECT_WUNLOCK(orig_object);
vm_page_busy_sleep(m, "spltwt");
vm_page_busy_sleep(m, "spltwt", false);
VM_OBJECT_WLOCK(orig_object);
VM_OBJECT_WLOCK(new_object);
goto retry;
@ -1453,7 +1453,7 @@ vm_object_collapse_scan_wait(vm_object_t object, vm_page_t p, vm_page_t next,
if (p == NULL)
VM_WAIT;
else
vm_page_busy_sleep(p, "vmocol");
vm_page_busy_sleep(p, "vmocol", false);
VM_OBJECT_WLOCK(object);
VM_OBJECT_WLOCK(backing_object);
return (TAILQ_FIRST(&backing_object->memq));
@ -1912,7 +1912,7 @@ vm_object_page_remove(vm_object_t object, vm_pindex_t start, vm_pindex_t end,
vm_page_lock(p);
if (vm_page_xbusied(p)) {
VM_OBJECT_WUNLOCK(object);
vm_page_busy_sleep(p, "vmopax");
vm_page_busy_sleep(p, "vmopax", true);
VM_OBJECT_WLOCK(object);
goto again;
}
@ -1927,7 +1927,7 @@ vm_object_page_remove(vm_object_t object, vm_pindex_t start, vm_pindex_t end,
}
if (vm_page_busied(p)) {
VM_OBJECT_WUNLOCK(object);
vm_page_busy_sleep(p, "vmopar");
vm_page_busy_sleep(p, "vmopar", false);
VM_OBJECT_WLOCK(object);
goto again;
}

20
sys/vm/vm_page.c
View File

@ -741,21 +741,20 @@ vm_page_sunbusy(vm_page_t m)
* This is used to implement the hard-path of busying mechanism.
*
* The given page must be locked.
*
* If nonshared is true, sleep only if the page is xbusy.
*/
void
vm_page_busy_sleep(vm_page_t m, const char *wmesg)
vm_page_busy_sleep(vm_page_t m, const char *wmesg, bool nonshared)
{
u_int x;
vm_page_lock_assert(m, MA_OWNED);
vm_page_assert_locked(m);
x = m->busy_lock;
if (x == VPB_UNBUSIED) {
vm_page_unlock(m);
return;
}
if ((x & VPB_BIT_WAITERS) == 0 &&
!atomic_cmpset_int(&m->busy_lock, x, x | VPB_BIT_WAITERS)) {
if (x == VPB_UNBUSIED || (nonshared && (x & VPB_BIT_SHARED) != 0) ||
((x & VPB_BIT_WAITERS) == 0 &&
!atomic_cmpset_int(&m->busy_lock, x, x | VPB_BIT_WAITERS))) {
vm_page_unlock(m);
return;
}
@ -1092,7 +1091,7 @@ vm_page_sleep_if_busy(vm_page_t m, const char *msg)
obj = m->object;
vm_page_lock(m);
VM_OBJECT_WUNLOCK(obj);
vm_page_busy_sleep(m, msg);
vm_page_busy_sleep(m, msg, false);
VM_OBJECT_WLOCK(obj);
return (TRUE);
}
@ -3455,7 +3454,8 @@ vm_page_grab(vm_object_t object, vm_pindex_t pindex, int allocflags)
vm_page_aflag_set(m, PGA_REFERENCED);
vm_page_lock(m);
VM_OBJECT_WUNLOCK(object);
vm_page_busy_sleep(m, "pgrbwt");
vm_page_busy_sleep(m, "pgrbwt", (allocflags &
VM_ALLOC_IGN_SBUSY) != 0);
VM_OBJECT_WLOCK(object);
goto retrylookup;
} else {

2
sys/vm/vm_page.h
View File

@ -436,7 +436,7 @@ malloc2vm_flags(int malloc_flags)
#endif
void vm_page_busy_downgrade(vm_page_t m);
void vm_page_busy_sleep(vm_page_t m, const char *msg);
void vm_page_busy_sleep(vm_page_t m, const char *msg, bool nonshared);
void vm_page_flash(vm_page_t m);
void vm_page_hold(vm_page_t mem);
void vm_page_unhold(vm_page_t mem);