bcachefs: Clean up option pre/post hooks, small fixes

The helpers are now:
- bch2_opt_hook_pre_set()
- bch2_opts_hooks_pre_set()
- bch2_opt_hook_post_set

Fix a bug where the filesystem discard option would incorrectly be
changed when setting the device option, and don't trigger rebalance
scans unnecessarily (when options aren't changing).

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

diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c
index b3fcffc91d6fa92b62b1d702007a68b7ee2114d5..386482ff8e7bdd58c50bf5c3557724a81f84482d 100644 (file)
--- a/fs/bcachefs/opts.c
+++ b/fs/bcachefs/opts.c
@@ -7,7 +7,9 @@
 #include "compress.h"
 #include "disk_groups.h"
 #include "error.h"
+#include "movinggc.h"
 #include "opts.h"
+#include "rebalance.h"
 #include "recovery_passes.h"
 #include "super-io.h"
 #include "util.h"
@@ -516,7 +518,7 @@ void bch2_opts_to_text(struct printbuf *out,
        }
 }
 
-int bch2_opt_check_may_set(struct bch_fs *c, struct bch_dev *ca, int id, u64 v)
+int bch2_opt_hook_pre_set(struct bch_fs *c, struct bch_dev *ca, enum bch_opt_id id, u64 v)
 {
        int ret = 0;
 
@@ -534,15 +536,17 @@ int bch2_opt_check_may_set(struct bch_fs *c, struct bch_dev *ca, int id, u64 v)
                if (v)
                        bch2_check_set_feature(c, BCH_FEATURE_ec);
                break;
+       default:
+               break;
        }
 
        return ret;
 }
 
-int bch2_opts_check_may_set(struct bch_fs *c)
+int bch2_opts_hooks_pre_set(struct bch_fs *c)
 {
        for (unsigned i = 0; i < bch2_opts_nr; i++) {
-               int ret = bch2_opt_check_may_set(c, NULL, i, bch2_opt_get_by_id(&c->opts, i));
+               int ret = bch2_opt_hook_pre_set(c, NULL, i, bch2_opt_get_by_id(&c->opts, i));
                if (ret)
                        return ret;
        }
@@ -550,6 +554,52 @@ int bch2_opts_check_may_set(struct bch_fs *c)
        return 0;
 }
 
+void bch2_opt_hook_post_set(struct bch_fs *c, struct bch_dev *ca, u64 inum,
+                           struct bch_opts *new_opts, enum bch_opt_id id)
+{
+       switch (id) {
+       case Opt_foreground_target:
+               if (new_opts->foreground_target &&
+                   !new_opts->background_target)
+                       bch2_set_rebalance_needs_scan(c, inum);
+               break;
+       case Opt_compression:
+               if (new_opts->compression &&
+                   !new_opts->background_compression)
+                       bch2_set_rebalance_needs_scan(c, inum);
+               break;
+       case Opt_background_target:
+               if (new_opts->background_target)
+                       bch2_set_rebalance_needs_scan(c, inum);
+               break;
+       case Opt_background_compression:
+               if (new_opts->background_compression)
+                       bch2_set_rebalance_needs_scan(c, inum);
+               break;
+       case Opt_rebalance_enabled:
+               bch2_rebalance_wakeup(c);
+               break;
+       case Opt_copygc_enabled:
+               bch2_copygc_wakeup(c);
+               break;
+       case Opt_discard:
+               if (!ca) {
+                       mutex_lock(&c->sb_lock);
+                       for_each_member_device(c, ca) {
+                               struct bch_member *m =
+                                       bch2_members_v2_get_mut(ca->disk_sb.sb, ca->dev_idx);
+                               SET_BCH_MEMBER_DISCARD(m, c->opts.discard);
+                       }
+
+                       bch2_write_super(c);
+                       mutex_unlock(&c->sb_lock);
+               }
+               break;
+       default:
+               break;
+       }
+}
+
 int bch2_parse_one_mount_opt(struct bch_fs *c, struct bch_opts *opts,
                             struct printbuf *parse_later,
                             const char *name, const char *val)
