svn commit: r366838 - head/sys/kern

Mark Johnston markj at FreeBSD.org
Mon Oct 19 16:54:07 UTC 2020


Author: markj
Date: Mon Oct 19 16:54:06 2020
New Revision: 366838
URL: https://svnweb.freebsd.org/changeset/base/366838

Log:
  vmem: Allocate btags before looping in vmem_xalloc()
  
  BT_MAXALLOC (4) is the number of boundary tags required to complete an
  allocation in the worst case: two to clip a free segment, and two to
  import from a parent arena.  vmem_xalloc() preallocates four boundary
  tags before attempting a search to simplify the segment allocation code.
  It implements a loop that:
  1) ensures that BT_MAXALLOC boundary tags are available,
  2) attempts to find and clip a free segment satisfying the allocation
     constraints, and failing that,
  3) attempts to import a segment.
  
  On !UMA_MD_SMALL_ALLOC platforms the btag zone has to handle recusion:
  it needs boundary tags to allocate boundary tags.  Thus we reserve
  2 * BT_MAXALLOC * mp_ncpus tags for use when recursing: the factor of 2
  is because there are two layers of vmem arenas, the per-domain arena and
  global arena.  For a single thread, 2 * BT_MAXALLOC tags should be
  sufficient.
  
  Because of the way the loop is structured, BT_MAXALLOC tags are not
  sufficient.  The first bt_fill() call may allocate BT_MAXALLOC tags,
  then import a segment (consuming two tags), then attempt to top up the
  preallocation before carving into the imported free segment, thus
  requiring up to six tags in the worst case.  Because we don't
  preallocate that many, this bug can cause deadlocks in rare scenarios.
  
  Fix the problem by moving the preallocation out the loop.  This assumes
  that only a single import is ever required to satisfy an allocation
  request.
  
  Thanks to manu, emaste and lwhsu for helping test debug patches.
  
  Reported by:	Jenkins (hardware CI lab)
  Reviewed by:	alc, kib, rlibby
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D26770

Modified:
  head/sys/kern/subr_vmem.c

Modified: head/sys/kern/subr_vmem.c
==============================================================================
--- head/sys/kern/subr_vmem.c	Mon Oct 19 16:52:27 2020	(r366837)
+++ head/sys/kern/subr_vmem.c	Mon Oct 19 16:54:06 2020	(r366838)
@@ -368,6 +368,24 @@ bt_free(vmem_t *vm, bt_t *bt)
 }
 
 /*
+ * Hide MAXALLOC tags before dropping the arena lock to ensure that a
+ * concurrent allocation attempt does not grab them.
+ */
+static void
+bt_save(vmem_t *vm)
+{
+	KASSERT(vm->vm_nfreetags >= BT_MAXALLOC,
+	    ("%s: insufficient free tags %d", __func__, vm->vm_nfreetags));
+	vm->vm_nfreetags -= BT_MAXALLOC;
+}
+
+static void
+bt_restore(vmem_t *vm)
+{
+	vm->vm_nfreetags += BT_MAXALLOC;
+}
+
+/*
  * freelist[0] ... [1, 1]
  * freelist[1] ... [2, 2]
  *  :
@@ -911,16 +929,11 @@ vmem_import(vmem_t *vm, vmem_size_t size, vmem_size_t 
 	if (vm->vm_limit != 0 && vm->vm_limit < vm->vm_size + size)
 		return (ENOMEM);
 
-	/*
-	 * Hide MAXALLOC tags so we're guaranteed to be able to add this
-	 * span and the tag we want to allocate from it.
-	 */
-	MPASS(vm->vm_nfreetags >= BT_MAXALLOC);
-	vm->vm_nfreetags -= BT_MAXALLOC;
+	bt_save(vm);
 	VMEM_UNLOCK(vm);
 	error = (vm->vm_importfn)(vm->vm_arg, size, flags, &addr);
 	VMEM_LOCK(vm);
-	vm->vm_nfreetags += BT_MAXALLOC;
+	bt_restore(vm);
 	if (error)
 		return (ENOMEM);
 
@@ -1048,19 +1061,23 @@ vmem_try_fetch(vmem_t *vm, const vmem_size_t size, vme
 	 */
 	if (vm->vm_qcache_max != 0 || vm->vm_reclaimfn != NULL) {
 		avail = vm->vm_size - vm->vm_inuse;
+		bt_save(vm);
 		VMEM_UNLOCK(vm);
 		if (vm->vm_qcache_max != 0)
 			qc_drain(vm);
 		if (vm->vm_reclaimfn != NULL)
 			vm->vm_reclaimfn(vm, flags);
 		VMEM_LOCK(vm);
+		bt_restore(vm);
 		/* If we were successful retry even NOWAIT. */
 		if (vm->vm_size - vm->vm_inuse > avail)
 			return (1);
 	}
 	if ((flags & M_NOWAIT) != 0)
 		return (0);
+	bt_save(vm);
 	VMEM_CONDVAR_WAIT(vm);
+	bt_restore(vm);
 	return (1);
 }
 
@@ -1108,13 +1125,14 @@ vmem_xalloc_nextfit(vmem_t *vm, const vmem_size_t size
 
 	error = ENOMEM;
 	VMEM_LOCK(vm);
-retry:
+
 	/*
 	 * Make sure we have enough tags to complete the operation.
 	 */
 	if (bt_fill(vm, flags) != 0)
 		goto out;
 
+retry:
 	/*
 	 * Find the next free tag meeting our constraints.  If one is found,
 	 * perform the allocation.
@@ -1390,15 +1408,14 @@ vmem_xalloc(vmem_t *vm, const vmem_size_t size0, vmem_
 	 */
 	first = bt_freehead_toalloc(vm, size, strat);
 	VMEM_LOCK(vm);
-	for (;;) {
-		/*
-		 * Make sure we have enough tags to complete the
-		 * operation.
-		 */
-		error = bt_fill(vm, flags);
-		if (error != 0)
-			break;
 
+	/*
+	 * Make sure we have enough tags to complete the operation.
+	 */
+	error = bt_fill(vm, flags);
+	if (error != 0)
+		goto out;
+	for (;;) {
 		/*
 	 	 * Scan freelists looking for a tag that satisfies the
 		 * allocation.  If we're doing BESTFIT we may encounter


More information about the svn-src-all mailing list