vm_object: remove the charge member

State that the object charge is zero if object->cred == NULL, or equal
to the ptoa(object->size) otherwise.

Besides being much simpler, the transition to use object->size corrects
the architectural issue with the use of object->charge.  The split
operations effectively carve the holes in the charged regions, but
single counter cannot properly express it.  As result, coalescing
anonymous mappings cannot calculate correctly if the extended mapping
already backed by the existing object is already accounted or not [1].

To properly solve the issue, either we need to start tracking exact
charged regions in the anonymous objects, which has the significant
overhead and complications.  Or give up on the slight over-accounting
and charge the whole object unconditionally, as it is done in the patch.

Reported by:	mmel, pho [1]
Reviewed by:	markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D54572

sys/dev/md/md.c

@@ -1617,7 +1617,6 @@ mdresize(struct md_s *sc, struct md_req *mdr)
 			    0, 0);
 			swap_release_by_cred(IDX_TO_OFF(oldpages -
 			    newpages), sc->cred);
-			sc->s_swap.object->charge = IDX_TO_OFF(newpages);
 			sc->s_swap.object->size = newpages;
 			VM_OBJECT_WUNLOCK(sc->s_swap.object);
 		} else if (newpages > oldpages) {
@@ -1637,7 +1636,6 @@ mdresize(struct md_s *sc, struct md_req *mdr)
 				}
 			}
 			VM_OBJECT_WLOCK(sc->s_swap.object);
-			sc->s_swap.object->charge = IDX_TO_OFF(newpages);
 			sc->s_swap.object->size = newpages;
 			VM_OBJECT_WUNLOCK(sc->s_swap.object);
 		}

sys/kern/uipc_shm.c

@@ -759,7 +759,6 @@ shm_dotruncate_locked(struct shmfd *shmfd, off_t length, void *rl_cookie)
 
 		/* Free the swap accounted for shm */
 		swap_release_by_cred(delta, object->cred);
-		object->charge -= delta;
 	} else {
 		if ((shmfd->shm_seals & F_SEAL_GROW) != 0)
 			return (EPERM);
@@ -768,7 +767,6 @@ shm_dotruncate_locked(struct shmfd *shmfd, off_t length, void *rl_cookie)
 		delta = IDX_TO_OFF(nobjsize - object->size);
 		if (!swap_reserve_by_cred(delta, object->cred))
 			return (ENOMEM);
-		object->charge += delta;
 	}
 	shmfd->shm_size = length;
 	mtx_lock(&shm_timestamp_lock);

sys/vm/swap_pager.c

@@ -784,10 +784,7 @@ swap_pager_init_object(vm_object_t object, void *handle, struct ucred *cred,
 
 	object->un_pager.swp.writemappings = 0;
 	object->handle = handle;
-	if (cred != NULL) {
-		object->cred = cred;
-		object->charge = size;
-	}
+	object->cred = cred;
 	return (true);
 }
 
@@ -900,8 +897,7 @@ swap_pager_dealloc(vm_object_t object)
 	 * Release the allocation charge.
 	 */
 	if (object->cred != NULL) {
-		swap_release_by_cred(object->charge, object->cred);
-		object->charge = 0;
+		swap_release_by_cred(ptoa(object->size), object->cred);
 		crfree(object->cred);
 		object->cred = NULL;
 	}

sys/vm/vm_fault.c

@@ -2310,13 +2310,12 @@ vm_fault_copy_entry(vm_map_t dst_map, vm_map_t src_map __unused,
 		 * directly.
 		 */
 		dst_object = vm_object_allocate_anon(atop(dst_entry->end -
-		    dst_entry->start), NULL, NULL, 0);
+		    dst_entry->start), NULL, NULL);
 #if VM_NRESERVLEVEL > 0
 		dst_object->flags |= OBJ_COLORED;
 		dst_object->pg_color = atop(dst_entry->start);
 #endif
 		dst_object->domain = src_object->domain;
-		dst_object->charge = dst_entry->end - dst_entry->start;
 
 		dst_entry->object.vm_object = dst_object;
 		dst_entry->offset = 0;
@@ -2329,7 +2328,7 @@ vm_fault_copy_entry(vm_map_t dst_map, vm_map_t src_map __unused,
 		    ("vm_fault_copy_entry: leaked swp charge"));
 		dst_object->cred = curthread->td_ucred;
 		crhold(dst_object->cred);
