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]);
_______________________________________________
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"

Reply via email to