svn commit: r355121 - head/sys/vm

Jeff Roberson jeff at FreeBSD.org
Tue Nov 26 22:17:03 UTC 2019


Author: jeff
Date: Tue Nov 26 22:17:02 2019
New Revision: 355121
URL: https://svnweb.freebsd.org/changeset/base/355121

Log:
  Refactor uma_zalloc_arg().  It is a mess of gotos and code which doesn't
  make sense after many partial refactors.  Attempt to make a smaller cache
  footprint for the fast path.
  
  Reviewed by:	markj, rlibby
  Differential Revision:	https://reviews.freebsd.org/D22470

Modified:
  head/sys/vm/uma_core.c

Modified: head/sys/vm/uma_core.c
==============================================================================
--- head/sys/vm/uma_core.c	Tue Nov 26 22:01:09 2019	(r355120)
+++ head/sys/vm/uma_core.c	Tue Nov 26 22:17:02 2019	(r355121)
@@ -273,7 +273,7 @@ static void bucket_init(void);
 static uma_bucket_t bucket_alloc(uma_zone_t zone, void *, int);
 static void bucket_free(uma_zone_t zone, uma_bucket_t, void *);
 static void bucket_zone_drain(void);
-static uma_bucket_t zone_alloc_bucket(uma_zone_t, void *, int, int, int);
+static uma_bucket_t zone_alloc_bucket(uma_zone_t, void *, int, int);
 static uma_slab_t zone_fetch_slab(uma_zone_t, uma_keg_t, int, int);
 static void *slab_alloc_item(uma_keg_t keg, uma_slab_t slab);
 static void slab_free_item(uma_zone_t zone, uma_slab_t slab, void *item);