-		*fork_charge += dst_object->charge;
+		*fork_charge += ptoa(dst_object->size);
 	} else if ((dst_object->flags & OBJ_SWAP) != 0 &&
 	    dst_object->cred == NULL) {
 		KASSERT(dst_entry->cred != NULL, ("no cred for entry %p",

sys/vm/vm_map.c

@@ -2428,7 +2428,7 @@ vm_map_entry_back(vm_map_entry_t entry)
 	KASSERT((entry->eflags & MAP_ENTRY_IS_SUB_MAP) == 0,
 	    ("map entry %p is a submap", entry));
 	object = vm_object_allocate_anon(atop(entry->end - entry->start), NULL,
-	    entry->cred, entry->end - entry->start);
+	    entry->cred);
 	entry->object.vm_object = object;
 	entry->offset = 0;
 	entry->cred = NULL;
@@ -2443,21 +2443,26 @@ vm_map_entry_back(vm_map_entry_t entry)
 static inline void
 vm_map_entry_charge_object(vm_map_t map, vm_map_entry_t entry)
 {
+	vm_object_t object;
 
 	VM_MAP_ASSERT_LOCKED(map);
 	KASSERT((entry->eflags & MAP_ENTRY_IS_SUB_MAP) == 0,
 	    ("map entry %p is a submap", entry));
-	if (entry->object.vm_object == NULL && !vm_map_is_system(map) &&
+	object = entry->object.vm_object;
+	if (object == NULL && !vm_map_is_system(map) &&
 	    (entry->eflags & MAP_ENTRY_GUARD) == 0)
 		vm_map_entry_back(entry);
-	else if (entry->object.vm_object != NULL &&
+	else if (object != NULL &&
 	    ((entry->eflags & MAP_ENTRY_NEEDS_COPY) == 0) &&
 	    entry->cred != NULL) {
-		VM_OBJECT_WLOCK(entry->object.vm_object);
-		KASSERT(entry->object.vm_object->cred == NULL,
+		VM_OBJECT_WLOCK(object);
+		KASSERT(object->cred == NULL,
 		    ("OVERCOMMIT: %s: both cred e %p", __func__, entry));
-		entry->object.vm_object->cred = entry->cred;
-		entry->object.vm_object->charge = entry->end - entry->start;
+		object->cred = entry->cred;
+		if (entry->end - entry->start < ptoa(object->size)) {
+			swap_reserve_force_by_cred(ptoa(object->size) -
+			    entry->end + entry->start, object->cred);
+		}
 		VM_OBJECT_WUNLOCK(entry->object.vm_object);
 		entry->cred = NULL;
 	}
@@ -2956,7 +2961,7 @@ vm_map_protect(vm_map_t map, vm_offset_t start, vm_offset_t end,
 		 * we cannot distinguish between non-charged and
 		 * charged clipped mapping of the same object later.
 		 */
-		KASSERT(obj->charge == 0,
+		KASSERT(obj->cred == NULL,
 		    ("vm_map_protect: object %p overcharged (entry %p)",
 		    obj, entry));
 		if (!swap_reserve(ptoa(obj->size))) {
@@ -2968,7 +2973,6 @@ vm_map_protect(vm_map_t map, vm_offset_t start, vm_offset_t end,
 
 		crhold(cred);
 		obj->cred = cred;
-		obj->charge = ptoa(obj->size);
 		VM_OBJECT_WUNLOCK(obj);
 	}
 
@@ -3942,7 +3946,7 @@ static void
 vm_map_entry_delete(vm_map_t map, vm_map_entry_t entry)
 {
 	vm_object_t object;
-	vm_pindex_t offidxstart, offidxend, size1;
+	vm_pindex_t offidxstart, offidxend, oldsize;
 	vm_size_t size;
 
 	vm_map_entry_unlink(map, entry, UNLINK_MERGE_NONE);
@@ -3989,15 +3993,11 @@ vm_map_entry_delete(vm_map_t map, vm_map_entry_t entry)
 			    OBJPR_NOTMAPPED);
 			if (offidxend >= object->size &&
 			    offidxstart < object->size) {
-				size1 = object->size;
+				oldsize = object->size;
 				object->size = offidxstart;
 				if (object->cred != NULL) {
-					size1 -= object->size;
-					KASSERT(object->charge >= ptoa(size1),
-					    ("object %p charge < 0", object));
-					swap_release_by_cred(ptoa(size1),
-					    object->cred);
-					object->charge -= ptoa(size1);
+					swap_release_by_cred(ptoa(oldsize -
+					    ptoa(object->size)), object->cred);
 				}
 			}
 		}
@@ -4198,7 +4198,7 @@ vm_map_copy_swap_object(vm_map_entry_t src_entry, vm_map_entry_t dst_entry,
 		    ("OVERCOMMIT: vm_map_copy_anon_entry: cred %p",
 		     src_object));
 		src_object->cred = src_entry->cred;
-		src_object->charge = size;
+		*fork_charge += ptoa(src_object->size) - size;
 	}
 	dst_entry->object.vm_object = src_object;
 	if (charged) {
@@ -4455,7 +4455,7 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fork_charge)
 					KASSERT(object->cred == NULL,
 					    ("vmspace_fork both cred"));
 					object->cred = old_entry->cred;
-					object->charge = old_entry->end -
+					*fork_charge += old_entry->end -
 					    old_entry->start;
 					old_entry->cred = NULL;
 				}
@@ -5145,7 +5145,7 @@ vm_map_lookup(vm_map_t *var_map,		/* IN/OUT */
 		if (vm_map_lock_upgrade(map))
 			goto RetryLookup;
 		entry->object.vm_object = vm_object_allocate_anon(atop(size),
-		    NULL, entry->cred, size);
+		    NULL, entry->cred);
 		entry->offset = 0;
 		entry->cred = NULL;
 		vm_map_lock_downgrade(map);
@@ -5403,9 +5403,8 @@ vm_map_print(vm_map_t map)
 			    (void *)entry->object.vm_object,
 			    (uintmax_t)entry->offset);
 			if (entry->object.vm_object && entry->object.vm_object->cred)
-				db_printf(", obj ruid %d charge %jx",
-				    entry->object.vm_object->cred->cr_ruid,
-				    (uintmax_t)entry->object.vm_object->charge);
+				db_printf(", obj ruid %d ",
+				    entry->object.vm_object->cred->cr_ruid);
 			if (entry->eflags & MAP_ENTRY_COW)
 				db_printf(", copy (%s)",
 				    (entry->eflags & MAP_ENTRY_NEEDS_COPY) ? "needed" : "done");

sys/vm/vm_object.c

@@ -196,9 +196,9 @@ vm_object_zdtor(void *mem, int size, void *arg)
 	KASSERT(object->type == OBJT_DEAD,
 	    ("object %p has non-dead type %d",
 	    object, object->type));
-	KASSERT(object->charge == 0 && object->cred == NULL,
-	    ("object %p has non-zero charge %ju (%p)",
-	    object, (uintmax_t)object->charge, object->cred));
+	KASSERT(object->cred == NULL,
+	    ("object %p has non-zero charge cred %p",
+	    object, object->cred));
 }
 #endif
 
@@ -254,7 +254,6 @@ _vm_object_allocate(objtype_t type, vm_pindex_t size, u_short flags,
 	refcount_init(&object->ref_count, 1);
 	object->memattr = VM_MEMATTR_DEFAULT;
 	object->cred = NULL;
-	object->charge = 0;
 	object->handle = handle;
 	object->backing_object = NULL;
 	object->backing_object_offset = (vm_ooffset_t) 0;
@@ -452,7 +451,7 @@ vm_object_allocate_dyn(objtype_t dyntype, vm_pindex_t size, u_short flags)
  */
 vm_object_t
 vm_object_allocate_anon(vm_pindex_t size, vm_object_t backing_object,
-    struct ucred *cred, vm_size_t charge)
+    struct ucred *cred)
 {
 	vm_object_t handle, object;
 
@@ -466,7 +465,6 @@ vm_object_allocate_anon(vm_pindex_t size, vm_object_t backing_object,
 	_vm_object_allocate(OBJT_SWAP, size,
 	    OBJ_ANON | OBJ_ONEMAPPING | OBJ_SWAP, object, handle);
 	object->cred = cred;
-	object->charge = cred != NULL ? charge : 0;
 	return (object);
 }
 
@@ -1448,7 +1446,7 @@ vm_object_shadow(vm_object_t *object, vm_ooffset_t *offset, vm_size_t length,
 	/*
 	 * Allocate a new object with the given length.
 	 */
-	result = vm_object_allocate_anon(atop(length), source, cred, length);
+	result = vm_object_allocate_anon(atop(length), source, cred);
 
 	/*
 	 * Store the offset into the source object, and fix up the offset into
@@ -1511,6 +1509,7 @@ vm_object_split(vm_map_entry_t entry)
 	struct pctrie_iter pages;
 	vm_page_t m;
 	vm_object_t orig_object, new_object, backing_object;
+	struct ucred *cred;
 	vm_pindex_t offidxstart;
 	vm_size_t size;
 
@@ -1525,9 +1524,26 @@ vm_object_split(vm_map_entry_t entry)
 
 	offidxstart = OFF_TO_IDX(entry->offset);
 	size = atop(entry->end - entry->start);
+	if (orig_object->cred != NULL) {
+		/*
+		 * vm_object_split() is currently called from
+		 * vmspace_fork(), and it might be tempting to add the
+		 * charge for the split object to fork_charge.  But
+		 * fork_charge is discharged on error when the copied
+		 * vmspace is destroyed.  Since the split object is
+		 * inserted into the shadow hierarchy serving the
+		 * source vm_map, it is kept even after the
+		 * unsuccessful fork, meaning that we have to force
+		 * its swap usage.
+		 */
+		cred = curthread->td_ucred;
+		crhold(cred);
+		swap_reserve_force_by_cred(ptoa(size), cred);
+	} else {
+		cred = NULL;
+	}
 
-	new_object = vm_object_allocate_anon(size, orig_object,
-	    orig_object->cred, ptoa(size));
+	new_object = vm_object_allocate_anon(size, orig_object, cred);
 
 	/*
 	 * We must wait for the orig_object to complete any in-progress
@@ -1550,12 +1566,6 @@ vm_object_split(vm_map_entry_t entry)
 		new_object->backing_object_offset = 
 		    orig_object->backing_object_offset + entry->offset;
 	}
-	if (orig_object->cred != NULL) {
-		crhold(orig_object->cred);
-		KASSERT(orig_object->charge >= ptoa(size),
-		    ("orig_object->charge < 0"));
-		orig_object->charge -= ptoa(size);
-	}
 
 	/*
 	 * Mark the split operation so that swap_pager_getpages() knows
@@ -2233,7 +2243,6 @@ vm_object_coalesce(vm_object_t prev_object, vm_ooffset_t prev_offset,
 				swap_release_by_cred(ptoa(prev_object->size -
 				    next_pindex), prev_object->cred);
 			}
-			prev_object->charge += charge;
 		} else if ((cflags & OBJCO_CHARGED) != 0) {
 			/*
 			 * The caller charged, but the object has
@@ -2786,9 +2795,8 @@ DB_SHOW_COMMAND(object, vm_object_print_static)
 	db_iprintf("Object %p: type=%d, size=0x%jx, res=%d, ref=%d, flags=0x%x",
 	    object, (int)object->type, (uintmax_t)object->size,
 	    object->resident_page_count, object->ref_count, object->flags);
-	db_iprintf(" ruid %d charge %jx\n",
-	    object->cred ? object->cred->cr_ruid : -1,
-	    (uintmax_t)object->charge);
+	db_iprintf(" ruid %d\n",
+	    object->cred ? object->cred->cr_ruid : -1);
 	db_iprintf(" sref=%d, backing_object(%d)=(%p)+0x%jx\n",
 	    atomic_load_int(&object->shadow_count),
 	    object->backing_object ? object->backing_object->ref_count : 0,

sys/vm/vm_object.h

@@ -175,7 +175,6 @@ struct vm_object {
 		} phys;
 	} un_pager;
 	struct ucred *cred;
-	vm_ooffset_t charge;
 	void *umtx_data;
 };
 
@@ -356,8 +355,7 @@ void umtx_shm_object_terminated(vm_object_t object);
 extern int umtx_shm_vnobj_persistent;
 
 vm_object_t vm_object_allocate (objtype_t, vm_pindex_t);
-vm_object_t vm_object_allocate_anon(vm_pindex_t, vm_object_t, struct ucred *,
-   vm_size_t);
+vm_object_t vm_object_allocate_anon(vm_pindex_t, vm_object_t, struct ucred *);
 vm_object_t vm_object_allocate_dyn(objtype_t, vm_pindex_t, u_short);
 boolean_t vm_object_coalesce(vm_object_t, vm_ooffset_t, vm_size_t, vm_size_t,
    int);