@@ -709,9 +759,11 @@ int bch2_opts_from_sb(struct bch_opts *opts, struct bch_sb *sb)
        return 0;
 }
 
-void __bch2_opt_set_sb(struct bch_sb *sb, int dev_idx,
+bool __bch2_opt_set_sb(struct bch_sb *sb, int dev_idx,
                       const struct bch_option *opt, u64 v)
 {
+       bool changed = false;
+
        if (opt->flags & OPT_SB_FIELD_SECTORS)
                v >>= 9;
 
@@ -721,26 +773,35 @@ void __bch2_opt_set_sb(struct bch_sb *sb, int dev_idx,
        if (opt->flags & OPT_SB_FIELD_ONE_BIAS)
                v++;
 
-       if ((opt->flags & OPT_FS) && opt->set_sb && dev_idx < 0)
+       if ((opt->flags & OPT_FS) && opt->set_sb && dev_idx < 0) {
+               changed = v != opt->get_sb(sb);
+
                opt->set_sb(sb, v);
+       }
 
        if ((opt->flags & OPT_DEVICE) && opt->set_member && dev_idx >= 0) {
                if (WARN(!bch2_member_exists(sb, dev_idx),
                         "tried to set device option %s on nonexistent device %i",
                         opt->attr.name, dev_idx))
-                       return;
+                       return false;
 
-               opt->set_member(bch2_members_v2_get_mut(sb, dev_idx), v);
+               struct bch_member *m = bch2_members_v2_get_mut(sb, dev_idx);
+               changed = v != opt->get_member(m);
+               opt->set_member(m, v);
        }
+
+       return changed;
 }
 
-void bch2_opt_set_sb(struct bch_fs *c, struct bch_dev *ca,
+bool bch2_opt_set_sb(struct bch_fs *c, struct bch_dev *ca,
                     const struct bch_option *opt, u64 v)
 {
        mutex_lock(&c->sb_lock);
-       __bch2_opt_set_sb(c->disk_sb.sb, ca ? ca->dev_idx : -1, opt, v);
-       bch2_write_super(c);
+       bool changed = __bch2_opt_set_sb(c->disk_sb.sb, ca ? ca->dev_idx : -1, opt, v);
+       if (changed)
+               bch2_write_super(c);
        mutex_unlock(&c->sb_lock);
+       return changed;
 }
 
 /* io opts: */

diff --git a/fs/bcachefs/opts.h b/fs/bcachefs/opts.h
index c97f2a6ad29f5c13e9409c74099138bb1f06ec2b..b7952405d502a6973f2677e4a343d0d1d645e086 100644 (file)
--- a/fs/bcachefs/opts.h
+++ b/fs/bcachefs/opts.h
@@ -612,10 +612,10 @@ void bch2_opt_set_by_id(struct bch_opts *, enum bch_opt_id, u64);
 
 u64 bch2_opt_from_sb(struct bch_sb *, enum bch_opt_id, int);
 int bch2_opts_from_sb(struct bch_opts *, struct bch_sb *);
-void __bch2_opt_set_sb(struct bch_sb *, int, const struct bch_option *, u64);
+bool __bch2_opt_set_sb(struct bch_sb *, int, const struct bch_option *, u64);
 
 struct bch_dev;
-void bch2_opt_set_sb(struct bch_fs *, struct bch_dev *, const struct bch_option *, u64);
+bool bch2_opt_set_sb(struct bch_fs *, struct bch_dev *, const struct bch_option *, u64);
 
 int bch2_opt_lookup(const char *);
 int bch2_opt_validate(const struct bch_option *, u64, struct printbuf *);
@@ -632,8 +632,11 @@ void bch2_opts_to_text(struct printbuf *,
                       struct bch_fs *, struct bch_sb *,
                       unsigned, unsigned, unsigned);
 
-int bch2_opt_check_may_set(struct bch_fs *, struct bch_dev *, int, u64);
-int bch2_opts_check_may_set(struct bch_fs *);
+int bch2_opt_hook_pre_set(struct bch_fs *, struct bch_dev *, enum bch_opt_id, u64);
+int bch2_opts_hooks_pre_set(struct bch_fs *);
+void bch2_opt_hook_post_set(struct bch_fs *, struct bch_dev *, u64,
+                           struct bch_opts *, enum bch_opt_id);
+
 int bch2_parse_one_mount_opt(struct bch_fs *, struct bch_opts *,
                             struct printbuf *, const char *, const char *);
 int bch2_parse_mount_opts(struct bch_fs *, struct bch_opts *, struct printbuf *,

diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index 1c3a20d096a389b5c181a9adb43d0e9faf77e932..65aab7ea182e23fa4f72fceb0dae94b3d69d6c07 100644 (file)
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -1130,7 +1130,7 @@ int bch2_fs_start(struct bch_fs *c)
        if (ret)
                goto err;
 
-       ret = bch2_opts_check_may_set(c);
+       ret = bch2_opts_hooks_pre_set(c);
        if (ret)
                goto err;
 

diff --git a/fs/bcachefs/sysfs.c b/fs/bcachefs/sysfs.c
index 82ee333ddd216cadec3eea28f998c4d0f4ab635f..bfdadeae970e31d23bf189369d2a8791a26f291d 100644 (file)
--- a/fs/bcachefs/sysfs.c
+++ b/fs/bcachefs/sysfs.c
@@ -637,36 +637,19 @@ static ssize_t sysfs_opt_store(struct bch_fs *c,
 
        u64 v;
        ret =   bch2_opt_parse(c, opt, strim(tmp), &v, NULL) ?:
-               bch2_opt_check_may_set(c, ca, id, v);
+               bch2_opt_hook_pre_set(c, ca, id, v);
        kfree(tmp);
 
        if (ret < 0)
                goto err;
 
-       bch2_opt_set_sb(c, ca, opt, v);
-       bch2_opt_set_by_id(&c->opts, id, v);
+       bool changed = bch2_opt_set_sb(c, ca, opt, v);
 
-       if (v &&
-           (id == Opt_background_target ||
-            (id == Opt_foreground_target && !c->opts.background_target) ||
-            id == Opt_background_compression ||
-            (id == Opt_compression && !c->opts.background_compression)))
-               bch2_set_rebalance_needs_scan(c, 0);
+       if (!ca)
+               bch2_opt_set_by_id(&c->opts, id, v);
 
-       if (v && id == Opt_rebalance_enabled)
-               bch2_rebalance_wakeup(c);
-
-       if (v && id == Opt_copygc_enabled)
-               bch2_copygc_wakeup(c);
-
-       if (id == Opt_discard && !ca) {
-               mutex_lock(&c->sb_lock);
-               for_each_member_device(c, ca)
-                       opt->set_member(bch2_members_v2_get_mut(ca->disk_sb.sb, ca->dev_idx), v);
-
-               bch2_write_super(c);
-               mutex_unlock(&c->sb_lock);
-       }
+       if (changed)
+               bch2_opt_hook_post_set(c, ca, 0, &c->opts, id);
 
        ret = size;
 err:

diff --git a/fs/bcachefs/xattr.c b/fs/bcachefs/xattr.c
index e6be32003f3b4c4379ac335f56698d9057bd40e1..423ace6272bef03d35438c0014463bbf9d36b880 100644 (file)
--- a/fs/bcachefs/xattr.c
+++ b/fs/bcachefs/xattr.c
@@ -529,7 +529,7 @@ static int bch2_xattr_bcachefs_set(const struct xattr_handler *handler,
                if (ret < 0)
                        goto err_class_exit;
 
-               ret = bch2_opt_check_may_set(c, NULL, opt_id, v);
+               ret = bch2_opt_hook_pre_set(c, NULL, opt_id, v);
                if (ret < 0)
                        goto err_class_exit;