@@ -282,6 +282,7 @@ static uma_keg_t uma_kcreate(uma_zone_t zone, size_t s
 static int zone_import(uma_zone_t, void **, int, int, int);
 static void zone_release(uma_zone_t, void **, int);
 static void uma_zero_item(void *, uma_zone_t);
+static bool cache_alloc(uma_zone_t, uma_cache_t, void *, int);
 
 void uma_print_zone(uma_zone_t);
 void uma_print_stats(void);
@@ -2441,18 +2442,58 @@ uma_zfree_pcpu_arg(uma_zone_t zone, void *item, void *
 	uma_zfree_arg(zone, item, udata);
 }
 
+static inline void *
+bucket_pop(uma_zone_t zone, uma_cache_t cache, uma_bucket_t bucket)
+{
+	void *item;
+
+	bucket->ub_cnt--;
+	item = bucket->ub_bucket[bucket->ub_cnt];
+#ifdef INVARIANTS
+	bucket->ub_bucket[bucket->ub_cnt] = NULL;
+	KASSERT(item != NULL, ("uma_zalloc: Bucket pointer mangled."));
+#endif
+	cache->uc_allocs++;
+
+	return (item);
+}
+
+static void *
+item_ctor(uma_zone_t zone, void *udata, int flags, void *item)
+{
+#ifdef INVARIANTS
+	int skipdbg;
+
+	skipdbg = uma_dbg_zskip(zone, item);
+	if (zone->uz_ctor != NULL &&
+	    (!skipdbg || zone->uz_ctor != trash_ctor ||
+	    zone->uz_dtor != trash_dtor) &&
+#else
+	if (__predict_false(zone->uz_ctor != NULL) &&
+#endif
+	    zone->uz_ctor(item, zone->uz_size, udata, flags) != 0) {
+		counter_u64_add(zone->uz_fails, 1);
+		zone_free_item(zone, item, udata, SKIP_DTOR | SKIP_CNT);
+		return (NULL);
+	}
+#ifdef INVARIANTS
+	if (!skipdbg)
+		uma_dbg_alloc(zone, NULL, item);
+#endif
+	if (flags & M_ZERO)
+		uma_zero_item(item, zone);
+
+	return (item);
+}
+
 /* See uma.h */
 void *
 uma_zalloc_arg(uma_zone_t zone, void *udata, int flags)
 {
-	uma_zone_domain_t zdom;
 	uma_bucket_t bucket;
 	uma_cache_t cache;
 	void *item;
-	int cpu, domain, lockfail, maxbucket;
-#ifdef INVARIANTS
-	bool skipdbg;
-#endif
+	int cpu, domain;
 
 	/* Enable entropy collection for RANDOM_ENABLE_UMA kernel option */
 	random_harvest_fast_uma(&zone, sizeof(zone), RANDOM_UMA);
@@ -2501,56 +2542,55 @@ uma_zalloc_arg(uma_zone_t zone, void *udata, int flags
 	 * the current cache; when we re-acquire the critical section, we
 	 * must detect and handle migration if it has occurred.
 	 */
-zalloc_restart:
 	critical_enter();
-	cpu = curcpu;
-	cache = &zone->uz_cpu[cpu];
-
-zalloc_start:
-	bucket = cache->uc_allocbucket;
-	if (bucket != NULL && bucket->ub_cnt > 0) {
-		bucket->ub_cnt--;
-		item = bucket->ub_bucket[bucket->ub_cnt];
-#ifdef INVARIANTS
-		bucket->ub_bucket[bucket->ub_cnt] = NULL;
-#endif
-		KASSERT(item != NULL, ("uma_zalloc: Bucket pointer mangled."));
-		cache->uc_allocs++;
-		critical_exit();
-#ifdef INVARIANTS
-		skipdbg = uma_dbg_zskip(zone, item);
-#endif
-		if (zone->uz_ctor != NULL &&
-#ifdef INVARIANTS
-		    (!skipdbg || zone->uz_ctor != trash_ctor ||
-		    zone->uz_dtor != trash_dtor) &&
-#endif
-		    zone->uz_ctor(item, zone->uz_size, udata, flags) != 0) {
-			counter_u64_add(zone->uz_fails, 1);
-			zone_free_item(zone, item, udata, SKIP_DTOR | SKIP_CNT);
-			return (NULL);
+	do {
+		cpu = curcpu;
+		cache = &zone->uz_cpu[cpu];
+		bucket = cache->uc_allocbucket;
+		if (__predict_true(bucket != NULL && bucket->ub_cnt != 0)) {
+			item = bucket_pop(zone, cache, bucket);
+			critical_exit();
+			return (item_ctor(zone, udata, flags, item));
 		}
-#ifdef INVARIANTS
-		if (!skipdbg)
-			uma_dbg_alloc(zone, NULL, item);
-#endif
-		if (flags & M_ZERO)
-			uma_zero_item(item, zone);
-		return (item);
-	}
+	} while (cache_alloc(zone, cache, udata, flags));
+	critical_exit();
 
 	/*
-	 * We have run out of items in our alloc bucket.
-	 * See if we can switch with our free bucket.
+	 * We can not get a bucket so try to return a single item.
 	 */
+	if (zone->uz_flags & UMA_ZONE_NUMA)
+		domain = PCPU_GET(domain);
+	else
+		domain = UMA_ANYDOMAIN;
+	return (zone_alloc_item_locked(zone, udata, domain, flags));
+}
+
+/*
+ * Replenish an alloc bucket and possibly restore an old one.  Called in
+ * a critical section.  Returns in a critical section.
+ *
+ * A false return value indicates failure and returns with the zone lock
+ * held.  A true return value indicates success and the caller should retry.
+ */
+static __noinline bool
+cache_alloc(uma_zone_t zone, uma_cache_t cache, void *udata, int flags)
+{
+	uma_zone_domain_t zdom;
+	uma_bucket_t bucket;
+	int cpu, domain;
+	bool lockfail;
+
+	CRITICAL_ASSERT(curthread);
+
+	/*
+	 * If we have run out of items in our alloc bucket see
+	 * if we can switch with the free bucket.
+	 */
 	bucket = cache->uc_freebucket;
-	if (bucket != NULL && bucket->ub_cnt > 0) {
-		CTR2(KTR_UMA,
-		    "uma_zalloc: zone %s(%p) swapping empty with alloc",
-		    zone->uz_name, zone);
+	if (bucket != NULL && bucket->ub_cnt != 0) {
 		cache->uc_freebucket = cache->uc_allocbucket;
 		cache->uc_allocbucket = bucket;
-		goto zalloc_start;
+		return (true);
 	}
 
 	/*
@@ -2562,16 +2602,6 @@ zalloc_start:
 	if (bucket != NULL)
 		bucket_free(zone, bucket, udata);
 
-	/* Short-circuit for zones without buckets and low memory. */
-	if (zone->uz_count == 0 || bucketdisable) {
-		ZONE_LOCK(zone);
-		if (zone->uz_flags & UMA_ZONE_NUMA)
-			domain = PCPU_GET(domain);
-		else
-			domain = UMA_ANYDOMAIN;
-		goto zalloc_item;
-	}
-
 	/*
 	 * Attempt to retrieve the item from the per-CPU cache has failed, so
 	 * we must go back to the zone.  This requires the zone lock, so we
@@ -2587,14 +2617,19 @@ zalloc_start:
 		ZONE_LOCK(zone);
 		lockfail = 1;
 	}
+
 	critical_enter();
+	/* Short-circuit for zones without buckets and low memory. */
+	if (zone->uz_count == 0 || bucketdisable)
+		return (false);
+
 	cpu = curcpu;
 	cache = &zone->uz_cpu[cpu];
 
 	/* See if we lost the race to fill the cache. */
 	if (cache->uc_allocbucket != NULL) {
 		ZONE_UNLOCK(zone);
-		goto zalloc_start;
+		return (true);
 	}
 
 	/*
@@ -2609,11 +2644,11 @@ zalloc_start:
 	}
 
 	if ((bucket = zone_fetch_bucket(zone, zdom)) != NULL) {
+		ZONE_UNLOCK(zone);
 		KASSERT(bucket->ub_cnt != 0,
 		    ("uma_zalloc_arg: Returning an empty bucket."));
 		cache->uc_allocbucket = bucket;
-		ZONE_UNLOCK(zone);
-		goto zalloc_start;
+		return (true);
 	}
 	/* We are no longer associated with this CPU. */
 	critical_exit();
@@ -2625,71 +2660,39 @@ zalloc_start:
 	if (lockfail && zone->uz_count < zone->uz_count_max)
 		zone->uz_count++;
 
-	if (zone->uz_max_items > 0) {
-		if (zone->uz_items >= zone->uz_max_items)
-			goto zalloc_item;
-		maxbucket = MIN(zone->uz_count,
-		    zone->uz_max_items - zone->uz_items);
-		zone->uz_items += maxbucket;
-	} else
-		maxbucket = zone->uz_count;
-	ZONE_UNLOCK(zone);
-
 	/*
-	 * Now lets just fill a bucket and put it on the free list.  If that
-	 * works we'll restart the allocation from the beginning and it
-	 * will use the just filled bucket.
+	 * Fill a bucket and attempt to use it as the alloc bucket.
 	 */
-	bucket = zone_alloc_bucket(zone, udata, domain, flags, maxbucket);
+	bucket = zone_alloc_bucket(zone, udata, domain, flags);
 	CTR3(KTR_UMA, "uma_zalloc: zone %s(%p) bucket zone returned %p",
 	    zone->uz_name, zone, bucket);
-	ZONE_LOCK(zone);
-	if (bucket != NULL) {
-		if (zone->uz_max_items > 0 && bucket->ub_cnt < maxbucket) {
-			MPASS(zone->uz_items >= maxbucket - bucket->ub_cnt);
-			zone->uz_items -= maxbucket - bucket->ub_cnt;
-			if (zone->uz_sleepers > 0 &&
-			    zone->uz_items < zone->uz_max_items)
-				wakeup_one(zone);
-		}
-		critical_enter();
-		cpu = curcpu;
-		cache = &zone->uz_cpu[cpu];
+	critical_enter();
+	if (bucket == NULL)
+		return (false);
 
-		/*
-		 * See if we lost the race or were migrated.  Cache the
-		 * initialized bucket to make this less likely or claim
-		 * the memory directly.
-		 */
-		if (cache->uc_allocbucket == NULL &&
-		    ((zone->uz_flags & UMA_ZONE_NUMA) == 0 ||
-		    domain == PCPU_GET(domain))) {
-			cache->uc_allocbucket = bucket;
-			zdom->uzd_imax += bucket->ub_cnt;
-		} else if (zone->uz_bkt_count >= zone->uz_bkt_max) {
-			critical_exit();
-			ZONE_UNLOCK(zone);
-			bucket_drain(zone, bucket);
-			bucket_free(zone, bucket, udata);
-			goto zalloc_restart;
-		} else
-			zone_put_bucket(zone, zdom, bucket, false);
-		ZONE_UNLOCK(zone);
-		goto zalloc_start;
-	} else if (zone->uz_max_items > 0) {
-		zone->uz_items -= maxbucket;
-		if (zone->uz_sleepers > 0 &&
-		    zone->uz_items + 1 < zone->uz_max_items)
-			wakeup_one(zone);
-	}
-
 	/*
-	 * We may not be able to get a bucket so return an actual item.
+	 * See if we lost the race or were migrated.  Cache the
+	 * initialized bucket to make this less likely or claim
+	 * the memory directly.
 	 */
-zalloc_item:
-	item = zone_alloc_item_locked(zone, udata, domain, flags);
-
-	return (item);
+	cpu = curcpu;
+	cache = &zone->uz_cpu[cpu];
+	if (cache->uc_allocbucket == NULL &&
+	    ((zone->uz_flags & UMA_ZONE_NUMA) == 0 ||
+	    domain == PCPU_GET(domain))) {
+		cache->uc_allocbucket = bucket;
+		zdom->uzd_imax += bucket->ub_cnt;
+	} else if (zone->uz_bkt_count >= zone->uz_bkt_max) {
+		critical_exit();
+		ZONE_UNLOCK(zone);
+		bucket_drain(zone, bucket);
+		bucket_free(zone, bucket, udata);
+		critical_enter();
+		return (true);
+	} else
+		zone_put_bucket(zone, zdom, bucket, false);
+	ZONE_UNLOCK(zone);
+	return (true);
 }
 
 void *
@@ -2943,9 +2946,10 @@ zone_import(uma_zone_t zone, void **bucket, int max, i
 }
 
 static uma_bucket_t
-zone_alloc_bucket(uma_zone_t zone, void *udata, int domain, int flags, int max)
+zone_alloc_bucket(uma_zone_t zone, void *udata, int domain, int flags)
 {
 	uma_bucket_t bucket;
+	int maxbucket, cnt;
 
 	CTR1(KTR_UMA, "zone_alloc:_bucket domain %d)", domain);
 
@@ -2953,13 +2957,25 @@ zone_alloc_bucket(uma_zone_t zone, void *udata, int do
 	if (domain != UMA_ANYDOMAIN && VM_DOMAIN_EMPTY(domain))
 		domain = UMA_ANYDOMAIN;
 
+	if (zone->uz_max_items > 0) {
+		if (zone->uz_items >= zone->uz_max_items)
+			return (false);
+		maxbucket = MIN(zone->uz_count,
+		    zone->uz_max_items - zone->uz_items);
+		zone->uz_items += maxbucket;
+	} else
+		maxbucket = zone->uz_count;
+	ZONE_UNLOCK(zone);
+
 	/* Don't wait for buckets, preserve caller's NOVM setting. */
 	bucket = bucket_alloc(zone, udata, M_NOWAIT | (flags & M_NOVM));
-	if (bucket == NULL)
-		return (NULL);
+	if (bucket == NULL) {
+		cnt = 0;
+		goto out;
+	}
 
 	bucket->ub_cnt = zone->uz_import(zone->uz_arg, bucket->ub_bucket,
-	    MIN(max, bucket->ub_entries), domain, flags);
+	    MIN(maxbucket, bucket->ub_entries), domain, flags);
 
 	/*
 	 * Initialize the memory if necessary.
@@ -2986,11 +3002,22 @@ zone_alloc_bucket(uma_zone_t zone, void *udata, int do
 		}
 	}
 
+	cnt = bucket->ub_cnt;
 	if (bucket->ub_cnt == 0) {
 		bucket_free(zone, bucket, udata);
 		counter_u64_add(zone->uz_fails, 1);
-		return (NULL);
+		bucket = NULL;
 	}
+out:
+	ZONE_LOCK(zone);
+	if (zone->uz_max_items > 0 && cnt < maxbucket) {
+		MPASS(zone->uz_items >= maxbucket - cnt);
+		zone->uz_items -= maxbucket - cnt;
+		if (zone->uz_sleepers > 0 &&
+		    (cnt == 0 ? zone->uz_items + 1 : zone->uz_items) <
+		    zone->uz_max_items)
+			wakeup_one(zone);
+	}
 
 	return (bucket);
 }
@@ -3024,9 +3051,6 @@ static void *
 zone_alloc_item_locked(uma_zone_t zone, void *udata, int domain, int flags)
 {
 	void *item;
-#ifdef INVARIANTS
-	bool skipdbg;
-#endif
 
 	ZONE_LOCK_ASSERT(zone);
 
@@ -3057,11 +3081,8 @@ zone_alloc_item_locked(uma_zone_t zone, void *udata, i
 		domain = UMA_ANYDOMAIN;
 
 	if (zone->uz_import(zone->uz_arg, &item, 1, domain, flags) != 1)
-		goto fail;
+		goto fail_cnt;
 
-#ifdef INVARIANTS
-	skipdbg = uma_dbg_zskip(zone, item);
-#endif
 	/*
 	 * We have to call both the zone's init (not the keg's init)
 	 * and the zone's ctor.  This is because the item is going from
@@ -3071,24 +3092,12 @@ zone_alloc_item_locked(uma_zone_t zone, void *udata, i
 	if (zone->uz_init != NULL) {
 		if (zone->uz_init(item, zone->uz_size, flags) != 0) {
 			zone_free_item(zone, item, udata, SKIP_FINI | SKIP_CNT);
-			goto fail;
+			goto fail_cnt;
 		}
 	}
-	if (zone->uz_ctor != NULL &&
-#ifdef INVARIANTS
-	    (!skipdbg || zone->uz_ctor != trash_ctor ||
-	    zone->uz_dtor != trash_dtor) &&
-#endif
-	    zone->uz_ctor(item, zone->uz_size, udata, flags) != 0) {
-		zone_free_item(zone, item, udata, SKIP_DTOR | SKIP_CNT);
+	item = item_ctor(zone, udata, flags, item);
+	if (item == NULL)
 		goto fail;
-	}
-#ifdef INVARIANTS
-	if (!skipdbg)
-		uma_dbg_alloc(zone, NULL, item);
-#endif
-	if (flags & M_ZERO)
-		uma_zero_item(item, zone);
 
 	counter_u64_add(zone->uz_allocs, 1);
 	CTR3(KTR_UMA, "zone_alloc_item item %p from %s(%p)", item,
@@ -3096,13 +3105,15 @@ zone_alloc_item_locked(uma_zone_t zone, void *udata, i
 
 	return (item);
 
+fail_cnt:
+	counter_u64_add(zone->uz_fails, 1);
 fail:
 	if (zone->uz_max_items > 0) {
 		ZONE_LOCK(zone);
+		/* XXX Decrement without wakeup */
 		zone->uz_items--;
 		ZONE_UNLOCK(zone);
 	}
-	counter_u64_add(zone->uz_fails, 1);
 	CTR2(KTR_UMA, "zone_alloc_item failed from %s(%p)",
 	    zone->uz_name, zone);
 	return (NULL);


More information about the svn-src-head mailing list