bcache: fix deadlock in bcache_allocator

[ Upstream commit 84c529aea182939e68f618ed9813740c9165c7eb ]

bcache_allocator can call the following:

 bch_allocator_thread()
  -> bch_prio_write()
     -> bch_bucket_alloc()
        -> wait on &ca->set->bucket_wait

But the wake up event on bucket_wait is supposed to come from
bch_allocator_thread() itself => deadlock:

[ 1158.490744] INFO: task bcache_allocato:15861 blocked for more than 10 seconds.
[ 1158.495929]       Not tainted 5.3.0-050300rc3-generic #201908042232
[ 1158.500653] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 1158.504413] bcache_allocato D    0 15861      2 0x80004000
[ 1158.504419] Call Trace:
[ 1158.504429]  __schedule+0x2a8/0x670
[ 1158.504432]  schedule+0x2d/0x90
[ 1158.504448]  bch_bucket_alloc+0xe5/0x370 [bcache]
[ 1158.504453]  ? wait_woken+0x80/0x80
[ 1158.504466]  bch_prio_write+0x1dc/0x390 [bcache]
[ 1158.504476]  bch_allocator_thread+0x233/0x490 [bcache]
[ 1158.504491]  kthread+0x121/0x140
[ 1158.504503]  ? invalidate_buckets+0x890/0x890 [bcache]
[ 1158.504506]  ? kthread_park+0xb0/0xb0
[ 1158.504510]  ret_from_fork+0x35/0x40

Fix by making the call to bch_prio_write() non-blocking, so that
bch_allocator_thread() never waits on itself.

Moreover, make sure to wake up the garbage collector thread when
bch_prio_write() is failing to allocate buckets.

BugLink: https://bugs.launchpad.net/bugs/1784665
BugLink: https://bugs.launchpad.net/bugs/1796292
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Signed-off-by: Coly Li <colyli@suse.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
index 6f776823b9ba56899fcc1548df11302f4ad18556..a1df0d95151c67b15d78802193be03530441d4d0 100644 (file)
--- a/drivers/md/bcache/alloc.c
+++ b/drivers/md/bcache/alloc.c
@@ -377,7 +377,10 @@ retry_invalidate:
                        if (!fifo_full(&ca->free_inc))
                                goto retry_invalidate;
 
                        if (!fifo_full(&ca->free_inc))
                                goto retry_invalidate;
 

-                       bch_prio_write(ca);
+                       if (bch_prio_write(ca, false) < 0) {
+                               ca->invalidate_needs_gc = 1;
+                               wake_up_gc(ca->set);
+                       }

                }
        }
 out:
                }
        }
 out:

diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 013e35a9e317a44c09426af9b26ff894c5a94692..deb924e1d790acf9afce209f0dac9d1aa0025d9a 100644 (file)
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -977,7 +977,7 @@ bool bch_cached_dev_error(struct cached_dev *dc);
 __printf(2, 3)
 bool bch_cache_set_error(struct cache_set *c, const char *fmt, ...);
 
 __printf(2, 3)
 bool bch_cache_set_error(struct cache_set *c, const char *fmt, ...);
 

-void bch_prio_write(struct cache *ca);
+int bch_prio_write(struct cache *ca, bool wait);

 void bch_write_bdev_super(struct cached_dev *dc, struct closure *parent);
 
 extern struct workqueue_struct *bcache_wq;
 void bch_write_bdev_super(struct cached_dev *dc, struct closure *parent);
 
 extern struct workqueue_struct *bcache_wq;

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index d2654880b7b9eb5afaf667b1f6ac19c6ce8cdbb3..64999c7a8033fc9fcf943286ef2d3fbcc4a1fc30 100644 (file)
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -529,12 +529,29 @@ static void prio_io(struct cache *ca, uint64_t bucket, int op,
        closure_sync(cl);
 }
 
        closure_sync(cl);
 }
 

-void bch_prio_write(struct cache *ca)
+int bch_prio_write(struct cache *ca, bool wait)

 {
        int i;
        struct bucket *b;
        struct closure cl;
 
 {
        int i;
        struct bucket *b;
        struct closure cl;
 

+       pr_debug("free_prio=%zu, free_none=%zu, free_inc=%zu",
+                fifo_used(&ca->free[RESERVE_PRIO]),
+                fifo_used(&ca->free[RESERVE_NONE]),
+                fifo_used(&ca->free_inc));
+
+       /*
+        * Pre-check if there are enough free buckets. In the non-blocking
+        * scenario it's better to fail early rather than starting to allocate
+        * buckets and do a cleanup later in case of failure.
+        */
+       if (!wait) {
+               size_t avail = fifo_used(&ca->free[RESERVE_PRIO]) +
+                              fifo_used(&ca->free[RESERVE_NONE]);
+               if (prio_buckets(ca) > avail)
+                       return -ENOMEM;
+       }
+

        closure_init_stack(&cl);
 
        lockdep_assert_held(&ca->set->bucket_lock);
        closure_init_stack(&cl);
 
        lockdep_assert_held(&ca->set->bucket_lock);


@@ -544,9 +561,6 @@ void bch_prio_write(struct cache *ca)
        atomic_long_add(ca->sb.bucket_size * prio_buckets(ca),
                        &ca->meta_sectors_written);
 
        atomic_long_add(ca->sb.bucket_size * prio_buckets(ca),
                        &ca->meta_sectors_written);
 

-       //pr_debug("free %zu, free_inc %zu, unused %zu", fifo_used(&ca->free),
-       //       fifo_used(&ca->free_inc), fifo_used(&ca->unused));
-

        for (i = prio_buckets(ca) - 1; i >= 0; --i) {
                long bucket;
                struct prio_set *p = ca->disk_buckets;
        for (i = prio_buckets(ca) - 1; i >= 0; --i) {
                long bucket;
                struct prio_set *p = ca->disk_buckets;


@@ -564,7 +578,7 @@ void bch_prio_write(struct cache *ca)
                p->magic        = pset_magic(&ca->sb);
                p->csum         = bch_crc64(&p->magic, bucket_bytes(ca) - 8);
 
                p->magic        = pset_magic(&ca->sb);
                p->csum         = bch_crc64(&p->magic, bucket_bytes(ca) - 8);
 

-               bucket = bch_bucket_alloc(ca, RESERVE_PRIO, true);
+               bucket = bch_bucket_alloc(ca, RESERVE_PRIO, wait);

                BUG_ON(bucket == -1);
 
                mutex_unlock(&ca->set->bucket_lock);
                BUG_ON(bucket == -1);
 
                mutex_unlock(&ca->set->bucket_lock);


@@ -593,6 +607,7 @@ void bch_prio_write(struct cache *ca)
 
                ca->prio_last_buckets[i] = ca->prio_buckets[i];
        }
 
                ca->prio_last_buckets[i] = ca->prio_buckets[i];
        }

+       return 0;

 }
 
 static void prio_read(struct cache *ca, uint64_t bucket)
 }
 
 static void prio_read(struct cache *ca, uint64_t bucket)


@@ -1962,7 +1977,7 @@ static int run_cache_set(struct cache_set *c)
 
                mutex_lock(&c->bucket_lock);
                for_each_cache(ca, c, i)
 
                mutex_lock(&c->bucket_lock);
                for_each_cache(ca, c, i)

-                       bch_prio_write(ca);
+                       bch_prio_write(ca, true);

                mutex_unlock(&c->bucket_lock);
 
                err = "cannot allocate new UUID bucket";
                mutex_unlock(&c->bucket_lock);
 
                err = "cannot allocate new UUID bucket";