Re: [PATCH 4/6] bcachefs: bch2_run_online_recovery_passes()

Brian Foster
2023-12-08T20:24:09Z

On Wed, Dec 06, 2023 at 03:33:08PM -0500, Kent Overstreet wrote:
> Add a new helper for running online recovery passes - i.e. online fsck.
> This is a subset of our normal recovery passes, and does not - for now -
> use or follow c->curr_recovery_pass.
>
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> ---
> fs/bcachefs/bcachefs.h | 7 ++++++
> fs/bcachefs/recovery.c | 51 ++++++++++++++++++++++++++++--------------
> fs/bcachefs/recovery.h | 1 +
> 3 files changed, 42 insertions(+), 17 deletions(-)
>
...
> diff --git a/fs/bcachefs/recovery.c b/fs/bcachefs/recovery.c
> index 585b4928964f..262c923b2f1a 100644
> --- a/fs/bcachefs/recovery.c
> +++ b/fs/bcachefs/recovery.c
...
> @@ -651,39 +651,56 @@ static bool should_run_recovery_pass(struct bch_fs *c, enum bch_recovery_pass pa
>
> static int bch2_run_recovery_pass(struct bch_fs *c, enum bch_recovery_pass pass)
> {
> + struct recovery_pass_fn *p = recovery_pass_fns + pass;
> int ret;
>
> - c->curr_recovery_pass = pass;
> + if (!(p->when & PASS_SILENT))
> + printk(KERN_INFO bch2_log_msg(c, "%s..."),
> + bch2_recovery_passes[pass]);
> + ret = p->fn(c);
> + if (ret)
> + return ret;
> + if (!(p->when & PASS_SILENT))
> + printk(KERN_CONT " done\n");
>
> - if (should_run_recovery_pass(c, pass)) {
> - struct recovery_pass_fn *p = recovery_pass_fns + pass;
> + return 0;
> +}
>
> - if (!(p->when & PASS_SILENT))
> - printk(KERN_INFO bch2_log_msg(c, "%s..."),
> - bch2_recovery_passes[pass]);
> - ret = p->fn(c);
> - if (ret)
> - return ret;
> - if (!(p->when & PASS_SILENT))
> - printk(KERN_CONT " done\n");
> +static int bch2_run_recovery_passes(struct bch_fs *c)
> +{
> + int ret = 0;
>
> - c->recovery_passes_complete |= BIT_ULL(pass);
> + while (c->curr_recovery_pass < ARRAY_SIZE(recovery_pass_fns)) {
> + if (should_run_recovery_pass(c, c->curr_recovery_pass)) {
> + ret = bch2_run_recovery_pass(c, c->curr_recovery_pass);
> + if (bch2_err_matches(ret, BCH_ERR_restart_recovery))
> + continue;
> + if (ret)
> + break;
> +
> + c->recovery_passes_complete |= BIT_ULL(c->curr_recovery_pass);
> + }
> + c->curr_recovery_pass++;
> }
>
> - return 0;
> + return ret;
> }
>
> -static int bch2_run_recovery_passes(struct bch_fs *c)
> +int bch2_run_online_recovery_passes(struct bch_fs *c)
> {
> int ret = 0;
>
> - while (c->curr_recovery_pass < ARRAY_SIZE(recovery_pass_fns)) {
> + for (unsigned i = 0; i < ARRAY_SIZE(recovery_pass_fns); i++) {
> + struct recovery_pass_fn *p = recovery_pass_fns + c->curr_recovery_pass;

Maybe I'm confused, but should this be using i?

Brian

> +
> + if (!(p->when & PASS_ONLINE))
> + continue;
> +
> ret = bch2_run_recovery_pass(c, c->curr_recovery_pass);
> if (bch2_err_matches(ret, BCH_ERR_restart_recovery))
> continue;
> if (ret)
> break;
> - c->curr_recovery_pass++;
> }
>
> return ret;
> diff --git a/fs/bcachefs/recovery.h b/fs/bcachefs/recovery.h
> index 852d30567da9..447590f60609 100644
> --- a/fs/bcachefs/recovery.h
> +++ b/fs/bcachefs/recovery.h
> @@ -25,6 +25,7 @@ static inline int bch2_run_explicit_recovery_pass(struct bch_fs *c,
> }
> }
>
> +int bch2_run_online_recovery_passes(struct bch_fs *);
> u64 bch2_fsck_recovery_passes(void);
>
> int bch2_fs_recovery(struct bch_fs *);
> --
> 2.42.0
>
>