bcachefs: Fix unmount path

There was a long standing race in the mount/unmount code - the VFS
intends for mount/unmount synchronizatino to be handled by the list of
superblocks, but we were still holding devices open after tearing down
our superblock in the unmount path.

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

diff --git a/fs/bcachefs/bcachefs.h b/fs/bcachefs/bcachefs.h
index baa8801c5412a4a47cfcef20afbfed87128f556d..f60d530313dcbd4af7c8007ceb27612b2250ceb6 100644 (file)
--- a/fs/bcachefs/bcachefs.h
+++ b/fs/bcachefs/bcachefs.h
@@ -491,7 +491,6 @@ enum {
        BCH_FS_ERRORS_FIXED,
 
        /* misc: */
-       BCH_FS_BDEV_MOUNTED,
        BCH_FS_FIXED_GENS,
        BCH_FS_ALLOC_WRITTEN,
        BCH_FS_REBUILD_REPLICAS,

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index a4a3085e51850b9962fd9096cb2bc923e1506002..3239c4717cc6c12c55da4aeb2366f80ef13b609b 100644 (file)
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1300,91 +1300,36 @@ static struct bch_fs *bch2_path_to_fs(const char *path)
                return ERR_PTR(ret);
 
        c = bch2_dev_to_fs(dev);
-       return c ?: ERR_PTR(-ENOENT);
-}
-
-static struct bch_fs *__bch2_open_as_blockdevs(const char *dev_name, char * const *devs,
-                                              unsigned nr_devs, struct bch_opts opts)
-{
-       struct bch_fs *c, *c1, *c2;
-       size_t i;
-
-       if (!nr_devs)
-               return ERR_PTR(-EINVAL);
-
-       c = bch2_fs_open(devs, nr_devs, opts);
-
-       if (IS_ERR(c) && PTR_ERR(c) == -EBUSY) {
-               /*
-                * Already open?
-                * Look up each block device, make sure they all belong to a
-                * filesystem and they all belong to the _same_ filesystem
-                */
-
-               c1 = bch2_path_to_fs(devs[0]);
-               if (IS_ERR(c1))
-                       return c;
-
-               for (i = 1; i < nr_devs; i++) {
-                       c2 = bch2_path_to_fs(devs[i]);
-                       if (!IS_ERR(c2))
-                               closure_put(&c2->cl);
-
-                       if (c1 != c2) {
-                               closure_put(&c1->cl);
-                               return c;
-                       }
-               }
-
-               c = c1;
-       }
-
-       if (IS_ERR(c))
-               return c;
-
-       down_write(&c->state_lock);
-
-       if (!test_bit(BCH_FS_STARTED, &c->flags)) {
-               up_write(&c->state_lock);
+       if (c)
                closure_put(&c->cl);
-               pr_err("err mounting %s: incomplete filesystem", dev_name);
-               return ERR_PTR(-EINVAL);
-       }
-
-       up_write(&c->state_lock);
-
-       set_bit(BCH_FS_BDEV_MOUNTED, &c->flags);
-       return c;
+       return c ?: ERR_PTR(-ENOENT);
 }
 
-static struct bch_fs *bch2_open_as_blockdevs(const char *_dev_name,
-                                            struct bch_opts opts)
+static char **split_devs(const char *_dev_name, unsigned *nr)
 {
        char *dev_name = NULL, **devs = NULL, *s;
-       struct bch_fs *c = ERR_PTR(-ENOMEM);
        size_t i, nr_devs = 0;
 
        dev_name = kstrdup(_dev_name, GFP_KERNEL);
        if (!dev_name)
-               goto err;
+               return NULL;
 
        for (s = dev_name; s; s = strchr(s + 1, ':'))
                nr_devs++;
 
-       devs = kcalloc(nr_devs, sizeof(const char *), GFP_KERNEL);
-       if (!devs)
-               goto err;
+       devs = kcalloc(nr_devs + 1, sizeof(const char *), GFP_KERNEL);
+       if (!devs) {
+               kfree(dev_name);
+               return NULL;
+       }
 
        for (i = 0, s = dev_name;
             s;
             (s = strchr(s, ':')) && (*s++ = '\0'))
                devs[i++] = s;
 
-       c = __bch2_open_as_blockdevs(_dev_name, devs, nr_devs, opts);
-err:
-       kfree(devs);
-       kfree(dev_name);
-       return c;
+       *nr = nr_devs;
+       return devs;
 }
 
 static int bch2_remount(struct super_block *sb, int *flags, char *data)
