svn commit: r368189 - head/sys/vm

[Mark Johnston]

Author: markj
Date: Mon Nov 30 16:18:33 2020
New Revision: 368189
URL: https://svnweb.freebsd.org/changeset/base/368189

Log:
  uma: Avoid allocating buckets with the cross-domain lock held
  
  Allocation of a bucket can trigger a cross-domain free in the bucket
  zone, e.g., if the per-CPU alloc bucket is empty, we free it and get
  migrated to a remote domain.  This can lead to deadlocks since a bucket
  zone may allocate buckets from itself or a pair of bucket zones could be
  allocating from each other.
  
  Fix the problem by dropping the cross-domain lock before allocating a
  new bucket and handling refill races.  Use a list of empty buckets to
  ensure that we can make forward progress.
  
  Reported by:	imp, mjg (witness(9) warnings)
  Discussed with:	jeff
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D27341

Modified:
  head/sys/vm/uma_core.c

Modified: head/sys/vm/uma_core.c
==============================================================================
--- head/sys/vm/uma_core.c	Mon Nov 30 15:04:35 2020	(r368188)
+++ head/sys/vm/uma_core.c	Mon Nov 30 16:18:33 2020	(r368189)
@@ -4250,7 +4250,7 @@ zfree_item:
 static void
 zone_free_cross(uma_zone_t zone, uma_bucket_t bucket, void *udata)
 {
-	struct uma_bucketlist fullbuckets;
+	struct uma_bucketlist emptybuckets, fullbuckets;
 	uma_zone_domain_t zdom;
 	uma_bucket_t b;
 	smr_seq_t seq;
@@ -4274,31 +4274,57 @@ zone_free_cross(uma_zone_t zone, uma_bucket_t bucket, 
 	 * lock on the current crossfree bucket.  A full matrix with
 	 * per-domain locking could be used if necessary.
 	 */
+	STAILQ_INIT(&emptybuckets);
 	STAILQ_INIT(&fullbuckets);
 	ZONE_CROSS_LOCK(zone);
-	while (bucket->ub_cnt > 0) {
+	for (; bucket->ub_cnt > 0; bucket->ub_cnt--) {
 		item = bucket->ub_bucket[bucket->ub_cnt - 1];
 		domain = item_domain(item);
 		zdom = ZDOM_GET(zone, domain);
 		if (zdom->uzd_cross == NULL) {
-			zdom->uzd_cross = bucket_alloc(zone, udata, M_NOWAIT);
-			if (zdom->uzd_cross == NULL)
-				break;
+			if ((b = STAILQ_FIRST(&emptybuckets)) != NULL) {
+				STAILQ_REMOVE_HEAD(&emptybuckets, ub_link);
+				zdom->uzd_cross = b;
+			} else {
+				/*
+				 * Avoid allocating a bucket with the cross lock
+				 * held, since allocation can trigger a
+				 * cross-domain free and bucket zones may
+				 * allocate from each other.
+				 */
+				ZONE_CROSS_UNLOCK(zone);
+				b = bucket_alloc(zone, udata, M_NOWAIT);
+				if (b == NULL)
+					goto out;
+				ZONE_CROSS_LOCK(zone);
+				if (zdom->uzd_cross != NULL) {
+					STAILQ_INSERT_HEAD(&emptybuckets, b,
+					    ub_link);
+				} else {
+					zdom->uzd_cross = b;
+				}
+			}
 		}
 		b = zdom->uzd_cross;
 		b->ub_bucket[b->ub_cnt++] = item;
 		b->ub_seq = seq;
 		if (b->ub_cnt == b->ub_entries) {
 			STAILQ_INSERT_HEAD(&fullbuckets, b, ub_link);
-			zdom->uzd_cross = NULL;
+			if ((b = STAILQ_FIRST(&emptybuckets)) != NULL)
+				STAILQ_REMOVE_HEAD(&emptybuckets, ub_link);
+			zdom->uzd_cross = b;
 		}
-		bucket->ub_cnt--;
 	}
 	ZONE_CROSS_UNLOCK(zone);
+out:
 	if (bucket->ub_cnt == 0)
 		bucket->ub_seq = SMR_SEQ_INVALID;
 	bucket_free(zone, bucket, udata);
 
+	while ((b = STAILQ_FIRST(&emptybuckets)) != NULL) {
+		STAILQ_REMOVE_HEAD(&emptybuckets, ub_link);
+		bucket_free(zone, b, udata);
+	}
 	while ((b = STAILQ_FIRST(&fullbuckets)) != NULL) {
 		STAILQ_REMOVE_HEAD(&fullbuckets, ub_link);
 		domain = item_domain(b->ub_bucket[0]);