bcachefs: Add locking for bch_fs.curr_recovery_pass

Recovery can rewind in certain situations - when we discover we need to
run a pass that doesn't normally run.

This can happen from another thread for btree node read errors, so we
need a bit of locking.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/bcachefs.h b/fs/bcachefs/bcachefs.h
index 7db81e182c3c4e31b8a2b20f421c83e12f014d99..fbd89f91625d0ee492988da5c0fce2568062d884 100644 (file)
--- a/fs/bcachefs/bcachefs.h
+++ b/fs/bcachefs/bcachefs.h
@@ -1060,6 +1060,7 @@ struct bch_fs {
        u64                     recovery_passes_complete;
        /* never rewinds version of curr_recovery_pass */
        enum bch_recovery_pass  recovery_pass_done;
+       spinlock_t              recovery_pass_lock;
        struct semaphore        online_fsck_mutex;
 
        /* DEBUG JUNK */

diff --git a/fs/bcachefs/recovery_passes.c b/fs/bcachefs/recovery_passes.c
index 1cc010bf1695555082ed660a0a6378ebe793d032..5e7722cc0879a9579ab10c862f5ff1e38ac035fc 100644 (file)
--- a/fs/bcachefs/recovery_passes.c
+++ b/fs/bcachefs/recovery_passes.c
@@ -100,8 +100,8 @@ u64 bch2_recovery_passes_from_stable(u64 v)
 /*
  * For when we need to rewind recovery passes and run a pass we skipped:
  */
-int bch2_run_explicit_recovery_pass(struct bch_fs *c,
-                                   enum bch_recovery_pass pass)
+static int __bch2_run_explicit_recovery_pass(struct bch_fs *c,
+                                            enum bch_recovery_pass pass)
 {
        if (c->opts.recovery_passes & BIT_ULL(pass))
                return 0;
@@ -109,6 +109,13 @@ int bch2_run_explicit_recovery_pass(struct bch_fs *c,
        if (c->curr_recovery_pass == ARRAY_SIZE(recovery_pass_fns))
                return -BCH_ERR_not_in_recovery;
 
+       if (pass < BCH_RECOVERY_PASS_set_may_go_rw &&
+           c->curr_recovery_pass >= BCH_RECOVERY_PASS_set_may_go_rw) {
+               bch_info(c, "need recovery pass %s (%u), but already rw",
+                        bch2_recovery_passes[pass], pass);
+               return -BCH_ERR_cannot_rewind_recovery;
+       }
+
        bch_info(c, "running explicit recovery pass %s (%u), currently at %s (%u)",
                 bch2_recovery_passes[pass], pass,
                 bch2_recovery_passes[c->curr_recovery_pass], c->curr_recovery_pass);
@@ -124,6 +131,16 @@ int bch2_run_explicit_recovery_pass(struct bch_fs *c,
        }
 }
 
+int bch2_run_explicit_recovery_pass(struct bch_fs *c,
+                                   enum bch_recovery_pass pass)
+{
+       unsigned long flags;
+       spin_lock_irqsave(&c->recovery_pass_lock, flags);
+       int ret = __bch2_run_explicit_recovery_pass(c, pass);
+       spin_unlock_irqrestore(&c->recovery_pass_lock, flags);
+       return ret;
+}
+
 int bch2_run_explicit_recovery_pass_persistent(struct bch_fs *c,
                                               enum bch_recovery_pass pass)
 {
@@ -237,30 +254,51 @@ int bch2_run_recovery_passes(struct bch_fs *c)
        c->opts.recovery_passes_exclude &= ~BCH_RECOVERY_PASS_set_may_go_rw;
 
        while (c->curr_recovery_pass < ARRAY_SIZE(recovery_pass_fns)) {
+               spin_lock_irq(&c->recovery_pass_lock);
+               unsigned pass = c->curr_recovery_pass;
+
                if (c->opts.recovery_pass_last &&
-                   c->curr_recovery_pass > c->opts.recovery_pass_last)
+                   c->curr_recovery_pass > c->opts.recovery_pass_last) {
+                       spin_unlock_irq(&c->recovery_pass_lock);
                        break;
+               }
 
-               if (should_run_recovery_pass(c, c->curr_recovery_pass)) {
-                       unsigned pass = c->curr_recovery_pass;
-
-                       ret =   bch2_run_recovery_pass(c, c->curr_recovery_pass) ?:
-                               bch2_journal_flush(&c->journal);
-                       if (bch2_err_matches(ret, BCH_ERR_restart_recovery) ||
-                           (ret && c->curr_recovery_pass < pass))
-                               continue;
-                       if (ret)
-                               break;
-
-                       c->recovery_passes_complete |= BIT_ULL(c->curr_recovery_pass);
+               if (!should_run_recovery_pass(c, pass)) {
+                       c->curr_recovery_pass++;
+                       c->recovery_pass_done = max(c->recovery_pass_done, pass);
+                       spin_unlock_irq(&c->recovery_pass_lock);
+                       continue;
+               }
+               spin_unlock_irq(&c->recovery_pass_lock);
+
+               ret =   bch2_run_recovery_pass(c, pass) ?:
+                       bch2_journal_flush(&c->journal);
+
+               spin_lock_irq(&c->recovery_pass_lock);
+               if (c->curr_recovery_pass < pass) {
+                       /*
+                        * bch2_run_explicit_recovery_pass() was called: we
+                        * can't always catch -BCH_ERR_restart_recovery because
+                        * it may have been called from another thread (btree
+                        * node read completion)
+                        */
+                       spin_unlock_irq(&c->recovery_pass_lock);
+                       continue;
+               } else if (c->curr_recovery_pass == pass) {
+                       c->curr_recovery_pass++;
+               } else {
+                       BUG();
                }
+               spin_unlock_irq(&c->recovery_pass_lock);
 
-               c->recovery_pass_done = max(c->recovery_pass_done, c->curr_recovery_pass);
+               if (ret)
+                       break;
 
-               if (!test_bit(BCH_FS_error, &c->flags))
-                       bch2_clear_recovery_pass_required(c, c->curr_recovery_pass);
+               c->recovery_passes_complete |= BIT_ULL(pass);
+               c->recovery_pass_done = max(c->recovery_pass_done, pass);
 
-               c->curr_recovery_pass++;
+               if (!test_bit(BCH_FS_error, &c->flags))
+                       bch2_clear_recovery_pass_required(c, pass);
        }
 
        return ret;

diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index 17442df7326d960468cc2a903487396f2a30e02f..d6411324cd3f8fc3b71410be5536aa99943305a7 100644 (file)
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -766,6 +766,7 @@ static struct bch_fs *bch2_fs_alloc(struct bch_sb *sb, struct bch_opts opts)
 
        refcount_set(&c->ro_ref, 1);
        init_waitqueue_head(&c->ro_ref_wait);
+       spin_lock_init(&c->recovery_pass_lock);
        sema_init(&c->online_fsck_mutex, 1);
 
        init_rwsem(&c->gc_lock);