@@ -1471,6 +1416,13 @@ static int bch2_show_options(struct seq_file *seq, struct dentry *root)
        return 0;
 }
 
+static void bch2_put_super(struct super_block *sb)
+{
+       struct bch_fs *c = sb->s_fs_info;
+
+       __bch2_fs_stop(c);
+}
+
 static const struct super_operations bch_super_operations = {
        .alloc_inode    = bch2_alloc_inode,
        .destroy_inode  = bch2_destroy_inode,
@@ -1481,24 +1433,39 @@ static const struct super_operations bch_super_operations = {
        .show_devname   = bch2_show_devname,
        .show_options   = bch2_show_options,
        .remount_fs     = bch2_remount,
-#if 0
        .put_super      = bch2_put_super,
+#if 0
        .freeze_fs      = bch2_freeze,
        .unfreeze_fs    = bch2_unfreeze,
 #endif
 };
 
-static int bch2_test_super(struct super_block *s, void *data)
-{
-       return s->s_fs_info == data;
-}
-
 static int bch2_set_super(struct super_block *s, void *data)
 {
        s->s_fs_info = data;
        return 0;
 }
 
+static int bch2_noset_super(struct super_block *s, void *data)
+{
+       return -EBUSY;
+}
+
+static int bch2_test_super(struct super_block *s, void *data)
+{
+       struct bch_fs *c = s->s_fs_info;
+       struct bch_fs **devs = data;
+       unsigned i;
+
+       if (!c)
+               return false;
+
+       for (i = 0; devs[i]; i++)
+               if (c != devs[i])
+                       return false;
+       return true;
+}
+
 static struct dentry *bch2_mount(struct file_system_type *fs_type,
                                 int flags, const char *dev_name, void *data)
 {
@@ -1507,7 +1474,9 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
        struct super_block *sb;
        struct inode *vinode;
        struct bch_opts opts = bch2_opts_empty();
-       unsigned i;
+       char **devs;
+       struct bch_fs **devs_to_fs = NULL;
+       unsigned i, nr_devs;
        int ret;
 
        opt_set(opts, read_only, (flags & SB_RDONLY) != 0);
@@ -1516,21 +1485,41 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
        if (ret)
                return ERR_PTR(ret);
 
-       c = bch2_open_as_blockdevs(dev_name, opts);
-       if (IS_ERR(c))
-               return ERR_CAST(c);
+       devs = split_devs(dev_name, &nr_devs);
+       if (!devs)
+               return ERR_PTR(-ENOMEM);
 
-       sb = sget(fs_type, bch2_test_super, bch2_set_super, flags|SB_NOSEC, c);
-       if (IS_ERR(sb)) {
-               closure_put(&c->cl);
-               return ERR_CAST(sb);
+       devs_to_fs = kcalloc(nr_devs + 1, sizeof(void *), GFP_KERNEL);
+       if (!devs_to_fs) {
+               sb = ERR_PTR(-ENOMEM);
+               goto got_sb;
        }
 
-       BUG_ON(sb->s_fs_info != c);
+       for (i = 0; i < nr_devs; i++)
+               devs_to_fs[i] = bch2_path_to_fs(devs[i]);
 
-       if (sb->s_root) {
-               closure_put(&c->cl);
+       sb = sget(fs_type, bch2_test_super, bch2_noset_super,
+                 flags|SB_NOSEC, devs_to_fs);
+       if (!IS_ERR(sb))
+               goto got_sb;
+
+       c = bch2_fs_open(devs, nr_devs, opts);
+
+       if (!IS_ERR(c))
+               sb = sget(fs_type, NULL, bch2_set_super, flags|SB_NOSEC, c);
+       else
+               sb = ERR_CAST(c);
+got_sb:
+       kfree(devs_to_fs);
+       kfree(devs[0]);
+       kfree(devs);
+
+       if (IS_ERR(sb))
+               return ERR_CAST(sb);
+
+       c = sb->s_fs_info;
 
+       if (sb->s_root) {
                if ((flags ^ sb->s_flags) & SB_RDONLY) {
                        ret = -EBUSY;
                        goto err_put_super;
@@ -1603,11 +1592,7 @@ static void bch2_kill_sb(struct super_block *sb)
        struct bch_fs *c = sb->s_fs_info;
 
        generic_shutdown_super(sb);
-
-       if (test_bit(BCH_FS_BDEV_MOUNTED, &c->flags))
-               bch2_fs_stop(c);
-       else
-               closure_put(&c->cl);
+       bch2_fs_free(c);
 }
 
 static struct file_system_type bcache_fs_type = {

diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index cb2b719165cee0069c34c3d7976d1f019b70883a..d0d46023163cd7e69e4dc828c84d4db8eef3e301 100644 (file)
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -465,7 +465,7 @@ int bch2_fs_read_write_early(struct bch_fs *c)
 
 /* Filesystem startup/shutdown: */
 
-static void bch2_fs_free(struct bch_fs *c)
+static void __bch2_fs_free(struct bch_fs *c)
 {
        unsigned i;
 
@@ -522,10 +522,10 @@ static void bch2_fs_release(struct kobject *kobj)
 {
        struct bch_fs *c = container_of(kobj, struct bch_fs, kobj);
 
-       bch2_fs_free(c);
+       __bch2_fs_free(c);
 }
 
-void bch2_fs_stop(struct bch_fs *c)
+void __bch2_fs_stop(struct bch_fs *c)
 {
        struct bch_dev *ca;
        unsigned i;
@@ -555,13 +555,6 @@ void bch2_fs_stop(struct bch_fs *c)
        kobject_put(&c->opts_dir);
        kobject_put(&c->internal);
 
-       mutex_lock(&bch_fs_list_lock);
-       list_del(&c->list);
-       mutex_unlock(&bch_fs_list_lock);
-
-       closure_sync(&c->cl);
-       closure_debug_destroy(&c->cl);
-
        /* btree prefetch might have kicked off reads in the background: */
        bch2_btree_flush_all_reads(c);
 
@@ -571,16 +564,39 @@ void bch2_fs_stop(struct bch_fs *c)
        cancel_work_sync(&c->btree_write_error_work);
        cancel_delayed_work_sync(&c->pd_controllers_update);
        cancel_work_sync(&c->read_only_work);
+}
 
-       for (i = 0; i < c->sb.nr_devices; i++)
-               if (c->devs[i])
-                       bch2_dev_free(rcu_dereference_protected(c->devs[i], 1));
+void bch2_fs_free(struct bch_fs *c)
+{
+       unsigned i;
+
+       mutex_lock(&bch_fs_list_lock);
+       list_del(&c->list);
+       mutex_unlock(&bch_fs_list_lock);
+
+       closure_sync(&c->cl);
+       closure_debug_destroy(&c->cl);
+
+       for (i = 0; i < c->sb.nr_devices; i++) {
+               struct bch_dev *ca = rcu_dereference_protected(c->devs[i], true);
+
+               if (ca) {
+                       bch2_free_super(&ca->disk_sb);
+                       bch2_dev_free(ca);
+               }
+       }
 
        bch_verbose(c, "shutdown complete");
 
        kobject_put(&c->kobj);
 }
 
+void bch2_fs_stop(struct bch_fs *c)
+{
+       __bch2_fs_stop(c);
+       bch2_fs_free(c);
+}
+
 static const char *bch2_fs_online(struct bch_fs *c)
 {
        struct bch_dev *ca;

diff --git a/fs/bcachefs/super.h b/fs/bcachefs/super.h
index fab4bee9c90e7e4c2236961abc1ed9c2e738deae..795229e2d6a11c31425d2326b751ef58d90da8e0 100644 (file)
--- a/fs/bcachefs/super.h
+++ b/fs/bcachefs/super.h
@@ -230,6 +230,8 @@ static inline void bch2_fs_lazy_rw(struct bch_fs *c)
                bch2_fs_read_write_early(c);
 }
 
+void __bch2_fs_stop(struct bch_fs *);
+void bch2_fs_free(struct bch_fs *);
 void bch2_fs_stop(struct bch_fs *);
 
 int bch2_fs_start(struct bch_fs *);