Author: kib
Date: Mon Jun 13 03:42:46 2016
New Revision: 301852
URL: https://svnweb.freebsd.org/changeset/base/301852
Log:
Fix inconsistent locking of the swap pager named objects list.
Right now, all modifications of the list are locked by sw_alloc_mtx.
But initial lookup of the object by the handle in swap_pager_alloc()
is not protected by sw_alloc_mtx, which means that
vm_pager_object_lookup() could follow freed pointer.
Create a new named swap object with the OBJT_SWAP type, instead
of OBJT_DEFAULT. With this change, swp_pager_meta_build() never need
to upgrade named OBJT_DEFAULT to OBJT_SWAP (in the other place, we do
not forbid for client code to create named OBJT_DEFAULT objects at
all).
That change allows to remove sw_alloc_mtx and make the list locked by
sw_alloc_sx lock. Update swap_pager_copy() to new locking mode.
Create helper swap_pager_alloc_init() to consolidate named and
anonymous swap objects creation, while a caller ensures that the
neccesary locks are held around the helper.
Reviewed by: alc
Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks
Approved by: re (hrs)
Modified:
head/sys/vm/swap_pager.c
Modified: head/sys/vm/swap_pager.c
==============================================================================
--- head/sys/vm/swap_pager.c Mon Jun 13 03:39:16 2016 (r301851)
+++ head/sys/vm/swap_pager.c Mon Jun 13 03:42:46 2016 (r301852)
@@ -345,7 +345,6 @@ static struct sx sw_alloc_sx;
#define NOBJLIST(handle) \
(&swap_pager_object_list[((int)(intptr_t)handle >> 4) & (NOBJLISTS-1)])
-static struct mtx sw_alloc_mtx; /* protect list manipulation */
static struct pagerlst swap_pager_object_list[NOBJLISTS];
static uma_zone_t swap_zone;
@@ -486,7 +485,6 @@ swap_pager_init(void)
for (i = 0; i < NOBJLISTS; ++i)
TAILQ_INIT(&swap_pager_object_list[i]);
- mtx_init(&sw_alloc_mtx, "swap_pager list", NULL, MTX_DEF);
mtx_init(&sw_dev_mtx, "swapdev", NULL, MTX_DEF);
sx_init(&sw_alloc_sx, "swspsx");
sx_init(&swdev_syscall_lock, "swsysc");
@@ -583,28 +581,46 @@ swap_pager_swap_init(void)
mtx_init(&swhash_mtx, "swap_pager swhash", NULL, MTX_DEF);
}
+static vm_object_t
+swap_pager_alloc_init(void *handle, struct ucred *cred, vm_ooffset_t size,
+ vm_ooffset_t offset)
+{
+ vm_object_t object;
+
+ if (cred != NULL) {
+ if (!swap_reserve_by_cred(size, cred))
+ return (NULL);
+ crhold(cred);
+ }
+ object = vm_object_allocate(OBJT_SWAP, OFF_TO_IDX(offset +
+ PAGE_MASK + size));
+ object->handle = handle;
+ if (cred != NULL) {
+ object->cred = cred;
+ object->charge = size;
+ }
+ object->un_pager.swp.swp_bcount = 0;
+ return (object);
+}
+
/*
* SWAP_PAGER_ALLOC() - allocate a new OBJT_SWAP VM object and
instantiate
* its metadata structures.
*
* This routine is called from the mmap and fork code to create a new
- * OBJT_SWAP object. We do this by creating an OBJT_DEFAULT object
- * and then converting it with swp_pager_meta_build().
- *
- * This routine may block in vm_object_allocate() and create a named
- * object lookup race, so we must interlock.
+ * OBJT_SWAP object.
*
- * MPSAFE
+ * This routine must ensure that no live duplicate is created for
+ * the named object request, which is protected against by
+ * holding the sw_alloc_sx lock in case handle != NULL.
*/
static vm_object_t
swap_pager_alloc(void *handle, vm_ooffset_t size, vm_prot_t prot,
vm_ooffset_t offset, struct ucred *cred)
{
vm_object_t object;
- vm_pindex_t pindex;
- pindex = OFF_TO_IDX(offset + PAGE_MASK + size);
- if (handle) {
+ if (handle != NULL) {
/*
* Reference existing named region or allocate new one. There
* should not be a race here against swp_pager_meta_build()
@@ -614,38 +630,16 @@ swap_pager_alloc(void *handle, vm_ooffse
sx_xlock(&sw_alloc_sx);
object = vm_pager_object_lookup(NOBJLIST(handle), handle);
if (object == NULL) {
- if (cred != NULL) {
- if (!swap_reserve_by_cred(size, cred)) {
- sx_xunlock(&sw_alloc_sx);
- return (NULL);
- }
- crhold(cred);
- }
- object = vm_object_allocate(OBJT_DEFAULT, pindex);
- VM_OBJECT_WLOCK(object);
- object->handle = handle;
- if (cred != NULL) {
- object->cred = cred;
- object->charge = size;
+ object = swap_pager_alloc_init(handle, cred, size,
+ offset);
+ if (object != NULL) {
+ TAILQ_INSERT_TAIL(NOBJLIST(object->handle),
+ object, pager_object_list);
}
- swp_pager_meta_build(object, 0, SWAPBLK_NONE);
- VM_OBJECT_WUNLOCK(object);
}
sx_xunlock(&sw_alloc_sx);
} else {
- if (cred != NULL) {
- if (!swap_reserve_by_cred(size, cred))
- return (NULL);
- crhold(cred);
- }
- object = vm_object_allocate(OBJT_DEFAULT, pindex);
- VM_OBJECT_WLOCK(object);
- if (cred != NULL) {
- object->cred = cred;
- object->charge = size;
- }
- swp_pager_meta_build(object, 0, SWAPBLK_NONE);
- VM_OBJECT_WUNLOCK(object);
+ object = swap_pager_alloc_init(handle, cred, size, offset);
}
return (object);
}
@@ -664,17 +658,22 @@ static void
swap_pager_dealloc(vm_object_t object)
{
+ VM_OBJECT_ASSERT_WLOCKED(object);
+ KASSERT((object->flags & OBJ_DEAD) != 0, ("dealloc of reachable obj"));
+
/*
* Remove from list right away so lookups will fail if we block for
* pageout completion.
*/
if (object->handle != NULL) {
- mtx_lock(&sw_alloc_mtx);
- TAILQ_REMOVE(NOBJLIST(object->handle), object,
pager_object_list);
- mtx_unlock(&sw_alloc_mtx);
+ VM_OBJECT_WUNLOCK(object);
+ sx_xlock(&sw_alloc_sx);
+ TAILQ_REMOVE(NOBJLIST(object->handle), object,
+ pager_object_list);
+ sx_xunlock(&sw_alloc_sx);
+ VM_OBJECT_WLOCK(object);
}
- VM_OBJECT_ASSERT_WLOCKED(object);
vm_object_pip_wait(object, "swpdea");
/*
@@ -901,16 +900,19 @@ swap_pager_copy(vm_object_t srcobject, v
* If destroysource is set, we remove the source object from the
* swap_pager internal queue now.
*/
- if (destroysource) {
- if (srcobject->handle != NULL) {
- mtx_lock(&sw_alloc_mtx);
- TAILQ_REMOVE(
- NOBJLIST(srcobject->handle),
- srcobject,
- pager_object_list
- );
- mtx_unlock(&sw_alloc_mtx);
- }
+ if (destroysource && srcobject->handle != NULL) {
+ vm_object_pip_add(srcobject, 1);
+ VM_OBJECT_WUNLOCK(srcobject);
+ vm_object_pip_add(dstobject, 1);
+ VM_OBJECT_WUNLOCK(dstobject);
+ sx_xlock(&sw_alloc_sx);
+ TAILQ_REMOVE(NOBJLIST(srcobject->handle), srcobject,
+ pager_object_list);
+ sx_xunlock(&sw_alloc_sx);
+ VM_OBJECT_WLOCK(dstobject);
+ vm_object_pip_wakeup(dstobject);
+ VM_OBJECT_WLOCK(srcobject);
+ vm_object_pip_wakeup(srcobject);
}
/*
@@ -1746,16 +1748,7 @@ swp_pager_meta_build(vm_object_t object,
if (object->type != OBJT_SWAP) {
object->type = OBJT_SWAP;
object->un_pager.swp.swp_bcount = 0;
-
- if (object->handle != NULL) {
- mtx_lock(&sw_alloc_mtx);
- TAILQ_INSERT_TAIL(
- NOBJLIST(object->handle),
- object,
- pager_object_list
- );
- mtx_unlock(&sw_alloc_mtx);
- }
+ KASSERT(object->handle == NULL, ("default pager with handle"));
}
/*
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
